This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fortran function calls with arguments
On 2019-01-29 9:47 a.m., Richard Bunt wrote:
> Hi Simon,
>
> Many thanks for the review comments.
>
> On 1/21/19 10:03 PM, Simon Marchi wrote:
>>
>> Thanks for the patch. I don't know Fortran, so I can't assess whether the
>> behavior you implement is the right one. If other maintainers have this
>> knowledge, they are welcome to complement this review. Otherwise, I am
>> ready to trust you on that matter.
>
> My definition of correct for this patch has been gleaned from two sources:
>
> 1. The GNU Fortran argument passing conventions which can be found here:
>
> https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html
>
> I've attempted to capture as much of this page as possible in the test case.
>
> 2. Valid source code. A user should be able to paste their Fortran function
> call from their source code into GDB and receive the expected result.
>
>>
>> Here are a few more high level comments in the mean time.
>>
>
> I've addressed all style and code/comment repositioning issues for v2 of
> the patch.
Thanks!
>>
>>> +struct value *
>>> +fortran_argument_convert (struct value *value, const bool is_artificial)
>>> +{
>>> + if (!is_artificial)
>>> + {
>>> + if (VALUE_LVAL (value) == not_lval
>>> + || VALUE_LVAL (value) == lval_internalvar)
>>
>> Just wondering, have you considered all lval_types here? If you were to pass
>> a variable that is in a register (lval_register) or composed of multiple pieces
>> (lval_computed), I guess we would need to allocate them too. Not sure about the
>> other ones. In fact, everything that is not lval_memory would likely hit this
>> assert in value_addr:
>>
>> if (VALUE_LVAL (arg1) != lval_memory)
>> error (_("Attempt to take address of value not located in memory."));
>>
>> So maybe this should be
>>
>> if (VALUE_LVAL (value) != lval_memory)
>>
>> ?
>>
>
> Yes, this makes more sense. That error is indeed hit if a function call is made
> where one of the arguments is a register. v2 of this patch now works in this case.
>
> I am not able to see how the user would be able to express the other types of
> lvalue (e.g. computed) from the user interface. Do you have any pointers on
> this? As it would be useful to add this to the test case if it is indeed possible
> for the user to provoke this.
lval_computed is not something you can simply trigger from the command line, I believe.
It's used when the compiler optimizes and decides to place parts of a value in different
places. For example (maybe a bit exaggerated), for an 8 bytes value, it could put the
first 3 in memory, the next 4 in a register, and the last one is optimized out. The location
of this value will be described by DWARF "pieces", which is essentially a sequence of
DWARF opcodes describing sequentially where all the pieces are. The value in GDB with be
lval_computed. You can check section "2.6.1.2 Composite Location Descriptions" of DWARF5.pdf
if you want to know more.
It's a bit tricky to test, because you need to need to generate predictable DWARF pieces.
The best way is probably to write the DWARF by hand, as in testsuite/gdb.dwarf2/var-access.exp.
Maybe it would be a bit overkill to include such a test in your test case, I'll let you decide
if it's worth it.
lval_xcallable refers to XMethods:
https://sourceware.org/gdb/onlinedocs/gdb/Xmethods-In-Python.html#Xmethods-In-Python
For lval_internalvar_component, the comment says "Part of a gdb internal variable (structure field)",
so I assume you need to have a structure in a GDB internal variable (set $foo = ...) and pass a field
of $foo to the function.
>
>> Same (move comment to f-lang.h). Also, can you describe what ARG and TYPE are?
>> How are they related?
>>
>
>>
>> Could you file bugs for the various setup_kfail?
>>
>
> I've filed the bug report for the optional attribute here:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=24147
>
> However, I think the second bug report should wait until the point this
> patch passes review, as the bug only exists in GDB HEAD with this patch applied.
> I've included what the bug report would consist of below to explain why this
> bug exists.
Yes, that's fine.
> Fortran allows function parameters to be tagged with a "value" attribute which
> indicates that an argument is to be passed by value, rather than the default
> of by reference. For example:
>
> {noformat}
> integer(kind=4) function one_arg_value(x)
> integer(kind=4), value :: x
> one_arg_value = x
> end function
> {noformat}
>
> p one_arg_value(10)
> $19 = 6318016
>
> Garbage is returned when this function is called with a version of GDB which
> has this patch applied. Most likely this is the location of 10 in the inferior
> placed into a 4-byte integer, since GDB is passing a pointer to this value
> rather than the value. NOTE: This use case will work on GDB without this patch
> applied, as it flips the default calling convention to that outlined in
> https://gcc.gnu.org/onlinedocs/gfortran/Argument-passing-conventions.html. This
> way functions calls will mostly work out of the box rather than mostly not work.
>
> If this function call was working as expected it would return 10.
>
> This was tested with:
> * A build of 36c25ffa1ab5d6d5ee0fa3fc32f128a58e78e7a2 + the following patch
> from the mailing list https://sourceware.org/ml/gdb-patches/2019-01/msg00448.html
> * On Ubuntu 16.04.
> * On x86 64.
> * Fortran programs compiled with GCC 8.2.
Thanks,
Simon