This improves the portability of the linker script and allows lld to link rom.o We can thus delete an ld check detecting "cannot move location counter backwards".
Dot assignment inside an output section has a inconsistent behavior which makes lld difficult to implement. See https://bugs.llvm.org/show_bug.cgi?id=43083
Signed-off-by: Fangrui Song maskray@google.com --- scripts/layoutrom.py | 7 ++++++- scripts/test-build.sh | 42 +----------------------------------------- 2 files changed, 7 insertions(+), 42 deletions(-)
diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py index 6616721..bbf07aa 100755 --- a/scripts/layoutrom.py +++ b/scripts/layoutrom.py @@ -335,14 +335,19 @@ def outRelSections(sections, startsym, useseg=0): if section.finalloc is not None] sections.sort(key=operator.itemgetter(0)) out = "" + dot = "_reloc_init_end" for addr, section in sections: loc = section.finalloc if useseg: loc = section.finalsegloc - out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym) + if isinstance(dot, str): + out += ". += 0x%x - %s ;\n" % (loc, dot) + elif dot < loc: + out += ". += 0x%x ;\n" % (loc-dot,) if section.name in ('.rodata.str1.1', '.rodata'): out += "_rodata%s = . ;\n" % (section.fileid,) out += "*%s.*(%s)\n" % (section.fileid, section.name) + dot = loc + section.size return out
# Build linker script output for a list of relocations. 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
On Fri, Mar 13, 2020 at 10:17:09PM -0700, Fangrui Song wrote:
This improves the portability of the linker script and allows lld to link rom.o We can thus delete an ld check detecting "cannot move location counter backwards".
Dot assignment inside an output section has a inconsistent behavior which makes lld difficult to implement. See https://bugs.llvm.org/show_bug.cgi?id=43083
Signed-off-by: Fangrui Song maskray@google.com
scripts/layoutrom.py | 7 ++++++- scripts/test-build.sh | 42 +----------------------------------------- 2 files changed, 7 insertions(+), 42 deletions(-)
diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py index 6616721..bbf07aa 100755 --- a/scripts/layoutrom.py +++ b/scripts/layoutrom.py @@ -335,14 +335,19 @@ def outRelSections(sections, startsym, useseg=0): if section.finalloc is not None] sections.sort(key=operator.itemgetter(0)) out = ""
- dot = "_reloc_init_end" for addr, section in sections: loc = section.finalloc if useseg: loc = section.finalsegloc
out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
if isinstance(dot, str):out += ". += 0x%x - %s ;\n" % (loc, dot)elif dot < loc:out += ". += 0x%x ;\n" % (loc-dot,) if section.name in ('.rodata.str1.1', '.rodata'): out += "_rodata%s = . ;\n" % (section.fileid,) out += "*%s.*(%s)\n" % (section.fileid, section.name)dot = loc + section.size
I think it's fine to use relative assignments instead of absolute assignments, but I'm struggling to understand the python code above. I recommend writing the code without isinstance() and use a more descriptive variable name than "dot".
return out# Build linker script output for a list of relocations. 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.
This change seems unrelated to the change above (and it isn't mentioned in the commit summary). I'm not sure why this alignment test (which is to catch old broken version of ld) should be removed. If it does need to be removed then I recommend separating it out to a separate patch and explain why it needs to be removed in the commit message.
Separately, as mentioned by Gerd and Paul, it would help if the patches were updated, sent as a series, and had commit messages that apply properly with "git am".
Thanks, -Kevin
-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.25.1 _______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On 2020-03-17, Kevin O'Connor wrote:
On Fri, Mar 13, 2020 at 10:17:09PM -0700, Fangrui Song wrote:
This improves the portability of the linker script and allows lld to link rom.o We can thus delete an ld check detecting "cannot move location counter backwards".
Dot assignment inside an output section has a inconsistent behavior which makes lld difficult to implement. See https://bugs.llvm.org/show_bug.cgi?id=43083
Signed-off-by: Fangrui Song maskray@google.com
scripts/layoutrom.py | 7 ++++++- scripts/test-build.sh | 42 +----------------------------------------- 2 files changed, 7 insertions(+), 42 deletions(-)
diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py index 6616721..bbf07aa 100755 --- a/scripts/layoutrom.py +++ b/scripts/layoutrom.py @@ -335,14 +335,19 @@ def outRelSections(sections, startsym, useseg=0): if section.finalloc is not None] sections.sort(key=operator.itemgetter(0)) out = ""
- dot = "_reloc_init_end" for addr, section in sections: loc = section.finalloc if useseg: loc = section.finalsegloc
out += ". = ( 0x%x - %s ) ;\n" % (loc, startsym)
if isinstance(dot, str):out += ". += 0x%x - %s ;\n" % (loc, dot)elif dot < loc:out += ". += 0x%x ;\n" % (loc-dot,) if section.name in ('.rodata.str1.1', '.rodata'): out += "_rodata%s = . ;\n" % (section.fileid,) out += "*%s.*(%s)\n" % (section.fileid, section.name)dot = loc + section.sizeI think it's fine to use relative assignments instead of absolute assignments, but I'm struggling to understand the python code above. I recommend writing the code without isinstance() and use a more descriptive variable name than "dot".
return out# Build linker script output for a list of relocations. 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.
This change seems unrelated to the change above (and it isn't mentioned in the commit summary). I'm not sure why this alignment test (which is to catch old broken version of ld) should be removed. If it does need to be removed then I recommend separating it out to a separate patch and explain why it needs to be removed in the commit message.
Separately, as mentioned by Gerd and Paul, it would help if the patches were updated, sent as a series, and had commit messages that apply properly with "git am".
Thanks, -Kevin
Thanks for review. I am sorry that I will not adopt the patch series suggestion. These are independent changes. I think I've started a topic about my interests to contribute to clang+llvm binary utilities+lld portability improvement. Adding a cover letter to replicate all the stuff does not seem very necessary. Maybe the more important point is that I lack the experience to manage something like PATCH {0,1,2,3,4,5,6,7,8}/8
Logically they really don't fit as a series.
I will defer the ld check cleanup to a future patch.
-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.25.1
From 7d3bf908066c0bae6383b03a8c22b7f0c00bd3f5 Mon Sep 17 00:00:00 2001 From: Fangrui Song maskray@google.com Date: Tue, 17 Mar 2020 23:34:44 -0700 Subject: [PATCH v2] romlayout32flag.lds: Use `. +=` instead of `. =`
This improves the portability of the linker script and allows lld to link rom.o
Dot assignment inside an output section has a inconsistent behavior which makes lld difficult to implement. See https://bugs.llvm.org/show_bug.cgi?id=43083
Signed-off-by: Fangrui Song maskray@google.com --- scripts/layoutrom.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/layoutrom.py b/scripts/layoutrom.py index 6616721..505e9a1 100755 --- a/scripts/layoutrom.py +++ b/scripts/layoutrom.py @@ -335,14 +335,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.
Thanks for review. I am sorry that I will not adopt the patch series suggestion. These are independent changes. I think I've started a topic about my interests to contribute to clang+llvm binary utilities+lld portability improvement. Adding a cover letter to replicate all the stuff does not seem very necessary. Maybe the more important point is that I lack the experience to manage something like PATCH {0,1,2,3,4,5,6,7,8}/8
Git will do all that for you. I assume you have these patches in a branch anyway? Then you can use "git format-patch master..$branch" to create numbered patches and "git send-email" to send them to the list.
HTH, Gerd
On Wed, Mar 18, 2020 at 12:04 AM Gerd Hoffmann kraxel@redhat.com wrote:
Thanks for review. I am sorry that I will not adopt the patch series suggestion. These are independent changes. I think I've started a topic about my interests to contribute to clang+llvm binary utilities+lld portability improvement. Adding a cover letter to replicate all the stuff does not seem very necessary. Maybe the more important point is that I lack the experience to manage something like PATCH {0,1,2,3,4,5,6,7,8}/8
Git will do all that for you. I assume you have these patches in a branch anyway? Then you can use "git format-patch master..$branch" to create numbered patches and "git send-email" to send them to the list.
HTH, Gerd
I have several branches. (I do very poor branch management: git checkout foo; edit; git commit --amend; git rebase --onto foo bar^' bar ...)
Unless explicitly marked (very few as you can see; I've also found new cleanup opportunities after sending out previous patches), they are independent. They don't conflict with each other. Apply order does not matter. Sending them as a series just for the sake of it is probably not really worth it.
-
Fangrui Song -
Fāng-ruì Sòng
-
Gerd Hoffmann -
Kevin O'Connor