This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Fortran function calls with arguments


Thanks for the review Tom.

On 2/13/19 9:24 PM, Tom Tromey wrote:
>>>>>> "Richard" == Richard Bunt <Richard.Bunt@arm.com> writes:
> 
> Richard> It is possible to call Fortran functions, but memory must be allocated in
> Richard> the inferior, so a pointer can be passed to the function, and the
> Richard> language must be set to C to enable C-style casting. This is cumbersome
> Richard> and not a pleasant debug experience.
> 
> Baking in the compiler ABI is unfortunate but I suppose nothing gdb
> hasn't done before; and anyway I don't think there's a way to express
> this "properly".

It is a shame that the use of DW_AT_calling_convention isn't more wide spread, then
GDB could simply follow the DWARF.

> 
> Richard> Since the GNU Fortran argument passing convention has been implemented,
> Richard> there is no guarantee that this patch will work correctly, in all cases,
> Richard> with other compilers.
> 
> Do you have any information at all on other compilers, or even whether
> older versions of GNU Fortran differ?

A few ABI differences are listed here:
https://www.fortran90.org/src/faq.html#are-fortran-compilers-abi-compatible

I believe the "logical" and the "calling convention" remarks are the most
relevant. As the testcase in this patch covers all the calling scenarios it
can (given the available DWARF), we can do better and collect empirical evidence on
differences.

I've run the test case against GNU 4.8.5, 7.3.0, 8.1 and 8.2 on x86_64 and no changes
in behaviour were detected. Since 4.8.5 and 8.2 have been tested, we can have confidence
that the ABI hasn't changed significantly in this time.

As for Fortran compilers from other vendors, I tested with 3 others. To note, I needed to
modify the testcase to: 

1. Exclude the use of ubound as this intrinsic was not available in all compilers.
2. Exclude the call which takes a subroutine as an argument as this crashed with other
compilers. I did not take the time to investigate why here.

With the test results I confirmed that this patch at least improves the situation for
compilers from other vendors.

gfortran
# of expected passes            8
# of unexpected failures        17
# of unknown successes          1
# of known failures             1
=>
# of expected passes            25
# of known failures             2

Compiler 1
# of expected passes            5
# of unexpected failures        20
# of known failures             2
=>
# of expected passes            18
# of unexpected failures        7
# of unknown successes          1
# of known failures             1

Compiler 2
# of expected passes            5
# of unexpected failures        20
# of unknown successes          1
# of known failures             1
=>
# of expected passes            16
# of unexpected failures        9
# of known failures             2 

Compiler 3
# of expected passes		4
# of unexpected failures	21
# of known failures		2
=>
# of expected passes		4
# of unexpected failures	21
# of known failures		2
No change in passing tests but the number of segmentation faults is reduced from 11 to 1 and many failures stem from output such as:
p sum_some(1,2,3)
$12 = (PTR TO -> ( integer )) 0x6
(gdb) FAIL: gdb.fortran/function-calls.exp: p sum_some(1,2,3)
where the expected result is stored in a pointer.

Some examples of ABI differences causing test failures:
1. Different representations of true and false.
p no_arg()
$1 = 4294967295

2. Differing return convention for complex numbers.
p complex_argument(fft)
Too few arguments in function call.
(gdb) FAIL: gdb.fortran/function-calls.exp: p complex_argument(fft)

> 
> Richard> +	      /* Arguments in Fortran are passed by address.  Coerce the
> Richard> +		 arguments here rather than in value_arg_coerce as otherwise
> Richard> +		 the call to malloc to place the non-lvalue parameters in
> Richard> +		 target memory is hit by this Fortran specific logic.  This
> Richard> +		 results in malloc being called with a pointer to an integer
> Richard> +		 followed by an attempt to malloc the arguments to malloc in
> Richard> +		 target memory.  Infinite recursion ensues.  */
> 
> Thanks for writing this comment.
> 
> Richard> +fortran_argument_convert (struct value *value, const bool is_artificial)
> 
> I think the "const" here and in the header can be removed.
> It doesn't add much here, and doesn't add anything in the header.
> 

I'll remove this.

> The patch is ok with that change.  Thanks for doing this.
> 
> Tom
>

Many thanks,

Rich

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]