[PATCH v3 0/5] Make seabios linkable with lld
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(-) -- 2.26.0.292.g33ef6b2f38-goog
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 <maskray@google.com> --- 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 -- 2.26.0.292.g33ef6b2f38-goog
lld requires output section descriptions to be sorted by address. Just sort the addresses beforehand. -- Changes v2 -> v3 * Sort sections by finalloc unconditionally Signed-off-by: Fangrui Song <maskray@google.com> --- 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 -- 2.26.0.292.g33ef6b2f38-goog
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. -- Changes v1 -> v2 * Add status=none to the dd command line. This suppresses dd's stderr output. * Move dd command to a separate command Signed-off-by: Fangrui Song <maskray@google.com> --- 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 $@" -- 2.26.0.292.g33ef6b2f38-goog
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". -- Changes v2 -> v3 * Add `first` to avoid overloading `location` * Delete startsym from the parameters Signed-off-by: Fangrui Song <maskray@google.com> --- 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 -- 2.26.0.292.g33ef6b2f38-goog
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 <maskray@google.com> --- 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 -- 2.26.0.292.g33ef6b2f38-goog
On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song <maskray@google.com> 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(-)
-- 2.26.0.292.g33ef6b2f38-goog
Ping... And https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/CNOAJWO... "[PATCH v2] Use readelf -WSrs instead of objdump -thr so that llvm-readelf can be used as a replacement"
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 <maskray@google.com> wrote:
This patch series make seabios linkable with lld.
[...] Ping...
And https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/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
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 <maskray@google.com> wrote:
This patch series make seabios linkable with lld.
[...] Ping...
And https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/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.
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
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.
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.
On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song <maskray@google.com> 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
On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song <maskray@google.com> 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
On Wed, Aug 19, 2020 at 7:43 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song <maskray@google.com> 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:)
On Wed, Apr 21, 2021 at 6:48 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Wed, Aug 19, 2020 at 7:43 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Tue, Jul 28, 2020 at 6:02 PM Fāng-ruì Sòng <maskray@google.com> wrote:
On Thu, Jun 25, 2020 at 2:05 PM Fangrui Song <maskray@google.com> 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.)
On 2020-04-14, Fāng-ruì Sòng wrote:
On Mon, Apr 6, 2020 at 5:55 PM Fangrui Song <maskray@google.com> 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(-)
-- 2.26.0.292.g33ef6b2f38-goog
Ping...
And https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/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.
participants (4)
-
Fangrui Song -
Fāng-ruì Sòng
-
Gerd Hoffmann -
Kevin O'Connor