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

Pali Rohár pali at kernel.org
Wed Apr 26 09:22:39 CEST 2023


On Wednesday 26 April 2023 09:13:55 Pali Rohár wrote:
> On Tuesday 25 April 2023 18:50:59 Simon Glass wrote:
> > Hi Pali,
> > 
> > On Tue, 25 Apr 2023 at 13:33, Pali Rohár <pali at kernel.org> wrote:
> > >
> > > 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...
> > 
> > $ ld -V
> > GNU ld (GNU Binutils) 2.40
> >   Supported emulations:
> >    i386pep
> >    i386pe
> > 
> > sglass at DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ cc -v
> > Using built-in specs.
> > COLLECT_GCC=cc
> > COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-pc-msys/11.3.0/lto-wrapper.exe
> > Target: x86_64-pc-msys
> > Configured with: /c/S/gcc/src/gcc-11.3.0/configure
> > --build=x86_64-pc-msys --prefix=/usr --libexecdir=/usr/lib
> > --enable-bootstrap --enable-shared --enable-shared-libgcc
> > --enable-static --enable-version-specific-runtime-libs
> > --with-arch=x86-64 --with-tune=generic --disable-multilib
> > --enable-__cxa_atexit --with-dwarf2
> > --enable-languages=c,c++,fortran,lto --enable-graphite
> > --enable-threads=posix --enable-libatomic --enable-libgomp
> > --disable-libitm --enable-libquadmath --enable-libquadmath-support
> > --disable-libssp --disable-win32-registry --disable-symvers
> > --with-gnu-ld --with-gnu-as --disable-isl-version-check
> > --enable-checking=release --without-libiconv-prefix
> > --without-libintl-prefix --with-system-zlib --enable-linker-build-id
> > --with-default-libstdcxx-abi=gcc4-compatible
> > --enable-libstdcxx-filesystem-ts
> > Thread model: posix
> > Supported LTO compression algorithms: zlib
> > 
> > 
> > >
> > > > > 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.
> > 
> > That is considerably more complicated, though.
> > 
> > >
> > > > 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.
> > 
> > Please see [1] for the file.
> > 
> > sglass at DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ objdump.exe -h /tmp/b/sandbox/u-boot.exe
> > 
> > /tmp/b/sandbox/u-boot.exe:     file format pei-x86-64
> > 
> > Sections:
> > Idx Name          Size      VMA               LMA               File off  Algn
> >   0 .text         00208b98  0000000100401000  0000000100401000  00000600  2**4
> >                   CONTENTS, ALLOC, LOAD, READONLY, CODE, DATA
> >   1 __u_boot_list 00014960  0000000100609ba0  0000000100609ba0  00209200  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   2 _u_boot_sandbox_getopt 000000c0  000000010061e500
> > 000000010061e500  0021dc00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   3 .data         00019a20  000000010061f000  000000010061f000  0021de00  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   4 .rodata.ttf.init 00036e10  0000000100639000  0000000100639000
> > 00237a00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   5 .rodata.splash.init 00001b20  0000000100670000  0000000100670000
> > 0026ea00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   6 .rodata.helloworld.init 00003030  0000000100672000
> > 0000000100672000  00270600  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   7 .data.efi_runtime 00000170  0000000100676000  0000000100676000
> > 00273800  2**4
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   8 .dtb.init.rodata 000007c0  0000000100677000  0000000100677000
> > 00273a00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >   9 .rdata        000a3fe0  0000000100678000  0000000100678000  00274200  2**4
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  10 .buildid      00000035  000000010071c000  000000010071c000  00318200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  11 .rodata.efi_runtime 00000400  000000010071d000  000000010071d000
> > 00318400  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  12 .pdata        0001785c  000000010071e000  000000010071e000  00318800  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  13 .xdata        00016f58  0000000100736000  0000000100736000  00330200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  14 .bss          0001ecc0  000000010074d000  000000010074d000  00000000  2**4
> >                   ALLOC
> >  15 .idata        00000894  000000010076c000  000000010076c000  00347200  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >  16 .rsrc         000004e8  000000010076d000  000000010076d000  00347c00  2**2
> >                   CONTENTS, ALLOC, LOAD, DATA
> >  17 .reloc        00005284  000000010076e000  000000010076e000  00348200  2**2
> >                   CONTENTS, ALLOC, LOAD, READONLY, DATA
> >  18 .debug_aranges 0000b720  0000000100774000  0000000100774000  0034d600  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  19 .debug_info   00a6c954  0000000100780000  0000000100780000  00358e00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  20 .debug_abbrev 000cb6b2  00000001011ed000  00000001011ed000  00dc5800  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  21 .debug_line   001f646a  00000001012b9000  00000001012b9000  00e91000  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  22 .debug_frame  000a9bd0  00000001014b0000  00000001014b0000  01087600  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  23 .debug_str    0001d6c1  000000010155a000  000000010155a000  01131200  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  24 .debug_loc    004673a1  0000000101578000  0000000101578000  0114ea00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  25 .debug_ranges 00060260  00000001019e0000  00000001019e0000  015b5e00  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> >  26 .debug_line_str 00000264  0000000101a41000  0000000101a41000  01616200  2**0
> >                   CONTENTS, READONLY, DEBUGGING
> > 
> > sglass at DESKTOP-OHNGJ4K MINGW64 ~/u-boot
> > $ !$
> > /tmp/b/sandbox/u-boot.exe
> > -bash: /tmp/b/sandbox/u-boot.exe: cannot execute binary file: Exec format error
> 
> I have run readpe parser on this binary and parser crashed! Have not
> seen such thing it. So I suspect that this binary is broken. I will look
> at it deeply later. Another thing which I see suspicious in above output
> are section names. They are too long and cannot fit into PE format which
> has (small) limit on section names. Not sure if this can be an issue...

Ok, this binary is really broken / corrupted. objdump -p also show lot
of errors "xdata section corrupt". Maybe this is just an another bug in
LD? I have found them a lot and some of them I reported to their
bugzilla. If you are trying to produce PE binaries it would be better to
switch to MS tools they always had better quality for production
software. GNU tools are here good just for playing or research.

> > 
> > Regards,
> > Simon
> > 
> > [1] https://drive.google.com/file/d/1vtJlvCd1BxkhRtQtUe4YqiRizcUP_zB2/view?usp=share_link


More information about the U-Boot mailing list