[PATCH 0/4] 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 and 3 are really independent and can be applied in arbitrary order.
4 depends on 3. I originally combined 3 and 4 and that was why I don't think a patch series made sense.

Fangrui Song (4):
  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  | 11 ++++++++++-
 scripts/test-build.sh | 42 +-----------------------------------------
 src/romlayout.S       |  2 +-
 4 files changed, 16 insertions(+), 43 deletions(-)

[PATCH 1/4] Make rom16.o linkable with lld

[Fangrui Song]

(1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC section via R_386_PC16.
GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.

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

Signed-off-by: Fangrui Song 
---
 scripts/layoutrom.py | 4 ++++
 src/romlayout.S      | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

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

# Write LD script includes for the given sections
 def outSections(sections, useseg=0):
+    if useseg:
+        sections.sort(key=lambda x: x.finalsegloc)
+    else:
+        sections.sort(key=lambda x: x.finalloc)
     out = ""
     for section in sections:
         loc = section.finalloc
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 2/4] 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 3/4] 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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
index 4c55390..94d4412 100755
--- a/scripts/layoutrom.py
+++ b/scripts/layoutrom.py
@@ -339,14 +339,19 @@ def outRelSections(sections, startsym, useseg=0):
                 if section.finalloc is not None]
     sections.sort(key=operator.itemgetter(0))
     out = ""
+    location = "_reloc_init_end"
     for addr, section in sections:
         loc = section.finalloc
         if useseg:
             loc = section.finalsegloc
-        out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
+        if location == "_reloc_init_end":
+            out += ". += 0x%x - %s ;\n" % (loc, location)
+        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.

[PATCH 4/4] 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

[PATCH 1/4] Make rom16.o linkable with lld

[Kevin O'Connor]

On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:
> (1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
> It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC section via R_386_PC16.
> GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.
>
> (2) lld requires output section descriptions to be sorted by address.
>  Just sort the addresses beforehand.

This looks like it should be two separate patches.

I know it's a pain to redo patches, but separating out changes helps
greatly when tracking down regressions via "git bisect".
> Signed-off-by: Fangrui Song
>
> scripts/layoutrom.py | 4 ++++
>  src/romlayout.S      | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> index 6616721..4c55390 100755
> --- a/scripts/layoutrom.py
> +++ b/scripts/layoutrom.py
> @@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], forcedelta=0):
>
> # Write LD script includes for the given sections
>  def outSections(sections, useseg=0):
> if useseg:
>    sections.sort(key=lambda x: x.finalsegloc)
> else:
>    sections.sort(key=lambda x: x.finalloc)

This looks odd to me - there shouldn't be a need to change the sort
order based on useseg, as finalloc should always have the same order
as finalsegloc.  Also, this code alters the input list which is
confusing - perhaps use "sections = sorted(sections, key=...)".
>  out = ""
>  for section in sections:
>      loc = section.finalloc
>
> 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 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

[Kevin O'Connor]

On Wed, Apr 01, 2020 at 10:29:13AM -0700, Fangrui Song wrote:
> 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.
> 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
>
> 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

My high-level feedback is that the above is very fragile.  I'd be
reluctant to adopt that hack.  What is the underlying issue that needs
to be addressed?

-Kevin
> $(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o $(OUT)romlayout32flat.lds
>    @echo "  Linking $@"
> -- 
> 2.26.0.rc2.310.g2932bb562d-goog
> _______________________________________________
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

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

[Kevin O'Connor]

On Wed, Apr 01, 2020 at 10:29:14AM -0700, Fangrui Song wrote:
> 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 | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> index 4c55390..94d4412 100755
> --- a/scripts/layoutrom.py
> +++ b/scripts/layoutrom.py
> @@ -339,14 +339,19 @@ def outRelSections(sections, startsym, useseg=0):
>                  if section.finalloc is not None]
>      sections.sort(key=operator.itemgetter(0))
>      out = ""
> location = "_reloc_init_end"
> for addr, section in sections:
>     loc = section.finalloc
>     if useseg:
>         loc = section.finalsegloc
>    out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
>    if location == "_reloc_init_end":
>        out += ". += 0x%x - %s ;\n" % (loc, location)
>    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

I'm finding this code confusing.  I would recommend "location" always
be a string or always be an integer - mixing them makes the code hard
to follow.  Also, this code removes the only reference to startsym,
but doesn't update the function signature.  Similarly, it hardcodes
"_reloc_init_end" when I think that should be passed to the function.

-Kevin
>  return out
>
> # Build linker script output for a list of relocations.
>
> 2.26.0.rc2.310.g2932bb562d-goog
> _______________________________________________
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

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

[Kevin O'Connor]

On Wed, Apr 01, 2020 at 10:29:15AM -0700, Fangrui Song wrote:
> 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

Thanks.  It would be great to remove this test.  Were you able to
bring up that old version of Ubuntu and verify that it produces
bootable code (or definitely errors) with the new code though?  I'd
like to make sure the new code doesn't result in silently broken code.

-Kevin
> 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
>
> 2.26.0.rc2.310.g2932bb562d-goog
> _______________________________________________
> SeaBIOS mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf

[Fangrui Song]

On 2020-04-06, Kevin O'Connor wrote:
> On Wed, Apr 01, 2020 at 10:29:13AM -0700, Fangrui Song wrote:
> > 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.
> > 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
> >
> > 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
>
> My high-level feedback is that the above is very fragile.  I'd be
> reluctant to adopt that hack.  What is the underlying issue that needs
> to be addressed?
>
> -Kevin

lld does not take ET_EXEC as input. This is a deliberate choice.
GNU gold does not accept ET_EXEC as well:

% gold a
  gold: error: a: unsupported ELF file type 2

1 ET_REL represents object files (.o)
2 ET_EXEC represents position-dependent executables.
3 ET_DYN represents shared objects (.so) (or PIE; for linking purposes, PIE cannot be accepted)
4 ET_CORE represents core files. For linking purposes, they cannot be accepted.

I don't know how GNU ld ends up accepting ET_EXEC. I am not even sure it
is an intentional decision. A lot of sections will not be meaningful
to the linker and accidentally mixing an ET_EXEC can likely lead to
hard-to-debug linking issues.

I made a similar change to Linux recently.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i...
> > $(OUT)rom.o: $(OUT)rom16.strip.o $(OUT)rom32seg.strip.o $(OUT)code32flat.o $(OUT)romlayout32flat.lds
> >    @echo "  Linking $@"
> > --
> > 2.26.0.rc2.310.g2932bb562d-goog
> > _______________________________________________
> > SeaBIOS mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]

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

[Fangrui Song]

On 2020-04-06, Kevin O'Connor wrote:
> On Wed, Apr 01, 2020 at 10:29:15AM -0700, Fangrui Song wrote:
> > 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
>
> Thanks.  It would be great to remove this test.  Were you able to
> bring up that old version of Ubuntu and verify that it produces
> bootable code (or definitely errors) with the new code though?  I'd
> like to make sure the new code doesn't result in silently broken code.
>
> -Kevin

I cannot test it easily. Can you do that for me, please?
> > 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
> >
> > 2.26.0.rc2.310.g2932bb562d-goog
> > _______________________________________________
> > SeaBIOS mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]

[PATCH 1/4] Make rom16.o linkable with lld

[Fangrui Song]

On 2020-04-06, Kevin O'Connor wrote:
> On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:
> > (1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
> > It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC section via R_386_PC16.
> > GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.
> >
> > (2) lld requires output section descriptions to be sorted by address.
> >  Just sort the addresses beforehand.
>
> This looks like it should be two separate patches.
>
> I know it's a pain to redo patches, but separating out changes helps
> greatly when tracking down regressions via "git bisect".

I would hope [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
can be applied first it it is acceptable.

Like I said, these 1~3 have no dependency at all.
Redoing a patch series and resending as a whole may mess up the list.
> > Signed-off-by: Fangrui Song
> >
> > scripts/layoutrom.py | 4 ++++
> >  src/romlayout.S      | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > index 6616721..4c55390 100755
> > --- a/scripts/layoutrom.py
> > +++ b/scripts/layoutrom.py
> > @@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], forcedelta=0):
> >
> > # Write LD script includes for the given sections
> >  def outSections(sections, useseg=0):
> > if useseg:
> >    sections.sort(key=lambda x: x.finalsegloc)
> > else:
> >    sections.sort(key=lambda x: x.finalloc)
>
> This looks odd to me - there shouldn't be a need to change the sort
> order based on useseg, as finalloc should always have the same order
> as finalsegloc.  Also, this code alters the input list which is
> confusing - perhaps use "sections = sorted(sections, key=...)".

Just to confirm, I should use:

if not useseg:
     sections = sorted(sections, key=lambda x: x.finalloc)
> >  out = ""
> >  for section in sections:
> >      loc = section.finalloc
> >
> > 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 1/4] Make rom16.o linkable with lld

[Kevin O'Connor]

On Mon, Apr 06, 2020 at 05:26:35PM -0700, Fangrui Song wrote:
> On 2020-04-06, Kevin O'Connor wrote:
> > On Wed, Apr 01, 2020 at 10:29:12AM -0700, Fangrui Song wrote:
> > > (1) In romlayout.S, .fixedaddr.\addr sections do have not the SHF_ALLOC flag.
> > > It does not make sense to reference a SHF_ALLOC section from a non-SHF_ALLOC section via R_386_PC16.
> > > GNU ld allows it but lld will warn. Add the SHF_ALLOC flag.
> > >
> > > (2) lld requires output section descriptions to be sorted by address.
> > >  Just sort the addresses beforehand.
> >
> > This looks like it should be two separate patches.
> >
> > I know it's a pain to redo patches, but separating out changes helps
> > greatly when tracking down regressions via "git bisect".
>
> I would hope [PATCH 2/4] Makefile: Change ET_EXEC to ET_REL so that lld can link bios.bin.elf
> can be applied first it it is acceptable.
>
> Like I said, these 1~3 have no dependency at all.
> Redoing a patch series and resending as a whole may mess up the list.
> > > Signed-off-by: Fangrui Song
> > >
> > > scripts/layoutrom.py | 4 ++++
> > >  src/romlayout.S      | 2 +-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > > index 6616721..4c55390 100755
> > > --- a/scripts/layoutrom.py
> > > +++ b/scripts/layoutrom.py
> > > @@ -321,6 +321,10 @@ def outXRefs(sections, useseg=0, exportsyms=[], forcedelta=0):
> > >
> > > # Write LD script includes for the given sections
> > >  def outSections(sections, useseg=0):
> > > if useseg:
> > >    sections.sort(key=lambda x: x.finalsegloc)
> > > else:
> > >    sections.sort(key=lambda x: x.finalloc)
> >
> > This looks odd to me - there shouldn't be a need to change the sort
> > order based on useseg, as finalloc should always have the same order
> > as finalsegloc.  Also, this code alters the input list which is
> > confusing - perhaps use "sections = sorted(sections, key=...)".
>
> Just to confirm, I should use:
>
> if not useseg:
>     sections = sorted(sections, key=lambda x: x.finalloc)

I was proposing to unconditionally use:
  sections = sorted(sections, key=lambda x: x.finalloc)

-Kevin
> > >  out = ""
> > >  for section in sections:
> > >      loc = section.finalloc
> > >
> > > 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 3/4] romlayout32flag.lds: Use `. +=` instead of `. =`

[Fangrui Song]

On 2020-04-06, Kevin O'Connor wrote:
> On Wed, Apr 01, 2020 at 10:29:14AM -0700, Fangrui Song wrote:
> > 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 | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py
> > index 4c55390..94d4412 100755
> > --- a/scripts/layoutrom.py
> > +++ b/scripts/layoutrom.py
> > @@ -339,14 +339,19 @@ def outRelSections(sections, startsym, useseg=0):
> >                  if section.finalloc is not None]
> >      sections.sort(key=operator.itemgetter(0))
> >      out = ""
> > location = "_reloc_init_end"
> > for addr, section in sections:
> >     loc = section.finalloc
> >     if useseg:
> >         loc = section.finalsegloc
> >    out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
> >    if location == "_reloc_init_end":
> >        out += ". += 0x%x - %s ;\n" % (loc, location)
> >    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
>
> I'm finding this code confusing.  I would recommend "location" always
> be a string or always be an integer - mixing them makes the code hard
> to follow.  Also, this code removes the only reference to startsym,
> but doesn't update the function signature.  Similarly, it hardcodes
> "_reloc_init_end" when I think that should be passed to the function.
>
> -Kevin

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

Non-zero useseg does not appear to be used, but I don't understand it
enough to fix it.
> >  return out
> >
> > # Build linker script output for a list of relocations.
> >
> > 2.26.0.rc2.310.g2932bb562d-goog
> > _______________________________________________
> > SeaBIOS mailing list -- [e-mail]
> > To unsubscribe send an email to [e-mail]