[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Heinrich Schuchardt]

The sandbox does not build on RISC-V when _init is not defined. Errors like
the following were observed. Which function was referred to depended on the
configuration.

    /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
    /common/cli_hush.c:3175: undefined reference to `_init'
    collect2: error: ld returned 1 exit status
    make: *** [Makefile:1726: u-boot] Error 1

    /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
    common/autoboot.c:335: undefined reference to `_init'
    collect2: error: ld returned 1 exit status
    make: *** [Makefile:1726: u-boot] Error 1

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 arch/sandbox/cpu/u-boot.lds | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
index a1f509c9ab..d9e7ffcea3 100644
--- a/arch/sandbox/cpu/u-boot.lds
+++ b/arch/sandbox/cpu/u-boot.lds
@@ -9,6 +9,8 @@ SECTIONS
 {

 	. = ALIGN(4);
+	/* RISC-V GCC wants this dummy symbol */
+	_init = .;
 	.u_boot_list : {
 		KEEP(*(SORT(.u_boot_list*)));
 	}
--
2.30.2

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Bin Meng]

On Thu, May 13, 2021 at 10:46 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> The sandbox does not build on RISC-V when _init is not defined. Errors like
> the following were observed. Which function was referred to depended on the
> configuration.
>
>     /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
>     /common/cli_hush.c:3175: undefined reference to `_init'
>     collect2: error: ld returned 1 exit status
>     make: *** [Makefile:1726: u-boot] Error 1
>
>     /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
>     common/autoboot.c:335: undefined reference to `_init'
>     collect2: error: ld returned 1 exit status
>     make: *** [Makefile:1726: u-boot] Error 1
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/u-boot.lds | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
> index a1f509c9ab..d9e7ffcea3 100644
> --- a/arch/sandbox/cpu/u-boot.lds
> +++ b/arch/sandbox/cpu/u-boot.lds
> @@ -9,6 +9,8 @@ SECTIONS
>  {
>
>         . = ALIGN(4);
> +       /* RISC-V GCC wants this dummy symbol */

Native GCC on RISC-V?

RISC-V GCC sounds like cross-compile for RISC-V

> +       _init = .;
>         .u_boot_list : {
>                 KEEP(*(SORT(.u_boot_list*)));
>         }
> --


Regards,
Bin

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Simon Glass]

Hi Heinrich,

On Thu, 13 May 2021 at 08:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> The sandbox does not build on RISC-V when _init is not defined. Errors like
> the following were observed. Which function was referred to depended on the
> configuration.
>
>     /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
>     /common/cli_hush.c:3175: undefined reference to `_init'
>     collect2: error: ld returned 1 exit status
>     make: *** [Makefile:1726: u-boot] Error 1
>
>     /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
>     common/autoboot.c:335: undefined reference to `_init'
>     collect2: error: ld returned 1 exit status
>     make: *** [Makefile:1726: u-boot] Error 1
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  arch/sandbox/cpu/u-boot.lds | 2 ++
>  1 file changed, 2 insertions(+)

Is this a toolchain bug?

Reviewed-by: Simon Glass <sjg@chromium.org>


>
> diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
> index a1f509c9ab..d9e7ffcea3 100644
> --- a/arch/sandbox/cpu/u-boot.lds
> +++ b/arch/sandbox/cpu/u-boot.lds
> @@ -9,6 +9,8 @@ SECTIONS
>  {
>
>         . = ALIGN(4);
> +       /* RISC-V GCC wants this dummy symbol */
> +       _init = .;
>         .u_boot_list : {
>                 KEEP(*(SORT(.u_boot_list*)));
>         }
> --
> 2.30.2
>

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Bin Meng]

On Fri, May 14, 2021 at 7:56 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Heinrich,
>
> On Thu, 13 May 2021 at 08:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > The sandbox does not build on RISC-V when _init is not defined. Errors like
> > the following were observed. Which function was referred to depended on the
> > configuration.
> >
> >     /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
> >     /common/cli_hush.c:3175: undefined reference to `_init'
> >     collect2: error: ld returned 1 exit status
> >     make: *** [Makefile:1726: u-boot] Error 1
> >
> >     /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
> >     common/autoboot.c:335: undefined reference to `_init'
> >     collect2: error: ld returned 1 exit status
> >     make: *** [Makefile:1726: u-boot] Error 1
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > ---
> >  arch/sandbox/cpu/u-boot.lds | 2 ++
> >  1 file changed, 2 insertions(+)
>
> Is this a toolchain bug?

+Jim who might know.

>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
>
> >
> > diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
> > index a1f509c9ab..d9e7ffcea3 100644
> > --- a/arch/sandbox/cpu/u-boot.lds
> > +++ b/arch/sandbox/cpu/u-boot.lds
> > @@ -9,6 +9,8 @@ SECTIONS
> >  {
> >
> >         . = ALIGN(4);
> > +       /* RISC-V GCC wants this dummy symbol */
> > +       _init = .;
> >         .u_boot_list : {
> >                 KEEP(*(SORT(.u_boot_list*)));
> >         }

Regards,
Bin

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Heinrich Schuchardt]

On 5/14/21 1:00 AM, Bin Meng wrote:
> On Thu, May 13, 2021 at 10:46 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> The sandbox does not build on RISC-V when _init is not defined. Errors like
>> the following were observed. Which function was referred to depended on the
>> configuration.
>>
>>      /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
>>      /common/cli_hush.c:3175: undefined reference to `_init'
>>      collect2: error: ld returned 1 exit status
>>      make: *** [Makefile:1726: u-boot] Error 1
>>
>>      /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
>>      common/autoboot.c:335: undefined reference to `_init'
>>      collect2: error: ld returned 1 exit status
>>      make: *** [Makefile:1726: u-boot] Error 1
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   arch/sandbox/cpu/u-boot.lds | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
>> index a1f509c9ab..d9e7ffcea3 100644
>> --- a/arch/sandbox/cpu/u-boot.lds
>> +++ b/arch/sandbox/cpu/u-boot.lds
>> @@ -9,6 +9,8 @@ SECTIONS
>>   {
>>
>>          . = ALIGN(4);
>> +       /* RISC-V GCC wants this dummy symbol */
>
> Native GCC on RISC-V?

I was compiling natively on the BeagleV board.
The same result can be observed in a chroot using qemu-riscv64-static.

chroot: gcc (Debian 11-20210327-1) 11.0.1 20210327
BeagleV: gcc (GCC) 10.3.1 20210422 (Red Hat 10.3.1-1)

Best regards

Heinrich

>
> RISC-V GCC sounds like cross-compile for RISC-V
>
>> +       _init = .;
>>          .u_boot_list : {
>>                  KEEP(*(SORT(.u_boot_list*)));
>>          }
>> --
>
>
> Regards,
> Bin
>

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Heinrich Schuchardt]

On 5/14/21 1:56 AM, Simon Glass wrote:
> Hi Heinrich,
>
> On Thu, 13 May 2021 at 08:46, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> The sandbox does not build on RISC-V when _init is not defined. Errors like
>> the following were observed. Which function was referred to depended on the
>> configuration.
>>
>>      /usr/bin/ld: common/built-in.o: in function `parse_stream_outer':
>>      /common/cli_hush.c:3175: undefined reference to `_init'
>>      collect2: error: ld returned 1 exit status
>>      make: *** [Makefile:1726: u-boot] Error 1
>>
>>      /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
>>      common/autoboot.c:335: undefined reference to `_init'
>>      collect2: error: ld returned 1 exit status
>>      make: *** [Makefile:1726: u-boot] Error 1
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   arch/sandbox/cpu/u-boot.lds | 2 ++
>>   1 file changed, 2 insertions(+)
>
> Is this a toolchain bug?

Possibly. _init and _fini are obsolete in current GCC versions.

I saw this both with GCC 10 and GCC 11. So we should work around it.

I did not hit the problem when compiling a simple freestanding program
for RISC-V with GCC.

Best regards

Heinrich

>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
>
>>
>> diff --git a/arch/sandbox/cpu/u-boot.lds b/arch/sandbox/cpu/u-boot.lds
>> index a1f509c9ab..d9e7ffcea3 100644
>> --- a/arch/sandbox/cpu/u-boot.lds
>> +++ b/arch/sandbox/cpu/u-boot.lds
>> @@ -9,6 +9,8 @@ SECTIONS
>>   {
>>
>>          . = ALIGN(4);
>> +       /* RISC-V GCC wants this dummy symbol */
>> +       _init = .;
>>          .u_boot_list : {
>>                  KEEP(*(SORT(.u_boot_list*)));
>>          }
>> --
>> 2.30.2
>>

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Jim Wilson]

On Thu, May 13, 2021 at 5:55 PM Bin Meng <bmeng.cn@gmail.com> wrote:

> On Fri, May 14, 2021 at 7:56 AM Simon Glass <sjg@chromium.org> wrote:
> > On Thu, 13 May 2021 at 08:46, Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
> > >     /usr/bin/ld: common/built-in.o: in function `bootdelay_process':
> > >     common/autoboot.c:335: undefined reference to `_init'
> > >     collect2: error: ld returned 1 exit status
> > >     make: *** [Makefile:1726: u-boot] Error 1
>

In the ELF standard, .init sections were deprecated and replaced with
.init_array sections in the early 1990s I think.  It took some time for the
toolchains to fully migrate from init to init_array, but by 2010 or so
everyone was using init_array instead of init, with init support retained
only for legacy code that hadn't been fixed to use init_array yet.  Because
RISC-V is a new target with no legacy code to support, we made the decision
to drop support for the obsolete init section.  Last I checked there are
only two glibc targets that have no init section support, with the other
one also being a new arch, like RISC-V.  Same goes for the embedded target,
so the RISC-V newlib port has no init section support also.

It sounds like you have some legacy user code that hasn't been fixed yet to
use init_array instead of init.  Or maybe it is a program loader that
supports both?  In which case it should be extended to not use init on new
targets that no longer support it.

Init_array supports stack unwinding (aka C++ EH) and init doesn't, so
init_array should always be preferred over init, unless you have a very old
toolchain that lacks init_array support.  Dropping init support from the
RISC-V toolchain allows us to save some bytes of program code size, and
save some cycles on program startup, which is good considering that this is
a feature that we don't need anymore.

Jim

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Heinrich Schuchardt]

On 5/18/21 9:29 PM, Jim Wilson wrote:
> On Thu, May 13, 2021 at 5:55 PM Bin Meng <bmeng.cn@gmail.com
> <mailto:bmeng.cn@gmail.com>> wrote:
>
>     On Fri, May 14, 2021 at 7:56 AM Simon Glass <sjg@chromium.org
>     <mailto:sjg@chromium.org>> wrote:
>      > On Thu, 13 May 2021 at 08:46, Heinrich Schuchardt
>     <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> wrote:
>      > >? ? ?/usr/bin/ld: common/built-in.o: in function
>     `bootdelay_process':
>      > >? ? ?common/autoboot.c:335: undefined reference to `_init'
>      > >? ? ?collect2: error: ld returned 1 exit status
>      > >? ? ?make: *** [Makefile:1726: u-boot] Error 1
>
>
> In the ELF standard, .init sections were deprecated and replaced with
> .init_array sections in the early 1990s I think.? It took some time for
> the toolchains to fully migrate from init to init_array, but by 2010 or
> so everyone was using init_array instead of init, with init support
> retained only for legacy code that hadn't been fixed to use init_array
> yet.? Because RISC-V is a new target with no legacy code to support, we
> made the decision to drop support for the obsolete init section.? Last I
> checked there are only two glibc targets that have no init section
> support, with the other one also being a new arch, like RISC-V.? Same
> goes for the embedded target, so the RISC-V newlib port has no init
> section support also.
>
> It sounds like you have some legacy user code that hasn't been fixed yet
> to use init_array instead of init.? Or maybe it is a program loader that
> supports both?? In which case it should be extended to not use init on
> new targets that no longer support it.

I am compiling the sandbox_defconfig target of upstream U-Boot with GCC
11. Please, indicate what "legacy user code" you are referring to.

Best regards

Heinrich

>
> Init_array supports stack unwinding (aka C++ EH) and init doesn't, so
> init_array should always be preferred over init, unless you have a very
> old toolchain that lacks init_array support.? Dropping init support from
> the RISC-V toolchain allows us to save some bytes of program code size,
> and save some cycles on program startup, which is good considering that
> this is a feature that we don't need anymore.
>
> Jim
>

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Jim Wilson]

On Tue, May 18, 2021 at 1:58 PM Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> I am compiling the sandbox_defconfig target of upstream U-Boot with GCC
> 11. Please, indicate what "legacy user code" you are referring to.
>

I have no idea what that is.  I'm not a uboot developer.

Jim

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Jim Wilson]

On Tue, May 18, 2021 at 2:32 PM Jim Wilson <jimw@sifive.com> wrote:

> On Tue, May 18, 2021 at 1:58 PM Heinrich Schuchardt <xypron.glpk@gmx.de>
> wrote:
>
>> I am compiling the sandbox_defconfig target of upstream U-Boot with GCC
>> 11. Please, indicate what "legacy user code" you are referring to.
>>
>
> I have no idea what that is.  I'm not a uboot developer.
>

By "user code" I mean that it isn't toolchain code, as the toolchain
doesn't use init.

Jim

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Bin Meng]

Hi Jim,

On Wed, May 19, 2021 at 5:36 AM Jim Wilson <jimw@sifive.com> wrote:
>
> On Tue, May 18, 2021 at 2:32 PM Jim Wilson <jimw@sifive.com> wrote:
>>
>> On Tue, May 18, 2021 at 1:58 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>
>>> I am compiling the sandbox_defconfig target of upstream U-Boot with GCC
>>> 11. Please, indicate what "legacy user code" you are referring to.
>>
>>
>> I have no idea what that is.  I'm not a uboot developer.
>
>
> By "user code" I mean that it isn't toolchain code, as the toolchain doesn't use init.

Heinrich was using a GCC natively (not cross-compile) on a RISC-V
board to build a "user code" and unfortunately it did not build.

Do you think this might be related to how GCC was configured for native RISC-V?

Regards,
Bin

[PATCH 1/1] sandbox: add symbol _init for RISC-V compilation

[Heinrich Schuchardt]

On 5/18/21 11:36 PM, Jim Wilson wrote:
> On Tue, May 18, 2021 at 2:32 PM Jim Wilson <jimw@sifive.com
> <mailto:jimw@sifive.com>> wrote:
>
>     On Tue, May 18, 2021 at 1:58 PM Heinrich Schuchardt
>     <xypron.glpk at gmx.de <mailto:xypron.glpk@gmx.de>> wrote:
>
>         I am compiling the sandbox_defconfig target of upstream U-Boot
>         with GCC
>         11. Please, indicate what "legacy user code" you are referring to.
>
>
>     I have no idea what that is.? I'm not a uboot developer.
>
>
> By "user code" I mean that it isn't toolchain code, as the toolchain
> doesn't use init.
>
> Jim

This seems to be the culprit:

common/board_f.c:269:
#elif defined(CONFIG_SANDBOX) || defined(CONFIG_EFI_APP)
 ????????gd->mon_len = (ulong)&_end - (ulong)_init;

gd->mon_len is used here:

common/board_f.c:344
gd->ram_top = board_get_usable_ram_top(gd->mon_len);

@Simon:
We don't relocate the sandbox code into the mmap'ed memory.
Why should we use a nonzero value of gd->mon_len on the sandbox?

When compiling the sandbox on x86_64 u-boot.map has:

.init           0x0000000000031000       0x17
  *(SORT_NONE(.init))
  .init          0x0000000000031000       0x12
/usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/crti.o
                 0x0000000000031000                _init
  .init          0x0000000000031012        0x5
/usr/lib/gcc/x86_64-linux-gnu/11/../../../x86_64-linux-gnu/crtn.o

This relates to
https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/i386/crti.S

This file defines symbols _init and _fini.

For aarch64 we have:

https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/aarch64/crti.S
This file contains symbols _init and _fini, too.

For RISC-V we have:

https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/riscv/crti.S

It contains only a comment
/* crti.S is empty because .init_array/.fini_array are used exclusively. */

Symbols _init and _fini don't exist on RISC-V.

As the usage of _init is deprecated in GCC we should not use it in any
U-Boot configuration, neither for SANDBOX nor for EFI_APP.

Best regards

Heinrich