alire-project / GNAT-FSF-builds

Builds of the GNAT Ada compiler from FSF GCC releases
MIT License
32 stars 9 forks source link

Interfaces.C.Strings tries to call in libc’s strlen #45

Closed simonjwright closed 1 year ago

simonjwright commented 1 year ago

Building this:

with Interfaces.C.Strings;
procedure Kev is
    Cp : Interfaces.C.Strings.chars_ptr
     := Interfaces.C.Strings.New_String ("foo");
   Ca : Interfaces.C.char_array
     := Interfaces.C.Strings.Value (Cp);
begin
   Str := Interfaces.C.Strings.New_String ("foo");
end Kev;

with

   for Target use "arm-eabi";
   for Runtime ("ada") use "light-cortex-m4f";

results in

/Users/simon/.config/alire/cache/dependencies/gnat_arm_elf_12.2.1_f4bfd822/bin/../lib/gcc/arm-eabi/12.2.0/../../../../arm-eabi/bin/ld: /users/simon/.config/alire/cache/dependencies/gnat_arm_elf_12.2.1_f4bfd822/arm-eabi/lib/gnat/light-cortex-m4f/adalib/libgnat.a(i-cstrin.o): in function `interfaces__c__strings__value':
i-cstrin.adb:(.text.interfaces__c__strings__value+0xe): undefined reference to `strlen'

On investigation, if optimising at -O2, when the compiler sees this

      Result : char_array (0 .. Strlen (Item));

where you would have expected the Strlen to be the one at line 201 what actually gets called is just strlen which is of course provided as part of libc.

This is fascinating: it seems as though the compiler recognises that the code generated in the local function Strlen is the same as ... what __builtin_strlen would have produced? and generates the call to strlen??

In fact, at -O2, i-cstrin.o references memcpy, memset, and strlen, whereas at lower levels it only references memcpy.

Other objects reference memcpy and memset, but that’s not an issue because the RTS provides them (s-memcop, s-memset).

Could this be fixed by providing s-strlen in the RTS?

Fabien-Chouteau commented 1 year ago

The correct answer here is to link with libc which makes sens when there's C code in the project anyway.

In this repo we are not looking to change the content of the run-times.

kevlar700 commented 1 year ago

There isn't any C code in my project when this error occurs. Yet Gnat studio shows Interfaces.C.Strings.Value available for use within the last chance handler for the light runtime. Is there a reason that the last chance handler can't provide an Ada string in the first place?

simonjwright commented 1 year ago

The correct answer here is to link with libc which makes sens when there's C code in the project anyway.

I didn’t put the C code in the project, no one did! GCC decided to implement Ada source by calling a function in libc.

I already said

   package Linker is
      for Default_Switches ("ada") use
        ("-T", project'Project_Dir & "src/link.ld",
         "-Wl,-gc-sections",
         "-lc");
   end Linker;

... which doesn’t work, because light-cortex-m4f/runtime.xml says

      for Required_Switches use Linker'Required_Switches &
        ("-Wl,-L${RUNTIME_DIR(Ada)}/adalib",
         "-nostartfiles", "-nolibc",
         "-L${RUNTIME_DIR(ada)}/ld_user") &
         Compiler.Common_Required_Switches;

and the -nolibc means that my -lc is ignored.

simonjwright commented 1 year ago

In this repo we are not looking to change the content of the run-times.

OK, but who then can we report the problem to? because s-memset & friends were introduced to avoid having to drag in libc. They were known to the developers because the Ada compiler explicitly translates array initialization and copying into calls on those primitives. Here we have a case where it seems that much lower levels of the compiler are translating Ada code into calls on strlen in libc. GCC 11.2 appears not to do this translation.

Fabien-Chouteau commented 1 year ago

https://github.com/AdaCore/bb-runtime would be a better place to discuss this.

Looking again at the code you provided @simonjwright, the Strlen defined at line 201 should indeed be used in this case. So this looks like it is more about disabling a GCC "optimization" or renaming this function, than adding a new s-strlen package.

simonjwright commented 1 year ago

See https://github.com/AdaCore/bb-runtimes/issues/64.

It was thought worth while to add s-memset, s-memcop -- maybe for high-integrity runtimes?