[PATCH 10/31] sandbox: Provide a linker script for MSYS2

Pali Rohár pali at kernel.org
Tue Apr 25 21:33:12 CEST 2023


On Tuesday 25 April 2023 13:23:04 Simon Glass wrote:
> Hi Pali,
> 
> On Tue, 25 Apr 2023 at 12:11, Pali Rohár <pali at kernel.org> wrote:
> >
> > On Tuesday 25 April 2023 12:01:04 Simon Glass wrote:
> > > Hi Pali,
> > >
> > > On Tue, 25 Apr 2023 at 10:21, Pali Rohár <pali at kernel.org> wrote:
> > > >
> > > > On Monday 24 April 2023 17:08:15 Simon Glass wrote:
> > > > > Add a script to allow the U-Boot sandbox executable to be built for
> > > > > Windows. Add a note as to why this seems to be necessary for now.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > ---
> > > > >
> > > > >  Makefile                       |  11 +-
> > > > >  arch/sandbox/config.mk         |   4 +
> > > > >  arch/sandbox/cpu/u-boot-pe.lds | 447 +++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 460 insertions(+), 2 deletions(-)
> > > > >  create mode 100644 arch/sandbox/cpu/u-boot-pe.lds
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index dd3fcd1782e5..0aa97a2c3b48 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -1730,6 +1730,13 @@ else
> > > > >  u-boot-keep-syms-lto :=
> > > > >  endif
> > > > >
> > > > > +ifeq ($(MSYS_VERSION),0)
> > > > > +add_ld_script := -T u-boot.lds
> > > > > +else
> > > > > +add_ld_script := u-boot.lds
> > > > > +$(warning msys)
> > > > > +endif
> > > > > +
> > > > >  # Rule to link u-boot
> > > > >  # May be overridden by arch/$(ARCH)/config.mk
> > > > >  ifeq ($(LTO_ENABLE),y)
> > > > > @@ -1738,7 +1745,7 @@ quiet_cmd_u-boot__ ?= LTO     $@
> > > > >               $(CC) -nostdlib -nostartfiles                                   \
> > > > >               $(LTO_FINAL_LDFLAGS) $(c_flags)                                 \
> > > > >               $(KBUILD_LDFLAGS:%=-Wl,%) $(LDFLAGS_u-boot:%=-Wl,%) -o $@       \
> > > > > -             -T u-boot.lds $(u-boot-init)                                    \
> > > > > +             $(add_ld_script) $(u-boot-init)                                 \
> > > > >               -Wl,--whole-archive                                             \
> > > > >                       $(u-boot-main)                                          \
> > > > >                       $(u-boot-keep-syms-lto)                                 \
> > > > > @@ -1749,7 +1756,7 @@ quiet_cmd_u-boot__ ?= LTO     $@
> > > > >  else
> > > > >  quiet_cmd_u-boot__ ?= LD      $@
> > > > >        cmd_u-boot__ ?= $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_u-boot) -o $@                \
> > > > > -             -T u-boot.lds $(u-boot-init)                                    \
> > > > > +             $(add_ld_script) $(u-boot-init)                                 \
> > > > >               --whole-archive                                                 \
> > > > >                       $(u-boot-main)                                          \
> > > > >               --no-whole-archive                                              \
> > > > > diff --git a/arch/sandbox/config.mk b/arch/sandbox/config.mk
> > > > > index 2d184c5f652a..e05daf57ef8e 100644
> > > > > --- a/arch/sandbox/config.mk
> > > > > +++ b/arch/sandbox/config.mk
> > > > > @@ -71,3 +71,7 @@ EFI_CRT0 := crt0_sandbox_efi.o
> > > > >  EFI_RELOC := reloc_sandbox_efi.o
> > > > >  AFLAGS_crt0_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > >  CFLAGS_reloc_sandbox_efi.o += -DHOST_ARCH="$(HOST_ARCH)"
> > > > > +
> > > > > +ifneq ($(MSYS_VERSION),0)
> > > > > +LDSCRIPT = $(srctree)/arch/sandbox/cpu/u-boot-pe.lds
> > > > > +endif
> > > > > diff --git a/arch/sandbox/cpu/u-boot-pe.lds b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > new file mode 100644
> > > > > index 000000000000..031e70fafd03
> > > > > --- /dev/null
> > > > > +++ b/arch/sandbox/cpu/u-boot-pe.lds
> > > > > @@ -0,0 +1,447 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0+ */
> > > > > +/*
> > > > > + * U-Boot note: This was obtained by using the -verbose linker option. The
> > > > > + * U-Boot additions are marked below.
> > > > > + *
> > > > > + * Ideally we would add sections to the executable, as is done with the Linux
> > > > > + * build. But PE executables do not appear to work correctly if unexpected
> > > > > + * sections are present:
> > > > > + *
> > > > > + *   $ /tmp/b/sandbox/u-boot.exe
> > > > > + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> > > > > + *
> > > > > + * So we take a approach of rewriting the whole file, for now. This will likely
> > > > > + * break in the future when a toolchain change is made.
> > > >
> > > > Why not rather provide "layer" linker script which does this "rewriting"
> > > > on top of the default linker script? With this way it is not needed to
> > > > update linker script when a toolchain change it.
> > > >
> > >
> > > How can we reliably do that, though? We don't really know the format
> > > of the script and where to insert stuff.
> > >
> > > Regards,
> > > Simon
> >
> > Well, I do not know what is the current issue. The proposed script in
> 
> See the comment at the top of the script:
> 
> + * Ideally we would add sections to the executable, as is done with the Linux
> + * build. But PE executables do not appear to work correctly if unexpected
> + * sections are present:
> + *
> + *   $ /tmp/b/sandbox/u-boot.exe
> + *   -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file:
> Exec format error
> + *
> + * So we take a approach of rewriting the whole file, for now. This will likely
> + * break in the future when a toolchain change is made.
> 
> > your patch looks like it is copied from some binutils version and then
> > modified.
> 
> Yes, exactly.
> 
> > Also I do not know from which binutils you have copied and
> > what modification you have done in it.
> 
> But I have marked this clearly in the script. See  /* U-Boot additions
> from here on */

I read this, but I have not found from which binutils you took it...
There are many released binutils versions and also each binutils
version has more PE linker scripts. So it is really hard to track from
which it comes. -verbose argument show you the final linker script but
we need to know also from which files it was composed...

> > So I cannot react or comment
> > anymore more here.
> >
> > My idea is that to write those modifications into new layer script.
> 
> So I think you are suggesting that I get binutils to emit the default
> script, then have a script which detects the end of the .rdata section
> and emits its own stuff in there? That may be more robust, since it is
> unlikely that the .rdata section will be removed.

Yes, exactly. And you do not need to take and copy default script from
binutils to u-boot sources. If you write "layer" linker script (I hope
that this is how it is called in LD documentation), then LD
automatically uses its own default script and apply your "layer" script
on it. So with this way you can completely avoid copy+paste files from
binutils to u-boot repository.

> But I was rather hoping that someone could help with the core problem,
> i.e. why I cannot just insert another section into the image, like we
> do with ELF?

That is interesting. Theoretically it should work but I have not seen it
widely used outside of NT kernel modules. So maybe there is some bug in
Windows loader for userspace binaries, or another bug in GNU LD, or just
a bug in u-boot / linker script.

Could you show linker script which is causing generation of that buggy
non-working binary? And ideally can you provide also that buggy EXE
binary? I could try to inspect it, but I do not have time to setup MSYS2
environment to compile my own buggy EXE binary.

> 
> Regards,
> Simon


More information about the U-Boot mailing list