Making the same mistake as the broken C interface to fortran

Making the same mistake as the broken C interface to fortran

[Chris M Moore]

Hi,

Read this interesting article today:

https://lwn.net/SubscriberLink/791393/41d57555202e8cdb/

Synopsis: C interfaces to Fortran makes some assumptions about how to 
call fortran ABIs (I don't need to pass the hidden length parameter if 
its a character*1) but now Gfortran has optimisations which assume a 
different calling convention (Thou shalt pass the hidden length).

There are work arounds (compile fortran with 
‑fno‑optimize‑sibling‑calls) but it seems that the proper fix is to pass 
the hidden length parameter.

I had a quick look at the LAPACK bindings and they both seem to use Ada 
characters. :/

-- 
sig pending (since 1995)

Re: Making the same mistake as the broken C interface to fortran

[Simon Wright]

Chris M Moore <zmower@ntlworld.com> writes:

> Read this interesting article today:
>
> https://lwn.net/SubscriberLink/791393/41d57555202e8cdb/
>
> Synopsis: C interfaces to Fortran makes some assumptions about how to
> call fortran ABIs (I don't need to pass the hidden length parameter if
> its a character*1) but now Gfortran has optimisations which assume a
> different calling convention (Thou shalt pass the hidden length).
>
> There are work arounds (compile fortran with
> ‑fno‑optimize‑sibling‑calls) but it seems that the proper fix is to
> pass the hidden length parameter.
>
> I had a quick look at the LAPACK bindings and they both seem to use
> Ada characters. :/

The code generated with Convention=Fortran should abide by the ABI, and
if that says to pass a hidden length parameter then that's what should
happen.

I don't know enough x86_64 (or thumb) assembler to be at all sure, but
it looks to me as though no length parameter gets passed.

Re: Making the same mistake as the broken C interface to fortran

[Chris M Moore]

On 02/07/2019 21:57, Simon Wright wrote:
> Chris M Moore <zmower@ntlworld.com> writes:
> 
>> Read this interesting article today:
>>
>> https://lwn.net/SubscriberLink/791393/41d57555202e8cdb/
>>
>> Synopsis: C interfaces to Fortran makes some assumptions about how to
>> call fortran ABIs (I don't need to pass the hidden length parameter if
>> its a character*1) but now Gfortran has optimisations which assume a
>> different calling convention (Thou shalt pass the hidden length).
>>
>> There are work arounds (compile fortran with
>> ‑fno‑optimize‑sibling‑calls) but it seems that the proper fix is to
>> pass the hidden length parameter.
>>
>> I had a quick look at the LAPACK bindings and they both seem to use
>> Ada characters. :/
> 
> The code generated with Convention=Fortran should abide by the ABI, and
> if that says to pass a hidden length parameter then that's what should
> happen.
> 
> I don't know enough x86_64 (or thumb) assembler to be at all sure, but
> it looks to me as though no length parameter gets passed.
> 

Hi Simon,

So the question is do we break the API or not?  We could make the 
bindings thicker and thus slower.  Or break the typing and force client 
software to change.  The software engineer in me says the former but I'm 
not a user of this API so I doubt my views count for much.

-- 
sig pending (since 1995)

Re: Making the same mistake as the broken C interface to fortran

[Randy Brukardt]

"Chris M Moore" <zmower@ntlworld.com> wrote in message 
news:2uYSE.298383$sJ3.119314@fx04.am4...
...
> So the question is do we break the API or not?

That's not the question. The question is ensuring that the Ada compiler 
properly implements the Fortran convention. If it does, then the API doesn't 
change. And if it doesn't, then fix the silly compiler (or get a different 
one that does the right thing). The Annex B "Implementation Advice" 
(specifically, B.5(22-26)) is very close to a requirement, in that the 
interface is not useful if one can't depend on the mapping.

If your worrying about what hack to use to make a broken compiler work 
(hopefully only because you need an immediate solution), the answer ought to 
be that no one cares -- do whatever makes sense. Because no one should be 
*requiring* an Ada compiler to do the wrong thing, so when the bug is 
ultimately fixed, you'll have to revert to some version of the original API. 
(That to me suggests using a thicker binding -- which is always preferable 
anyway IMHO -- raw C or Fortran interfaces don't make good Ada libraries.)

BTW, I note that type Fortran_Character is in fact an array type, so that 
could be confusing some readers of the API (it surely would have confused 
me).

                                                     Randy.












We could make the
> bindings thicker and thus slower.  Or break the typing and force client 
> software to change.  The software engineer in me says the former but I'm 
> not a user of this API so I doubt my views count for much.
>
> -- 
> sig pending (since 1995) 

Re: Making the same mistake as the broken C interface to fortran

[Chris M Moore]

On 03/07/2019 20:02, Randy Brukardt wrote:
> "Chris M Moore" <zmower@ntlworld.com> wrote in message
> news:2uYSE.298383$sJ3.119314@fx04.am4...
> ...
>> So the question is do we break the API or not?
> 
> That's not the question. The question is ensuring that the Ada compiler
> properly implements the Fortran convention. If it does, then the API doesn't
> change. And if it doesn't, then fix the silly compiler (or get a different
> one that does the right thing). The Annex B "Implementation Advice"
> (specifically, B.5(22-26)) is very close to a requirement, in that the
> interface is not useful if one can't depend on the mapping.
> 
<snip>
> 
> BTW, I note that type Fortran_Character is in fact an array type, so that
> could be confusing some readers of the API (it surely would have confused
> me).

I'm sure GNAT does the right thing if you're using Fortran_Character. 
Unfortunately the bindings use Standard.Character.

>> We could make the
>> bindings thicker and thus slower.  Or break the typing and force client
>> software to change.  The software engineer in me says the former but I'm
>> not a user of this API so I doubt my views count for much.

Or maybe the answer is both.  Leave in the Character interface with a 
body to do the conversion and add another version to do the thin wrapper 
using Fortran_Character.  Nothing breaks but if speed becomes a problem 
then switch to using the proper thin version.

Chris

-- 
sig pending (since 1995)

Re: Making the same mistake as the broken C interface to fortran

[Simon Wright]

Chris M Moore <zmower@ntlworld.com> writes:

> On 03/07/2019 20:02, Randy Brukardt wrote:
>> "Chris M Moore" <zmower@ntlworld.com> wrote in message
>> news:2uYSE.298383$sJ3.119314@fx04.am4...
>> ...
>>> So the question is do we break the API or not?
>>
>> That's not the question. The question is ensuring that the Ada
>> compiler properly implements the Fortran convention. If it does, then
>> the API doesn't change. And if it doesn't, then fix the silly
>> compiler (or get a different one that does the right thing). The
>> Annex B "Implementation Advice" (specifically, B.5(22-26)) is very
>> close to a requirement, in that the interface is not useful if one
>> can't depend on the mapping.
>>
> <snip>
>>
>> BTW, I note that type Fortran_Character is in fact an array type, so
>> that could be confusing some readers of the API (it surely would have
>> confused me).

It confused the heck out of me, too. You'd've expected a warning at
least if you use a dodgy parameter type!

I did a little poking around with GCC 9.1.0:

Fortran:

      subroutine callee (c)
      character (1), intent (in) :: c

      print *, 'parameter c is ', c

      end

Ada:

with Interfaces.Fortran;
procedure Call is
   procedure Callee_C (C : Character)
   with
     Import,
     Convention => Fortran,
     External_Name => "callee_";
   procedure Callee_F (C : Interfaces.Fortran.Fortran_Character)
   with
     Import,
     Convention => Fortran,
     External_Name => "callee_";
   procedure Callee_S (S : String)
   with
     Import,
     Convention => Fortran,
     External_Name => "callee_";
begin
   Callee_C ('c');
   Callee_F ((1 => 'f'));
   Callee_F ("F string");
   Callee_S ("A string");
end Call;

Result:

$ gfortran -c callee.f
$ gnatmake -gnatwa call.adb -largs callee.o -lgfortran
gcc -c -gnatwa call.adb
gnatbind -x call.ali
gnatlink call.ali callee.o -lgfortran
$ ./call
 parameter c is c
 parameter c is f
 parameter c is F
 parameter c is A
 
> I'm sure GNAT does the right thing if you're using
> Fortran_Character. Unfortunately the bindings use Standard.Character.

Which bindings are those? I can certainly look at changing the ones in
gnat-math-extn (Should be https://sf.net/p/gnat-math-extn, but down at
the moment), but those are entirely internal.

====================

For what it's worth, the issue is that a long-standing feature of the
Fortran ABI is that 'character' is an array type, so that the fat
parameter contains (1) the address of the first element and (2) the
length of the actual (I'm not sure whether these two parts are
necessarily contiguous). If the Fortran only ever accesses the first
element, it doesn't matter if the length part is never actually passed
on the stack, which is what happens when Ada or C passes Character or
char rsp.

However, with a new optimisation, if the Fortran compiler recognises an
opportunity for "tail recursion" (or a sibling call), it reuses
the current stack arguments for the sibling call, including the length
component of the character argument. If that length component was never
actually pushed by the original caller, oops, trashed stack.

Re: Making the same mistake as the broken C interface to fortran

[Chris M Moore]

On 04/07/2019 09:38, Simon Wright wrote:
> Chris M Moore <zmower@ntlworld.com> writes:
> 
>> On 03/07/2019 20:02, Randy Brukardt wrote:
>>> "Chris M Moore" <zmower@ntlworld.com> wrote in message
>>> news:2uYSE.298383$sJ3.119314@fx04.am4...
>>> ...
>>>> So the question is do we break the API or not?
>>>
>>> That's not the question. The question is ensuring that the Ada
>>> compiler properly implements the Fortran convention. If it does, then
>>> the API doesn't change. And if it doesn't, then fix the silly
>>> compiler (or get a different one that does the right thing). The
>>> Annex B "Implementation Advice" (specifically, B.5(22-26)) is very
>>> close to a requirement, in that the interface is not useful if one
>>> can't depend on the mapping.
>>>
>> <snip>
>>>
>>> BTW, I note that type Fortran_Character is in fact an array type, so
>>> that could be confusing some readers of the API (it surely would have
>>> confused me).
> 
> It confused the heck out of me, too. You'd've expected a warning at
> least if you use a dodgy parameter type!

Agreed.

> I did a little poking around with GCC 9.1.0:
> 
> Fortran:
> 
>        subroutine callee (c)
>        character (1), intent (in) :: c
> 
>        print *, 'parameter c is ', c
> 
>        end

I'm having flashbacks.  Send help. ;)

> Ada:
> 
> with Interfaces.Fortran;
> procedure Call is
>     procedure Callee_C (C : Character)
>     with
>       Import,
>       Convention => Fortran,
>       External_Name => "callee_";
>     procedure Callee_F (C : Interfaces.Fortran.Fortran_Character)
>     with
>       Import,
>       Convention => Fortran,
>       External_Name => "callee_";
>     procedure Callee_S (S : String)
>     with
>       Import,
>       Convention => Fortran,
>       External_Name => "callee_";
> begin
>     Callee_C ('c');

I've looked at the assembler for this.  Passing 'c' results in only a 
byte pushed to the stack.  And then (aha!) the address on the stack is 
placed in the rdi register.

>     Callee_F ((1 => 'f'));
>     Callee_F ("F string");
>     Callee_S ("A string");

These pass the address of a fixed string (in the ro text area) in the 
rdi register.

> end Call;
> 
> Result:
> 
> $ gfortran -c callee.f
> $ gnatmake -gnatwa call.adb -largs callee.o -lgfortran
> gcc -c -gnatwa call.adb
> gnatbind -x call.ali
> gnatlink call.ali callee.o -lgfortran
> $ ./call
>   parameter c is >   parameter c is f
>   parameter c is F
>   parameter c is A

So it works at the moment for the small example we have.

>> I'm sure GNAT does the right thing if you're using
>> Fortran_Character. Unfortunately the bindings use Standard.Character.
> 
> Which bindings are those?

I was looking at the LAPACK ones on sourceforge.

> I can certainly look at changing the ones in
> gnat-math-extn (Should be https://sf.net/p/gnat-math-extn, but down at
> the moment), but those are entirely internal.

I'd wait until we have conclusive proof we have a problem.  I'll do some 
more digging.

<snip>

-- 
sig pending (since 1995)

Re: Making the same mistake as the broken C interface to fortran

[Simon Wright]

Chris M Moore <zmower@ntlworld.com> writes:

> On 04/07/2019 09:38, Simon Wright wrote:
>> procedure Call is
>>     procedure Callee_C (C : Character)
>>     with
>>       Import,
>>       Convention => Fortran,
>>       External_Name => "callee_";
>>     procedure Callee_F (C : Interfaces.Fortran.Fortran_Character)
>>     with
>>       Import,
>>       Convention => Fortran,
>>       External_Name => "callee_";
>>     procedure Callee_S (S : String)
>>     with
>>       Import,
>>       Convention => Fortran,
>>       External_Name => "callee_";
>> begin
>>     Callee_C ('c');
>
> I've looked at the assembler for this.  Passing 'c' results in only a
> byte pushed to the stack.  And then (aha!) the address on the stack is
> placed in the rdi register.

Because all Fortran parameters are passed by reference.

Re: Making the same mistake as the broken C interface to fortran

[Chris M Moore]

On 05/07/2019 18:44, Simon Wright wrote:
> Chris M Moore <zmower@ntlworld.com> writes:
> 
>> On 04/07/2019 09:38, Simon Wright wrote:
>>> procedure Call is
>>>      procedure Callee_C (C : Character)
>>>      with
>>>        Import,
>>>        Convention => Fortran,
>>>        External_Name => "callee_";
>>>      procedure Callee_F (C : Interfaces.Fortran.Fortran_Character)
>>>      with
>>>        Import,
>>>        Convention => Fortran,
>>>        External_Name => "callee_";
>>>      procedure Callee_S (S : String)
>>>      with
>>>        Import,
>>>        Convention => Fortran,
>>>        External_Name => "callee_";
>>> begin
>>>      Callee_C ('c');
>>
>> I've looked at the assembler for this.  Passing 'c' results in only a
>> byte pushed to the stack.  And then (aha!) the address on the stack is
>> placed in the rdi register.
> 
> Because all Fortran parameters are passed by reference.


I spoke too soon when I said

 > I'm sure GNAT does the right thing if you're using Fortran_Character.

If I change callee.f to

       subroutine callee (c)
       character (len=*), intent (in) :: c

       print *, 'parameter c is ', c

       end

then STORAGE_ERROR is the order of the day no matter the call used. 
Looking at the assembler, this is because GNAT does not pass the length 
of the string.

I compared it to fcall.f:

       program fcall
       call callee("OK")
       call callee("Oh noes")
       stop
       end

and this unsurprisingly does pass the lengths.

I've used the webform on the Community section of the GNAT website to 
provide feedback.  I've pointed out that the issue also affects single 
chacter parameters.

Chris

-- 
sig pending (since 1995)