[PATCH v3 0/5] Make seabios linkable with lld

[Fangrui Song]

This patch series make seabios linkable with lld.

This is beneficial for FreeBSD ports as well
https://svnweb.freebsd.org/ports/head/misc/seabios/Makefile
as they can drop an LLD_UNSAFE

As a maintainer of lld ELF, I have triaged numerous pieces of software.
seabios is the only one making use of 
This drops the only instance https://sourceware.org/bugzilla/show_bug.cgi?id=12726

1, 2, 3 and 4 are really independent and can be applied in arbitrary order.
5 depends on 4. I originally combined 4 and 5 and that was why I don't think a patch series made sense.

Fangrui Song (5):
  romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr
  Make rom16.o linkable with lld
  Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
  romlayout32flag.lds: Use `. +=` instead of `. =`
  test-build.sh: Delete unneeded LD capability test

Makefile              |  4 ++++
 scripts/layoutrom.py  | 13 ++++++++++---
 scripts/test-build.sh | 42 +-----------------------------------------
 src/romlayout.S       |  2 +-
 4 files changed, 16 insertions(+), 45 deletions(-)

[PATCH v3 1/5] romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr

[Fangrui Song]

It does not make sense to reference a SHF_ALLOC section from a
non-SHF_ALLOC section via R_386_PC16. Conceptually, even if a
non-SHF_ALLOC is loaded as part of the memory image, the distance
between it and a SHF_ALLOC section may not be a constant, so the linker
cannot reasonably resolve the relocation.

GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.

Signed-off-by: Fangrui Song 
---
 src/romlayout.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/romlayout.S b/src/romlayout.S
index c4a4635..a854783 100644
--- a/src/romlayout.S
+++ b/src/romlayout.S
@@ -587,7 +587,7 @@ entry_18:

// Specify a location in the fixed part of bios area.
         .macro ORG addr
-        .section .fixedaddr.\addr
+        .section .fixedaddr.\addr,"a"
         .endm

ORG 0xe05b

[PATCH v3 2/5] Make rom16.o linkable with lld

[Fangrui Song]

lld requires output section descriptions to be sorted by address.
Just sort the addresses beforehand.

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 6616721..caed387 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -321,6 +321,7 @@ def outXRefs(sections, useseg=0, exportsyms=[], forcedelta=0):

# Write LD script includes for the given sections
 def outSections(sections, useseg=0):
+    sections = sorted(sections, key=lambda x: x.finalloc)
     out = ""
     for section in sections:
         loc = section.finalloc

[PATCH v3 3/5] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

[Fangrui Song]

Accepting ET_EXEC as an input file is an extremely rare GNU ld feature
that lld does not intend to support, because this is error-prone.

See Linux kernel commit 90ceddcb495008ac8ba7a3dce297841efcd7d584 for
another use of the dd trick.

Signed-off-by: Fangrui Song 
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 5f7d537..118dec0 100644
--- a/Makefile
+++ b/Makefile
@@ -177,10 +177,14 @@ $(OUT)romlayout32seg.lds $(OUT)romlayout32flat.lds $(OUT)code32flat.o $(OUT)code
 $(OUT)rom16.o: $(OUT)code16.o $(OUT)romlayout16.lds
    @echo "  Linking $@"
    $(Q)$(LD) -T $(OUT)romlayout16.lds $< -o $@
+	# Change e_type to ET_REL so that it can be used to link rom.o.
+	# Unlike GNU ld, lld does not allow an ET_EXEC input.
+	printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none

$(OUT)rom32seg.o: $(OUT)code32seg.o $(OUT)romlayout32seg.lds
    @echo "  Linking $@"
    $(Q)$(LD) -T $(OUT)romlayout32seg.lds $< -o $@
+	printf '\1' | dd of=$@ conv=notrunc bs=1 seek=16 status=none

$(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o $(OUT)romlayout32flat.lds
    @echo "  Linking $@"

[PATCH v3 4/5] romlayout32flag.lds: Use `. +=` instead of `. =`

[Fangrui Song]

This improves the portability of the linker script and allows lld to link rom.o

Dot assignment inside an output section has an inconsistent behavior
which makes lld difficult to implement.
See https://bugs.llvm.org/show_bug.cgi?id=43083

Dropping `. =` turns out to be beneficial to older GNU ld as well
because we can delete an ld check detecting "cannot move location
counter backwards".

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index caed387..caa182a 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -331,19 +331,25 @@ def outSections(sections, useseg=0):
     return out

# Write LD script includes for the given sections using relative offsets
-def outRelSections(sections, startsym, useseg=0):
+def outRelSections(sections, useseg=0):
     sections = [(section.finalloc, section) for section in sections
                 if section.finalloc is not None]
     sections.sort(key=operator.itemgetter(0))
     out = ""
+    first = True
     for addr, section in sections:
         loc = section.finalloc
         if useseg:
             loc = section.finalsegloc
-        out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
+        if first:
+            out += ". += 0x%x - _reloc_init_end ;\n" % (loc,)
+            first = False
+        elif location != loc:
+            out += ". += 0x%x ;\n" % (loc-location,)
         if section.name in ('.rodata.str1.1', '.rodata'):
             out += "_rodata%s = . ;\n" % (section.fileid,)
         out += "*%s.*(%s)\n" % (section.fileid, section.name)
+        location = loc + section.size
     return out

# Build linker script output for a list of relocations.
@@ -444,7 +450,7 @@ def writeLinkerScripts(li, out16, out32seg, out32flat):
        sec32all_start,
        multiboot_header,
        relocstr,
-       outRelSections(li.sections, 'code32flat_start'))
+       outRelSections(li.sections))
     out = COMMONHEADER + out + COMMONTRAILER + """
 ENTRY(%s)
 PHDRS

[PATCH v3 5/5] test-build.sh: Delete unneeded LD capability test

[Fangrui Song]

The previous commit changed romlayout32flag.lds to use `. += ` instead
of `. =`.
We no longer need the LD capability test checking
https://sourceware.org/bugzilla/show_bug.cgi?id=12726

Signed-off-by: Fangrui Song 
---
 scripts/test-build.sh | 42 +-----------------------------------------
 1 file changed, 1 insertion(+), 41 deletions(-)

diff --git a/scripts/test-build.sh b/scripts/test-build.sh
index 25cc2f2..8b35d6f 100755
--- a/scripts/test-build.sh
+++ b/scripts/test-build.sh
@@ -4,50 +4,10 @@
 mkdir -p ${OUT}
 TMPFILE1=${OUT}/tmp_testcompile1.c
 TMPFILE1o=${OUT}/tmp_testcompile1.o
-TMPFILE1_ld=${OUT}/tmp_testcompile1.lds
 TMPFILE2=${OUT}/tmp_testcompile2.c
 TMPFILE2o=${OUT}/tmp_testcompile2.o
 TMPFILE3o=${OUT}/tmp_testcompile3.o

-# Test if ld's alignment handling is correct.  This is a known problem
-# with the linker that ships with Ubuntu 11.04.
-cat - > $TMPFILE1 <<EOF
-const char v1[] __attribute__((section(".text.v1"))) = "0123456789";
-const char v2[] __attribute__((section(".text.v2"))) = "0123456789";
-EOF
-cat - > $TMPFILE1_ld <<EOF
-SECTIONS
-{
-     .mysection 0x88f0 : {
-. = 0x10 ;
-*(.text.v1)
-. = 0x20 ;
-*(.text.v2)
-. = 0x30 ;
-     }
-}
-EOF
-$CC -O -g -c $TMPFILE1 -o $TMPFILE1o > /dev/null 2>&1
-if [ $? -ne 0 ]; then
-    echo "Unable to execute the C compiler ($CC)." >&2
-    echo "" >&2
-    echo "Please install a working compiler and retry." >&2
-    echo -1
-    exit 0
-fi
-$LD -T $TMPFILE1_ld $TMPFILE1o -o $TMPFILE2o > /dev/null 2>&1
-if [ $? -ne 0 ]; then
-    echo "The version of LD on this system ($LD) does not properly handle" >&2
-    echo "alignments.  As a result, this project can not be built." >&2
-    echo "" >&2
-    echo "The problem may be the result of this LD bug report:" >&2
-    echo " http://sourceware.org/bugzilla/show_bug.cgi?id=12726" >&2
-    echo "" >&2
-    echo "Please update to a working version of binutils and retry." >&2
-    echo -1
-    exit 0
-fi
-
 # Test for "-fwhole-program".  Older versions of gcc (pre v4.1) don't
 # support the whole-program optimization - detect that.
 $CC -fwhole-program -S -o /dev/null -xc /dev/null > /dev/null 2>&1
@@ -87,4 +47,4 @@ echo 0
 # "ebp" register is clobberred in an "asm" statement.  The code has
 # been modified to not clobber "ebp" - no test is available yet.

-rm -f $TMPFILE1 $TMPFILE1o $TMPFILE1_ld $TMPFILE2 $TMPFILE2o $TMPFILE3o
+rm -f $TMPFILE1 $TMPFILE1o $TMPFILE2 $TMPFILE2o $TMPFILE3o

[Fāng-ruì Sòng]

On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song  wrote:
> This patch series make seabios linkable with lld.
>
> This is beneficial for FreeBSD ports as well
> https://svnweb.freebsd.org/ports/head/misc/seabios/Makefile
> as they can drop an LLD_UNSAFE
>
> As a maintainer of lld ELF, I have triaged numerous pieces of software.
> seabios is the only one making use of
> This drops the only instance https://sourceware.org/bugzilla/show_bug.cgi?id=12726
>
> 1, 2, 3 and 4 are really independent and can be applied in arbitrary order.
> 5 depends on 4. I originally combined 4 and 5 and that was why I don't think a patch series made sense.
>
> Fangrui Song (5):
>   romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr
>   Make rom16.o linkable with lld
>   Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
>   romlayout32flag.lds: Use `. +=` instead of `. =`
>   test-build.sh: Delete unneeded LD capability test
>
> Makefile              |  4 ++++
>  scripts/layoutrom.py  | 13 ++++++++++---
>  scripts/test-build.sh | 42 +-----------------------------------------
>  src/romlayout.S       |  2 +-
>  4 files changed, 16 insertions(+), 45 deletions(-)

Ping...

And https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/CNOAJWO...
"[PATCH v2] Use readelf -WSrs instead of objdump -thr so that
llvm-readelf can be used as a replacement"

[Kevin O'Connor]

On Tue, Apr 14, 2020 at 12:07:54PM -0700, Fāng-ruì Sòng wrote:
> On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song  wrote:
> > This patch series make seabios linkable with lld.

[...]
> Ping...
>
> And https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/CNOAJWO...
> "[PATCH v2] Use readelf -WSrs instead of objdump -thr so that
> llvm-readelf can be used as a replacement"

Thanks.

As I understand the status:

- patch 3 is not something I'm comfortable merging

- the remaining patches look okay to me, but they need testing on a
  wide variety of distros to ensure there isn't a regression.  I'll do
  that when I can, but that wont be within the next 4-6 weeks.

-Kevin

[Fāng-ruì Sòng]

On 2020-04-16, Kevin O'Connor wrote:
> On Tue, Apr 14, 2020 at 12:07:54PM -0700, Fāng-ruì Sòng wrote:
> > On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song  wrote:
> > > This patch series make seabios linkable with lld.
>
> [...]
> > Ping...
> >
> > And https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/CNOAJWO...
> > "[PATCH v2] Use readelf -WSrs instead of objdump -thr so that
> > llvm-readelf can be used as a replacement"
>
> Thanks.
>
> As I understand the status:
> patch 3 is not something I'm comfortable merging

You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"

Let me add more descriptions. I am a maintainer of lld ELF (I
contributed most features and wrote most items of lld 9/10's release notes).

A linker accepts ET_REL as regular object files. The input sections
are taken to make up output sections. The input symbols are taken to
track dependencies and resolve relocations.

A linker accepts ET_DYN as shared objects. Input sections are ignored.
Only symbols are used. Many sections are synthesized by a linker and do
not make sense to be reused as input.

ET_EXEC, position dependent executables. The type is more similar to
ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
do not make sense to be reused as input. Sometimes there are needs to
read its symbols so that another component can know which symbols should
be exported. For this use case, --just-symbols is used.

seabios' use of ET_EXEC is very different. I have built thousands
third-party pieces of software. seabios and (for a short period because
I have fixed it) the Linux kernel are the only two cases an ET_EXEC
input's sections are used.  The symbol table appears to be ignored. This
is really an unusual use case. Another lld maintainer and I think this
is just a very error-prone case. I hence decide that this is not worth
supporting. GNU ld accepts it, but note that GNU gold disallows it as
well
https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f0592...

Since seabios just uses ET_EXEC as something similar to an ET_REL.
Binary patching the file seems the most portable approach.
(This Linux kernel patch has been merged into the mainline.)

I will stick around the IRC channel for some time. Feel free to ask me
to clarify more things if you still have questions.
> the remaining patches look okay to me, but they need testing on a
>
> wide variety of distros to ensure there isn't a regression.  I'll do
>  that when I can, but that wont be within the next 4-6 weeks.
>
> -Kevin

Thanks. I was hoping the patches could be accepted before I back ported
them to our internal systems.

I will now just start patching our internal code.
("[PATCH v2] Use readelf -WSrs instead of objdump -thr so that llvm-readelf can be used as a replacement")
I will report back if I manage to build/test things to add another data point.

[Gerd Hoffmann]

Hi,
> Since seabios just uses ET_EXEC as something similar to an ET_REL.
> Binary patching the file seems the most portable approach.

Wouldn't it be better to add a switch to ld to request ET_REL output
instead?

cheers,
  Gerd

[Fāng-ruì Sòng]

On 2020-04-21, Gerd Hoffmann wrote:
> Hi,

Hi Gerd, thank you for your comments.
> > You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > bios.bin.elf"
> >
> > Let me add more descriptions. I am a maintainer of lld ELF (I
> > contributed most features and wrote most items of lld 9/10's release
> > notes).
> >
> > A linker accepts ET_REL as regular object files. The input sections
> > are taken to make up output sections. The input symbols are taken to
> > track dependencies and resolve relocations.
> >
> > A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > Only symbols are used. Many sections are synthesized by a linker and do
> > not make sense to be reused as input.
> >
> > ET_EXEC, position dependent executables. The type is more similar to
> > ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > do not make sense to be reused as input. Sometimes there are needs to
> > read its symbols so that another component can know which symbols should
> > be exported. For this use case, --just-symbols is used.
> >
> > seabios' use of ET_EXEC is very different. I have built thousands
> > third-party pieces of software. seabios and (for a short period because
> > I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > input's sections are used.  The symbol table appears to be ignored. This
> > is really an unusual use case. Another lld maintainer and I think this
> > is just a very error-prone case. I hence decide that this is not worth
> > supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > well
> > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> >
> > Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > Binary patching the file seems the most portable approach.
> > (This Linux kernel patch has been merged into the mainline.)
>
> Wouldn't it be better to add a switch to ld to request ET_REL output
> instead?
>
> cheers,
>  Gerd

I added some context in my previous comment. Before I said "binary
patching", I analyzed possible linker input types
(ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
special case. Let me add a concrete example:

% cat a.c
   void _start() {}
   % clang -fuse-ld=bfd a.c -nostdlib -o a
   % clang -fuse-ld=bfd a -nostdlib -o aa
   /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
   /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created

An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
.dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
for a subsequent link. seabios is special in that it has a linker script
to explicitly concatenate some sections from the ET_EXEC input and (as I
understand it) just ignores all symbols.
This usage makes it really special and there is probably no other
project doing the same. A lld specific linker option will be neither
useful nor good for compatibility because GNU ld does not have the option.

For a new linker, we can take time to think what features are
error-prone and reject such features in advance to help detect such
problems. We are not in GNU ld's situation "our previous versions
support this arguably brittle use cases and our future versions may have
to be compatible".

Hope the explanation makes sense.

[Fangrui Song]

On 2020-04-21, Fāng-ruì Sòng wrote:
> On 2020-04-21, Gerd Hoffmann wrote:
> > Hi,
>
> Hi Gerd, thank you for your comments.
> > > You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > > bios.bin.elf"
> > >
> > > Let me add more descriptions. I am a maintainer of lld ELF (I
> > > contributed most features and wrote most items of lld 9/10's release
> > > notes).
> > >
> > > A linker accepts ET_REL as regular object files. The input sections
> > > are taken to make up output sections. The input symbols are taken to
> > > track dependencies and resolve relocations.
> > >
> > > A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > > Only symbols are used. Many sections are synthesized by a linker and do
> > > not make sense to be reused as input.
> > >
> > > ET_EXEC, position dependent executables. The type is more similar to
> > > ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > > do not make sense to be reused as input. Sometimes there are needs to
> > > read its symbols so that another component can know which symbols should
> > > be exported. For this use case, --just-symbols is used.
> > >
> > > seabios' use of ET_EXEC is very different. I have built thousands
> > > third-party pieces of software. seabios and (for a short period because
> > > I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > > input's sections are used.  The symbol table appears to be ignored. This
> > > is really an unusual use case. Another lld maintainer and I think this
> > > is just a very error-prone case. I hence decide that this is not worth
> > > supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > > well
> > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> > >
> > > Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > > Binary patching the file seems the most portable approach.
> > > (This Linux kernel patch has been merged into the mainline.)
> >
> > Wouldn't it be better to add a switch to ld to request ET_REL output
> > instead?
> >
> > cheers,
> > Gerd
>
> I added some context in my previous comment. Before I said "binary
> patching", I analyzed possible linker input types
> (ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
> special case. Let me add a concrete example:
>
> % cat a.c
>  void _start() {}
>  % clang -fuse-ld=bfd a.c -nostdlib -o a
>  % clang -fuse-ld=bfd a -nostdlib -o aa
>  /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
>  /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created
>
> An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
> .dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
> for a subsequent link. seabios is special in that it has a linker script
> to explicitly concatenate some sections from the ET_EXEC input and (as I
> understand it) just ignores all symbols.
> This usage makes it really special and there is probably no other
> project doing the same. A lld specific linker option will be neither
> useful nor good for compatibility because GNU ld does not have the option.
>
> For a new linker, we can take time to think what features are
> error-prone and reject such features in advance to help detect such
> problems. We are not in GNU ld's situation "our previous versions
> support this arguably brittle use cases and our future versions may have
> to be compatible".
>
> Hope the explanation makes sense.

Hi Kevin,

Internally we have been using seabios built with LLD and llvm-readelf
for two months now, without any issue. I hope the stability can give you
more confidence accepting the patches.

About PATCH 3/5 "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"
The maintainer of binutils agreed with my resolution (ET_EXEC should not be accepted as linker input)
and pushed
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a87e1817a435dab6c6...

GNU ld from binutils 2.35 onwards will not allow this use case.

Hope you can take another look when you get time.

[Fāng-ruì Sòng]

On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song  wrote:
> On 2020-04-21, Fāng-ruì Sòng wrote:
> > On 2020-04-21, Gerd Hoffmann wrote:
> > > Hi,
> >
> > Hi Gerd, thank you for your comments.
> > > > You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > > > bios.bin.elf"
> > > >
> > > > Let me add more descriptions. I am a maintainer of lld ELF (I
> > > > contributed most features and wrote most items of lld 9/10's release
> > > > notes).
> > > >
> > > > A linker accepts ET_REL as regular object files. The input sections
> > > > are taken to make up output sections. The input symbols are taken to
> > > > track dependencies and resolve relocations.
> > > >
> > > > A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > > > Only symbols are used. Many sections are synthesized by a linker and do
> > > > not make sense to be reused as input.
> > > >
> > > > ET_EXEC, position dependent executables. The type is more similar to
> > > > ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > > > do not make sense to be reused as input. Sometimes there are needs to
> > > > read its symbols so that another component can know which symbols should
> > > > be exported. For this use case, --just-symbols is used.
> > > >
> > > > seabios' use of ET_EXEC is very different. I have built thousands
> > > > third-party pieces of software. seabios and (for a short period because
> > > > I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > > > input's sections are used.  The symbol table appears to be ignored. This
> > > > is really an unusual use case. Another lld maintainer and I think this
> > > > is just a very error-prone case. I hence decide that this is not worth
> > > > supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > > > well
> > > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> > > >
> > > > Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > > > Binary patching the file seems the most portable approach.
> > > > (This Linux kernel patch has been merged into the mainline.)
> > >
> > > Wouldn't it be better to add a switch to ld to request ET_REL output
> > > instead?
> > >
> > > cheers,
> > > Gerd
> >
> > I added some context in my previous comment. Before I said "binary
> > patching", I analyzed possible linker input types
> > (ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
> > special case. Let me add a concrete example:
> >
> > % cat a.c
> >  void _start() {}
> >  % clang -fuse-ld=bfd a.c -nostdlib -o a
> >  % clang -fuse-ld=bfd a -nostdlib -o aa
> >  /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
> >  /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created
> >
> > An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
> > .dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
> > for a subsequent link. seabios is special in that it has a linker script
> > to explicitly concatenate some sections from the ET_EXEC input and (as I
> > understand it) just ignores all symbols.
> > This usage makes it really special and there is probably no other
> > project doing the same. A lld specific linker option will be neither
> > useful nor good for compatibility because GNU ld does not have the option.
> >
> > For a new linker, we can take time to think what features are
> > error-prone and reject such features in advance to help detect such
> > problems. We are not in GNU ld's situation "our previous versions
> > support this arguably brittle use cases and our future versions may have
> > to be compatible".
> >
> > Hope the explanation makes sense.
>
> Hi Kevin,
>
> Internally we have been using seabios built with LLD and llvm-readelf
> for two months now, without any issue. I hope the stability can give you
> more confidence accepting the patches.
>
> About PATCH 3/5 "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"
> The maintainer of binutils agreed with my resolution (ET_EXEC should not be accepted as linker input)
> and pushed
> https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a87e1817a435dab6c6...
>
> GNU ld from binutils 2.35 onwards will not allow this use case.
>
> Hope you can take another look when you get time.

Friendly ping.

ET_EXEC issue was fixed. Hope you can take a look at other patches. Thanks

[Fāng-ruì Sòng]

On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng  wrote:
> On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song  wrote:
> > On 2020-04-21, Fāng-ruì Sòng wrote:
> > > On 2020-04-21, Gerd Hoffmann wrote:
> > > > Hi,
> > >
> > > Hi Gerd, thank you for your comments.
> > > > > You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > > > > bios.bin.elf"
> > > > >
> > > > > Let me add more descriptions. I am a maintainer of lld ELF (I
> > > > > contributed most features and wrote most items of lld 9/10's release
> > > > > notes).
> > > > >
> > > > > A linker accepts ET_REL as regular object files. The input sections
> > > > > are taken to make up output sections. The input symbols are taken to
> > > > > track dependencies and resolve relocations.
> > > > >
> > > > > A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > > > > Only symbols are used. Many sections are synthesized by a linker and do
> > > > > not make sense to be reused as input.
> > > > >
> > > > > ET_EXEC, position dependent executables. The type is more similar to
> > > > > ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > > > > do not make sense to be reused as input. Sometimes there are needs to
> > > > > read its symbols so that another component can know which symbols should
> > > > > be exported. For this use case, --just-symbols is used.
> > > > >
> > > > > seabios' use of ET_EXEC is very different. I have built thousands
> > > > > third-party pieces of software. seabios and (for a short period because
> > > > > I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > > > > input's sections are used.  The symbol table appears to be ignored. This
> > > > > is really an unusual use case. Another lld maintainer and I think this
> > > > > is just a very error-prone case. I hence decide that this is not worth
> > > > > supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > > > > well
> > > > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> > > > >
> > > > > Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > > > > Binary patching the file seems the most portable approach.
> > > > > (This Linux kernel patch has been merged into the mainline.)
> > > >
> > > > Wouldn't it be better to add a switch to ld to request ET_REL output
> > > > instead?
> > > >
> > > > cheers,
> > > > Gerd
> > >
> > > I added some context in my previous comment. Before I said "binary
> > > patching", I analyzed possible linker input types
> > > (ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
> > > special case. Let me add a concrete example:
> > >
> > > % cat a.c
> > >  void _start() {}
> > >  % clang -fuse-ld=bfd a.c -nostdlib -o a
> > >  % clang -fuse-ld=bfd a -nostdlib -o aa
> > >  /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
> > >  /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created
> > >
> > > An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
> > > .dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
> > > for a subsequent link. seabios is special in that it has a linker script
> > > to explicitly concatenate some sections from the ET_EXEC input and (as I
> > > understand it) just ignores all symbols.
> > > This usage makes it really special and there is probably no other
> > > project doing the same. A lld specific linker option will be neither
> > > useful nor good for compatibility because GNU ld does not have the option.
> > >
> > > For a new linker, we can take time to think what features are
> > > error-prone and reject such features in advance to help detect such
> > > problems. We are not in GNU ld's situation "our previous versions
> > > support this arguably brittle use cases and our future versions may have
> > > to be compatible".
> > >
> > > Hope the explanation makes sense.
> >
> > Hi Kevin,
> >
> > Internally we have been using seabios built with LLD and llvm-readelf
> > for two months now, without any issue. I hope the stability can give you
> > more confidence accepting the patches.
> >
> > About PATCH 3/5 "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"
> > The maintainer of binutils agreed with my resolution (ET_EXEC should not be accepted as linker input)
> > and pushed
> > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a87e1817a435dab6c6...
> >
> > GNU ld from binutils 2.35 onwards will not allow this use case.
> >
> > Hope you can take another look when you get time.
>
> Friendly ping.
>
> ET_EXEC issue was fixed. Hope you can take a look at other patches. Thanks

Ping

[Fāng-ruì Sòng]

On Wed, Aug 19, 2020 at 7:43 PM Fāng-ruì Sòng  wrote:
> On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng  wrote:
> > On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song  wrote:
> > > On 2020-04-21, Fāng-ruì Sòng wrote:
> > > > On 2020-04-21, Gerd Hoffmann wrote:
> > > > > Hi,
> > > >
> > > > Hi Gerd, thank you for your comments.
> > > > > > You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > > > > > bios.bin.elf"
> > > > > >
> > > > > > Let me add more descriptions. I am a maintainer of lld ELF (I
> > > > > > contributed most features and wrote most items of lld 9/10's release
> > > > > > notes).
> > > > > >
> > > > > > A linker accepts ET_REL as regular object files. The input sections
> > > > > > are taken to make up output sections. The input symbols are taken to
> > > > > > track dependencies and resolve relocations.
> > > > > >
> > > > > > A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > > > > > Only symbols are used. Many sections are synthesized by a linker and do
> > > > > > not make sense to be reused as input.
> > > > > >
> > > > > > ET_EXEC, position dependent executables. The type is more similar to
> > > > > > ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > > > > > do not make sense to be reused as input. Sometimes there are needs to
> > > > > > read its symbols so that another component can know which symbols should
> > > > > > be exported. For this use case, --just-symbols is used.
> > > > > >
> > > > > > seabios' use of ET_EXEC is very different. I have built thousands
> > > > > > third-party pieces of software. seabios and (for a short period because
> > > > > > I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > > > > > input's sections are used.  The symbol table appears to be ignored. This
> > > > > > is really an unusual use case. Another lld maintainer and I think this
> > > > > > is just a very error-prone case. I hence decide that this is not worth
> > > > > > supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > > > > > well
> > > > > > https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> > > > > >
> > > > > > Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > > > > > Binary patching the file seems the most portable approach.
> > > > > > (This Linux kernel patch has been merged into the mainline.)
> > > > >
> > > > > Wouldn't it be better to add a switch to ld to request ET_REL output
> > > > > instead?
> > > > >
> > > > > cheers,
> > > > > Gerd
> > > >
> > > > I added some context in my previous comment. Before I said "binary
> > > > patching", I analyzed possible linker input types
> > > > (ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
> > > > special case. Let me add a concrete example:
> > > >
> > > > % cat a.c
> > > >  void _start() {}
> > > >  % clang -fuse-ld=bfd a.c -nostdlib -o a
> > > >  % clang -fuse-ld=bfd a -nostdlib -o aa
> > > >  /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
> > > >  /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created
> > > >
> > > > An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
> > > > .dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
> > > > for a subsequent link. seabios is special in that it has a linker script
> > > > to explicitly concatenate some sections from the ET_EXEC input and (as I
> > > > understand it) just ignores all symbols.
> > > > This usage makes it really special and there is probably no other
> > > > project doing the same. A lld specific linker option will be neither
> > > > useful nor good for compatibility because GNU ld does not have the option.
> > > >
> > > > For a new linker, we can take time to think what features are
> > > > error-prone and reject such features in advance to help detect such
> > > > problems. We are not in GNU ld's situation "our previous versions
> > > > support this arguably brittle use cases and our future versions may have
> > > > to be compatible".
> > > >
> > > > Hope the explanation makes sense.
> > >
> > > Hi Kevin,
> > >
> > > Internally we have been using seabios built with LLD and llvm-readelf
> > > for two months now, without any issue. I hope the stability can give you
> > > more confidence accepting the patches.
> > >
> > > About PATCH 3/5 "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"
> > > The maintainer of binutils agreed with my resolution (ET_EXEC should not be accepted as linker input)
> > > and pushed
> > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a87e1817a435dab6c6...
> > >
> > > GNU ld from binutils 2.35 onwards will not allow this use case.
> > >
> > > Hope you can take another look when you get time.
> >
> > Friendly ping.
> >
> > ET_EXEC issue was fixed. Hope you can take a look at other patches. Thanks
>
> Ping

Ping from 2021:)

[Fāng-ruì Sòng]

On Wed, Apr 21, 2021 at 6:48 PM Fāng-ruì Sòng  wrote:
> On Wed, Aug 19, 2020 at 7:43 PM Fāng-ruì Sòng  wrote:
> > On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng  wrote:
> > > On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song  wrote:
> > > > On 2020-04-21, Fāng-ruì Sòng wrote:
> > > > > On 2020-04-21, Gerd Hoffmann wrote:
> > > > > > Hi,
> > > > >
> > > > > Hi Gerd, thank you for your comments.
> > > > > > >You mean "Makefile: Change ET_EXEC to ET_REL so that lld can link
> > > > > > >bios.bin.elf"
> > > > > > >
> > > > > > >Let me add more descriptions. I am a maintainer of lld ELF (I
> > > > > > >contributed most features and wrote most items of lld 9/10's release
> > > > > > >notes).
> > > > > > >
> > > > > > >A linker accepts ET_REL as regular object files. The input sections
> > > > > > >are taken to make up output sections. The input symbols are taken to
> > > > > > >track dependencies and resolve relocations.
> > > > > > >
> > > > > > >A linker accepts ET_DYN as shared objects. Input sections are ignored.
> > > > > > >Only symbols are used. Many sections are synthesized by a linker and do
> > > > > > >not make sense to be reused as input.
> > > > > > >
> > > > > > >ET_EXEC, position dependent executables. The type is more similar to
> > > > > > >ET_DYN. Many synthesized sections (.hash .gnu.hash .plt .dynstr .dynsym)
> > > > > > >do not make sense to be reused as input. Sometimes there are needs to
> > > > > > >read its symbols so that another component can know which symbols should
> > > > > > >be exported. For this use case, --just-symbols is used.
> > > > > > >
> > > > > > >seabios' use of ET_EXEC is very different. I have built thousands
> > > > > > >third-party pieces of software. seabios and (for a short period because
> > > > > > >I have fixed it) the Linux kernel are the only two cases an ET_EXEC
> > > > > > >input's sections are used.  The symbol table appears to be ignored. This
> > > > > > >is really an unusual use case. Another lld maintainer and I think this
> > > > > > >is just a very error-prone case. I hence decide that this is not worth
> > > > > > >supporting. GNU ld accepts it, but note that GNU gold disallows it as
> > > > > > >well
> > > > > > >https://sourceware.org/git/?p=binutils-gdb.git;a=blob;hb=49d1d1f53df552f059%...
> > > > > > >
> > > > > > >Since seabios just uses ET_EXEC as something similar to an ET_REL.
> > > > > > >Binary patching the file seems the most portable approach.
> > > > > > >(This Linux kernel patch has been merged into the mainline.)
> > > > > >
> > > > > > Wouldn't it be better to add a switch to ld to request ET_REL output
> > > > > > instead?
> > > > > >
> > > > > > cheers,
> > > > > > Gerd
> > > > >
> > > > > I added some context in my previous comment. Before I said "binary
> > > > > patching", I analyzed possible linker input types
> > > > > (ET_REL/ET_DYN/ET_EXEC) and discussed why ET_EXEC as ET_REL is really a
> > > > > special case. Let me add a concrete example:
> > > > >
> > > > > % cat a.c
> > > > >  void _start() {}
> > > > >  % clang -fuse-ld=bfd a.c -nostdlib -o a
> > > > >  % clang -fuse-ld=bfd a -nostdlib -o aa
> > > > >  /usr/bin/ld.bfd: warning: cannot create .eh_frame_hdr section, --eh-frame-hdr ignored
> > > > >  /usr/bin/ld.bfd: error in a(.eh_frame); no .eh_frame_hdr table will be created
> > > > >
> > > > > An ET_EXEC can have many linker synthetized sections (.eh_frame_hdr,
> > > > > .dynamic, .gnu.hash, .hash, .plt, etc) which will really make no sense
> > > > > for a subsequent link. seabios is special in that it has a linker script
> > > > > to explicitly concatenate some sections from the ET_EXEC input and (as I
> > > > > understand it) just ignores all symbols.
> > > > > This usage makes it really special and there is probably no other
> > > > > project doing the same. A lld specific linker option will be neither
> > > > > useful nor good for compatibility because GNU ld does not have the option.
> > > > >
> > > > > For a new linker, we can take time to think what features are
> > > > > error-prone and reject such features in advance to help detect such
> > > > > problems. We are not in GNU ld's situation "our previous versions
> > > > > support this arguably brittle use cases and our future versions may have
> > > > > to be compatible".
> > > > >
> > > > > Hope the explanation makes sense.
> > > >
> > > > Hi Kevin,
> > > >
> > > > Internally we have been using seabios built with LLD and llvm-readelf
> > > > for two months now, without any issue. I hope the stability can give you
> > > > more confidence accepting the patches.
> > > >
> > > > About PATCH 3/5 "Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf"
> > > > The maintainer of binutils agreed with my resolution (ET_EXEC should not be accepted as linker input)
> > > > and pushed
> > > > https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=a87e1817a435dab6c6...
> > > >
> > > > GNU ld from binutils 2.35 onwards will not allow this use case.
> > > >
> > > > Hope you can take another look when you get time.
> > >
> > > Friendly ping.
> > >
> > > ET_EXEC issue was fixed. Hope you can take a look at other patches. Thanks
> >
> > Ping
>
> Ping from 2021:)

Well, you can probably understand I am quite burned out now...

Someone informed me that FreeBSD is still interested at seabios
migrating these stuff from GCC+GNU ld to llvm-project alternatives.
So let me ping another time...

(I am not subscribed.)

[Fāng-ruì Sòng]

On 2020-04-14, Fāng-ruì Sòng wrote:
> On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song  wrote:
> > This patch series make seabios linkable with lld.
> >
> > This is beneficial for FreeBSD ports as well
> > https://svnweb.freebsd.org/ports/head/misc/seabios/Makefile
> > as they can drop an LLD_UNSAFE
> >
> > As a maintainer of lld ELF, I have triaged numerous pieces of software.
> > seabios is the only one making use of
> > This drops the only instance https://sourceware.org/bugzilla/show_bug.cgi?id=12726
> >
> > 1, 2, 3 and 4 are really independent and can be applied in arbitrary order.
> > 5 depends on 4. I originally combined 4 and 5 and that was why I don't think a patch series made sense.
> >
> > Fangrui Song (5):
> >   romlayout.S: Add missing SHF_ALLOC flag to .fixedaddr.\addr
> >   Make rom16.o linkable with lld
> >   Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
> >   romlayout32flag.lds: Use `. +=` instead of `. =`
> >   test-build.sh: Delete unneeded LD capability test
> >
> > Makefile              |  4 ++++
> >  scripts/layoutrom.py  | 13 ++++++++++---
> >  scripts/test-build.sh | 42 +-----------------------------------------
> >  src/romlayout.S       |  2 +-
> >  4 files changed, 16 insertions(+), 45 deletions(-)
>
> Ping...
>
> And https://mail.coreboot.org/hyperkitty/list/[e-mail]/thread/CNOAJWO...
> "[PATCH v2] Use readelf -WSrs instead of objdump -thr so that
> llvm-readelf can be used as a replacement"

Ping^2

To add some confidence, we applied the llvm-readelf and LLD patches
internally and things have been working fine for a while.