* [PATCH] gas: copy st_size only if unset
@ 2022-03-31 3:37 Fangrui Song
2022-03-31 6:22 ` Jan Beulich
0 siblings, 1 reply; 11+ messages in thread
From: Fangrui Song @ 2022-03-31 3:37 UTC (permalink / raw)
To: binutils, Alan Modra, Nick Clifton
For
```
.size foo1, 1
foo1:
.set bar1, foo1
.size bar1, 2
.size bar2, 2
.set bar2, foo1
.set bar3, foo2
.size bar3, 2
.size bar4, 2
.set bar4, foo2
.size foo2, 1
foo2:
```
bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
(generally directives on the new symbol should win) and is relied upon by glibc
stdio-common/errlist.c:
```
.hidden _sys_errlist_internal
.globl _sys_errlist_internal
.type _sys_errlist_internal, @object
.size _sys_errlist_internal, 1072
_sys_errlist_internal:
.globl __GLIBC_2_1_sys_errlist
.set __GLIBC_2_1_sys_errlist, _sys_errlist_internal
.type __GLIBC_2_1_sys_errlist, %object
.size __GLIBC_2_1_sys_errlist, 125 * (64 / 8)
// glibc expects that .size __GLIBC_2_1_sys_errlist, 125 * (64 / 8) wins.
```
The behavior of bar2/bar3/bar4 seems brittle. To avoid the reordering of the two
code blocks which will result in the bar3 situation, glibc compiles errlist.c
with gcc -fno-toplevel-reorder (previously -fno-unit-at-a-time).
To fix the inconsistency and improve robustness, make bar2/bar3/bar4 match bar1,
removing the directive order sensitivity.
There is a pity that `.size dest, 0` is indistinguishable from the case where
dest is unset, but it should be fine.
PR gas/29012
* config/obj-elf.c (elf_copy_symbol_attributes): don't copy if src's size
has been set.
* testsuite/gas/elf/elf.exp: New test.
* testsuite/gas/elf/size.d: New file.
* testsuite/gas/elf/size.s: Likewise.
---
gas/config/obj-elf.c | 24 ++++++++++--------------
gas/testsuite/gas/elf/elf.exp | 1 +
gas/testsuite/gas/elf/size.d | 13 +++++++++++++
gas/testsuite/gas/elf/size.s | 20 ++++++++++++++++++++
4 files changed, 44 insertions(+), 14 deletions(-)
create mode 100644 gas/testsuite/gas/elf/size.d
create mode 100644 gas/testsuite/gas/elf/size.s
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c
index b56f8aa5220..b218ff6a574 100644
--- a/gas/config/obj-elf.c
+++ b/gas/config/obj-elf.c
@@ -2154,27 +2154,23 @@ elf_obj_symbol_clone_hook (symbolS *newsym, symbolS *orgsym ATTRIBUTE_UNUSED)
}
}
-/* When setting one symbol equal to another, by default we probably
- want them to have the same "size", whatever it means in the current
- context. */
-
void
elf_copy_symbol_attributes (symbolS *dest, symbolS *src)
{
struct elf_obj_sy *srcelf = symbol_get_obj (src);
struct elf_obj_sy *destelf = symbol_get_obj (dest);
- if (srcelf->size)
+ /* If size is unset, copy size from src. Because we don't track whether
+ .size has been used, we can't differentiate .size dest, 0 from the case
+ where dest's size is unset. */
+ if (!destelf->size && S_GET_SIZE (dest) == 0)
{
- if (destelf->size == NULL)
- destelf->size = XNEW (expressionS);
- *destelf->size = *srcelf->size;
+ if (srcelf->size)
+ {
+ destelf->size = XNEW (expressionS);
+ *destelf->size = *srcelf->size;
+ }
+ S_SET_SIZE (dest, S_GET_SIZE (src));
}
- else
- {
- free (destelf->size);
- destelf->size = NULL;
- }
- S_SET_SIZE (dest, S_GET_SIZE (src));
/* Don't copy visibility. */
S_SET_OTHER (dest, (ELF_ST_VISIBILITY (S_GET_OTHER (dest))
| (S_GET_OTHER (src) & ~ELF_ST_VISIBILITY (-1))));
diff --git a/gas/testsuite/gas/elf/elf.exp b/gas/testsuite/gas/elf/elf.exp
index 081ab4ec683..963730a87a5 100644
--- a/gas/testsuite/gas/elf/elf.exp
+++ b/gas/testsuite/gas/elf/elf.exp
@@ -277,6 +277,7 @@ if { [is_elf_format] } then {
run_dump_test "section28"
run_dump_test "section29"
run_dump_test "sh-link-zero"
+ run_dump_test "size"
run_dump_test "dwarf2-1" $dump_opts
run_dump_test "dwarf2-2" $dump_opts
run_dump_test "dwarf2-3" $dump_opts
diff --git a/gas/testsuite/gas/elf/size.d b/gas/testsuite/gas/elf/size.d
new file mode 100644
index 00000000000..12de8a70c39
--- /dev/null
+++ b/gas/testsuite/gas/elf/size.d
@@ -0,0 +1,13 @@
+#readelf: -sW
+#name: ELF symbol size
+
+#...
+ +[0-9]+: 0+ +1 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +foo1
+ +[0-9]+: 0+ +2 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar1
+ +[0-9]+: 0+ +2 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar2
+ +[0-9]+: 0+ +2 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar3
+ +[0-9]+: 0+ +1 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +foo2
+ +[0-9]+: 0+ +2 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar4
+ +[0-9]+: 0+ +1 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar5
+ +[0-9]+: 0+ +1 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar6
+#pass
diff --git a/gas/testsuite/gas/elf/size.s b/gas/testsuite/gas/elf/size.s
new file mode 100644
index 00000000000..b7729f9c3b3
--- /dev/null
+++ b/gas/testsuite/gas/elf/size.s
@@ -0,0 +1,20 @@
+.text
+
+.size foo1, 1
+foo1:
+
+.set bar1, foo1
+.size bar1, 2
+.size bar2, 2
+.set bar2, foo1
+
+.set bar3, foo2
+.size bar3, 2
+.size bar4, 2
+.set bar4, foo2
+
+.set bar5, foo1
+.set bar6, foo2
+
+.size foo2, 1
+foo2:
--
2.35.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-03-31 3:37 [PATCH] gas: copy st_size only if unset Fangrui Song
@ 2022-03-31 6:22 ` Jan Beulich
2022-04-01 1:33 ` Fangrui Song
0 siblings, 1 reply; 11+ messages in thread
From: Jan Beulich @ 2022-03-31 6:22 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils, Alan Modra, Nick Clifton
On 31.03.2022 05:37, Fangrui Song wrote:
> For
> ```
> .size foo1, 1
> foo1:
>
> .set bar1, foo1
> .size bar1, 2
> .size bar2, 2
> .set bar2, foo1
>
> .set bar3, foo2
> .size bar3, 2
> .size bar4, 2
> .set bar4, foo2
>
> .size foo2, 1
> foo2:
> ```
>
> bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
> (generally directives on the new symbol should win) and is relied upon by glibc
> stdio-common/errlist.c:
But what about
foo1:
.size foo1, 1
foo2:
.size foo2, 2
.set bar, foo1
.set bar, foo2
I would think bar's size should end up being 2 in this case, whereas I
think it would end up being 1 with your change.
Jan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-03-31 6:22 ` Jan Beulich
@ 2022-04-01 1:33 ` Fangrui Song
2022-04-04 11:27 ` Jan Beulich
2022-04-08 7:42 ` Alan Modra
0 siblings, 2 replies; 11+ messages in thread
From: Fangrui Song @ 2022-04-01 1:33 UTC (permalink / raw)
To: Jan Beulich; +Cc: binutils, Alan Modra, Nick Clifton
On 2022-03-31, Jan Beulich wrote:
>On 31.03.2022 05:37, Fangrui Song wrote:
>> For
>> ```
>> .size foo1, 1
>> foo1:
>>
>> .set bar1, foo1
>> .size bar1, 2
>> .size bar2, 2
>> .set bar2, foo1
>>
>> .set bar3, foo2
>> .size bar3, 2
>> .size bar4, 2
>> .set bar4, foo2
>>
>> .size foo2, 1
>> foo2:
>> ```
>>
>> bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
>> (generally directives on the new symbol should win) and is relied upon by glibc
>> stdio-common/errlist.c:
>
>But what about
>
>foo1:
> .size foo1, 1
>foo2:
> .size foo2, 2
> .set bar, foo1
> .set bar, foo2
>
>I would think bar's size should end up being 2 in this case, whereas I
>think it would end up being 1 with your change.
>
>Jan
bar's size is 2. The patch has no effect, because `dest` variables for the two
invocations are (surprisingly) different!
bar is a volatile symbol (created by .set instead of .equiv). The
second .set directive creates calls gas.c/read.c:3273 symbol_clone (symbolP, 1).
That said, I think it makes sense to add tests for this as well.
I'll use the following if this patch is accepted.
.text
.size foo1, 1
foo1:
.set bar1, foo1
.size bar1, 2
.size bar2, 2
.set bar2, foo1
.set bar3, foo2
.size bar3, 2
.size bar4, 2
.set bar4, foo2
.set bar5, foo1
.set bar6, foo2
.size foo2, 3
foo2:
.set bar7, foo1
.set bar7, foo2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-01 1:33 ` Fangrui Song
@ 2022-04-04 11:27 ` Jan Beulich
2022-04-08 7:42 ` Alan Modra
1 sibling, 0 replies; 11+ messages in thread
From: Jan Beulich @ 2022-04-04 11:27 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils, Alan Modra, Nick Clifton
On 01.04.2022 03:33, Fangrui Song wrote:
> On 2022-03-31, Jan Beulich wrote:
>> On 31.03.2022 05:37, Fangrui Song wrote:
>>> For
>>> ```
>>> .size foo1, 1
>>> foo1:
>>>
>>> .set bar1, foo1
>>> .size bar1, 2
>>> .size bar2, 2
>>> .set bar2, foo1
>>>
>>> .set bar3, foo2
>>> .size bar3, 2
>>> .size bar4, 2
>>> .set bar4, foo2
>>>
>>> .size foo2, 1
>>> foo2:
>>> ```
>>>
>>> bar1's size is 2 while bar2, bar3, bar4's is 1. The behavior of bar1 makes sense
>>> (generally directives on the new symbol should win) and is relied upon by glibc
>>> stdio-common/errlist.c:
>>
>> But what about
>>
>> foo1:
>> .size foo1, 1
>> foo2:
>> .size foo2, 2
>> .set bar, foo1
>> .set bar, foo2
>>
>> I would think bar's size should end up being 2 in this case, whereas I
>> think it would end up being 1 with your change.
>>
>> Jan
>
> bar's size is 2. The patch has no effect, because `dest` variables for the two
> invocations are (surprisingly) different!
>
> bar is a volatile symbol (created by .set instead of .equiv). The
> second .set directive creates calls gas.c/read.c:3273 symbol_clone (symbolP, 1).
Ah, yes.
> That said, I think it makes sense to add tests for this as well.
> I'll use the following if this patch is accepted.
>
> .text
>
> .size foo1, 1
> foo1:
>
> .set bar1, foo1
> .size bar1, 2
> .size bar2, 2
> .set bar2, foo1
>
> .set bar3, foo2
> .size bar3, 2
> .size bar4, 2
> .set bar4, foo2
>
> .set bar5, foo1
> .set bar6, foo2
>
> .size foo2, 3
> foo2:
>
> .set bar7, foo1
> .set bar7, foo2
Patch is okay with the extended testcase.
Jan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-01 1:33 ` Fangrui Song
2022-04-04 11:27 ` Jan Beulich
@ 2022-04-08 7:42 ` Alan Modra
2022-04-08 21:26 ` Fangrui Song
1 sibling, 1 reply; 11+ messages in thread
From: Alan Modra @ 2022-04-08 7:42 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils
Your patch regressed these tests. Please investigate.
aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
aarch64-elf +FAIL: ELF symbol size
aarch64-linux +FAIL: ELF symbol size
alpha-linux +FAIL: ELF symbol size
alpha-netbsd +FAIL: ELF symbol size
alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
hppa64-hp-hpux11.23 +FAIL: ELF symbol size
hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
nds32be-elf +FAIL: readelf -wiaoRlL dw5
nds32le-linux +FAIL: readelf -wiaoRlL dw5
riscv32-elf +FAIL: ELF symbol size
riscv64-linux +FAIL: ELF symbol size
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 7:42 ` Alan Modra
@ 2022-04-08 21:26 ` Fangrui Song
2022-04-08 23:01 ` Fangrui Song
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Fangrui Song @ 2022-04-08 21:26 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On 2022-04-08, Alan Modra wrote:
>Your patch regressed these tests. Please investigate.
>
>aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
>aarch64-elf +FAIL: ELF symbol size
>aarch64-linux +FAIL: ELF symbol size
>alpha-linux +FAIL: ELF symbol size
>alpha-netbsd +FAIL: ELF symbol size
>alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
>hppa64-hp-hpux11.23 +FAIL: ELF symbol size
>hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>nds32be-elf +FAIL: readelf -wiaoRlL dw5
>nds32le-linux +FAIL: readelf -wiaoRlL dw5
>riscv32-elf +FAIL: ELF symbol size
>riscv64-linux +FAIL: ELF symbol size
>
>--
>Alan Modra
>Australia Development Lab, IBM
Sorry for the breakage. a3a7f5e1586467b137b8dcdcd2f74f5efa9f3919 should fix aarch64/alpha/hppa/riscv.
I am puzzled by
> nds32be-elf +FAIL: readelf -wiaoRlL dw5
> nds32le-linux +FAIL: readelf -wiaoRlL dw5
../../configure CFLAGS='-O0 -g' CXXFLAGS='-O0 -g' --target=nds32be-elf
make -j 50 all-binutils && make -j 50 check-binutils
binutils/tmpdir/dw5.o does not change with "gas: copy st_size only if unset"
> hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
I am also puzzled by this. I confirm it passes on --target=hppa64-linux
I'll need to investigate it.
PS: is there a shortcut than building all-binutils/all-gas with various
different --target= (especially these exotic ones like
alpha/hppa/nds32)?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 21:26 ` Fangrui Song
@ 2022-04-08 23:01 ` Fangrui Song
2022-04-08 23:06 ` Fangrui Song
2022-04-09 1:12 ` Alan Modra
2022-04-09 0:43 ` Alan Modra
2022-04-09 4:29 ` Jeff Law
2 siblings, 2 replies; 11+ messages in thread
From: Fangrui Song @ 2022-04-08 23:01 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On 2022-04-08, Fangrui Song wrote:
>On 2022-04-08, Alan Modra wrote:
>>Your patch regressed these tests. Please investigate.
>>
>>aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
>>aarch64-elf +FAIL: ELF symbol size
>>aarch64-linux +FAIL: ELF symbol size
>>alpha-linux +FAIL: ELF symbol size
>>alpha-netbsd +FAIL: ELF symbol size
>>alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
>>hppa64-hp-hpux11.23 +FAIL: ELF symbol size
>>hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>nds32be-elf +FAIL: readelf -wiaoRlL dw5
>>nds32le-linux +FAIL: readelf -wiaoRlL dw5
>>riscv32-elf +FAIL: ELF symbol size
>>riscv64-linux +FAIL: ELF symbol size
>>
>>--
>>Alan Modra
>>Australia Development Lab, IBM
>
>Sorry for the breakage. a3a7f5e1586467b137b8dcdcd2f74f5efa9f3919 should fix aarch64/alpha/hppa/riscv.
>I am puzzled by
>
>>nds32be-elf +FAIL: readelf -wiaoRlL dw5
>>nds32le-linux +FAIL: readelf -wiaoRlL dw5
OK, I think this is related to Nick's
19c26da69d68d5d863f37c06ad73ab6292d02ffa
("Add code to display the contents of .debug_loclists sections which contain offset entry tables.")
There is a diagnostic which has moved from somewhere in the middle to
the top:
readelf: Warning: unable to apply unsupported type 208 to section .debug_loclists
Adding #... as the first line will fix the test for nds32*, but I am
unsure whether the test should be adjusted to unsupport nds32*.
>../../configure CFLAGS='-O0 -g' CXXFLAGS='-O0 -g' --target=nds32be-elf
>make -j 50 all-binutils && make -j 50 check-binutils
>binutils/tmpdir/dw5.o does not change with "gas: copy st_size only if unset"
>
>>hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>
>I am also puzzled by this. I confirm it passes on --target=hppa64-linux
>I'll need to investigate it.
>
>
>PS: is there a shortcut than building all-binutils/all-gas with various
>different --target= (especially these exotic ones like
>alpha/hppa/nds32)?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 23:01 ` Fangrui Song
@ 2022-04-08 23:06 ` Fangrui Song
2022-04-09 1:12 ` Alan Modra
1 sibling, 0 replies; 11+ messages in thread
From: Fangrui Song @ 2022-04-08 23:06 UTC (permalink / raw)
To: Alan Modra; +Cc: binutils
On 2022-04-08, Fangrui Song wrote:
>On 2022-04-08, Fangrui Song wrote:
>>On 2022-04-08, Alan Modra wrote:
>>>Your patch regressed these tests. Please investigate.
>>>
>>>aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
>>>aarch64-elf +FAIL: ELF symbol size
>>>aarch64-linux +FAIL: ELF symbol size
>>>alpha-linux +FAIL: ELF symbol size
>>>alpha-netbsd +FAIL: ELF symbol size
>>>alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
>>>hppa64-hp-hpux11.23 +FAIL: ELF symbol size
>>>hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>>hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>>nds32be-elf +FAIL: readelf -wiaoRlL dw5
>>>nds32le-linux +FAIL: readelf -wiaoRlL dw5
>>>riscv32-elf +FAIL: ELF symbol size
>>>riscv64-linux +FAIL: ELF symbol size
>>>
>>>--
>>>Alan Modra
>>>Australia Development Lab, IBM
>>
>>Sorry for the breakage. a3a7f5e1586467b137b8dcdcd2f74f5efa9f3919 should fix aarch64/alpha/hppa/riscv.
>>I am puzzled by
>>
>>>nds32be-elf +FAIL: readelf -wiaoRlL dw5
>>>nds32le-linux +FAIL: readelf -wiaoRlL dw5
>
>OK, I think this is related to Nick's
>19c26da69d68d5d863f37c06ad73ab6292d02ffa
>("Add code to display the contents of .debug_loclists sections which contain offset entry tables.")
>
>There is a diagnostic which has moved from somewhere in the middle to
>the top:
>
> readelf: Warning: unable to apply unsupported type 208 to section .debug_loclists
>
>Adding #... as the first line will fix the test for nds32*, but I am
>unsure whether the test should be adjusted to unsupport nds32*.
>
>>../../configure CFLAGS='-O0 -g' CXXFLAGS='-O0 -g' --target=nds32be-elf
>>make -j 50 all-binutils && make -j 50 check-binutils
>>binutils/tmpdir/dw5.o does not change with "gas: copy st_size only if unset"
>>
>>>hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>>hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>>
>>I am also puzzled by this. I confirm it passes on --target=hppa64-linux
>>I'll need to investigate it.
I have tested that the XPASS->PASS is unrelated to this particular gas patch.
>>
>>PS: is there a shortcut than building all-binutils/all-gas with various
>>different --target= (especially these exotic ones like
>>alpha/hppa/nds32)?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 21:26 ` Fangrui Song
2022-04-08 23:01 ` Fangrui Song
@ 2022-04-09 0:43 ` Alan Modra
2022-04-09 4:29 ` Jeff Law
2 siblings, 0 replies; 11+ messages in thread
From: Alan Modra @ 2022-04-09 0:43 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils
On Fri, Apr 08, 2022 at 02:26:13PM -0700, Fangrui Song wrote:
> On 2022-04-08, Alan Modra wrote:
> > Your patch regressed these tests. Please investigate.
> >
> > aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
> > aarch64-elf +FAIL: ELF symbol size
> > aarch64-linux +FAIL: ELF symbol size
> > alpha-linux +FAIL: ELF symbol size
> > alpha-netbsd +FAIL: ELF symbol size
> > alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
> > hppa64-hp-hpux11.23 +FAIL: ELF symbol size
> > hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> > hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> > nds32be-elf +FAIL: readelf -wiaoRlL dw5
> > nds32le-linux +FAIL: readelf -wiaoRlL dw5
> > riscv32-elf +FAIL: ELF symbol size
> > riscv64-linux +FAIL: ELF symbol size
> >
> > --
> > Alan Modra
> > Australia Development Lab, IBM
>
> Sorry for the breakage. a3a7f5e1586467b137b8dcdcd2f74f5efa9f3919 should fix aarch64/alpha/hppa/riscv.
Thanks for fixing these.
> I am puzzled by
>
> > nds32be-elf +FAIL: readelf -wiaoRlL dw5
> > nds32le-linux +FAIL: readelf -wiaoRlL dw5
>
> ../../configure CFLAGS='-O0 -g' CXXFLAGS='-O0 -g' --target=nds32be-elf
> make -j 50 all-binutils && make -j 50 check-binutils
> binutils/tmpdir/dw5.o does not change with "gas: copy st_size only if unset"
Hmm, I just did a couple of clean builds and confirmed the fail is
unrelated to your patch. I'll look into why this test was failing for
me.
> > hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> > hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
>
> I am also puzzled by this. I confirm it passes on --target=hppa64-linux
> I'll need to investigate it.
>
>
> PS: is there a shortcut than building all-binutils/all-gas with various
> different --target= (especially these exotic ones like
> alpha/hppa/nds32)?
No, because gas only supports one target, unlike many of the other
binutils that can be configured to support multiple targets.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 23:01 ` Fangrui Song
2022-04-08 23:06 ` Fangrui Song
@ 2022-04-09 1:12 ` Alan Modra
1 sibling, 0 replies; 11+ messages in thread
From: Alan Modra @ 2022-04-09 1:12 UTC (permalink / raw)
To: Fangrui Song; +Cc: binutils
On Fri, Apr 08, 2022 at 04:01:13PM -0700, Fangrui Song wrote:
> On 2022-04-08, Fangrui Song wrote:
> > On 2022-04-08, Alan Modra wrote:
> > > Your patch regressed these tests. Please investigate.
> > >
> > > aarch64_be-linux-gnu_ilp32 +FAIL: ELF symbol size
> > > aarch64-elf +FAIL: ELF symbol size
> > > aarch64-linux +FAIL: ELF symbol size
> > > alpha-linux +FAIL: ELF symbol size
> > > alpha-netbsd +FAIL: ELF symbol size
> > > alpha-unknown-freebsd4.7 +FAIL: ELF symbol size
> > > hppa64-hp-hpux11.23 +FAIL: ELF symbol size
> > > hppa64-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> > > hppa-linux +XPASS: .reloc against undefined local symbol (PR 27228)
> > > nds32be-elf +FAIL: readelf -wiaoRlL dw5
> > > nds32le-linux +FAIL: readelf -wiaoRlL dw5
> > > riscv32-elf +FAIL: ELF symbol size
> > > riscv64-linux +FAIL: ELF symbol size
> > >
> > > --
> > > Alan Modra
> > > Australia Development Lab, IBM
> >
> > Sorry for the breakage. a3a7f5e1586467b137b8dcdcd2f74f5efa9f3919 should fix aarch64/alpha/hppa/riscv.
> > I am puzzled by
> >
> > > nds32be-elf +FAIL: readelf -wiaoRlL dw5
> > > nds32le-linux +FAIL: readelf -wiaoRlL dw5
>
> OK, I think this is related to Nick's
> 19c26da69d68d5d863f37c06ad73ab6292d02ffa
> ("Add code to display the contents of .debug_loclists sections which contain offset entry tables.")
>
> There is a diagnostic which has moved from somewhere in the middle to
> the top:
>
> readelf: Warning: unable to apply unsupported type 208 to section .debug_loclists
>
> Adding #... as the first line will fix the test for nds32*, but I am
> unsure whether the test should be adjusted to unsupport nds32*.
You are exactly correct in your analysis. Or we are both wrong in the
same way, because I independently came to the same conclusion. :-)
The test shouldn't be adjusted. It's now showing a defect in readelf
support for nds32 that was accidentally hidden before Nick's change.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] gas: copy st_size only if unset
2022-04-08 21:26 ` Fangrui Song
2022-04-08 23:01 ` Fangrui Song
2022-04-09 0:43 ` Alan Modra
@ 2022-04-09 4:29 ` Jeff Law
2 siblings, 0 replies; 11+ messages in thread
From: Jeff Law @ 2022-04-09 4:29 UTC (permalink / raw)
To: binutils
On 4/8/2022 3:26 PM, Fangrui Song wrote:
>
>
>
> PS: is there a shortcut than building all-binutils/all-gas with various
> different --target= (especially these exotic ones like
> alpha/hppa/nds32)?
Not really. But you can head to the upstream GCC tester and grab log
files. It happens to also test binutils on a variety of targets.
The tester recently moved out of AWS back to a machine I host. So the
redirect from the gcc web pages needs updating. Anyway, point your
browser here:
http://law-sandy.freeddns.org:8080/job/nds32le-elf/
From that page we see that the build on April 5 was good and the build
on April 6 was bad. It flipped back to green today as I told the system
generate new baselines for the nds targets as I'm primarily interested
in catching GCC regressions.
You can dig into the log files to get the git hashes in use. I use
those to see my bisections when I need to chase something down.
Anyway, thought you might find it useful.
Jeff
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-04-09 4:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-31 3:37 [PATCH] gas: copy st_size only if unset Fangrui Song
2022-03-31 6:22 ` Jan Beulich
2022-04-01 1:33 ` Fangrui Song
2022-04-04 11:27 ` Jan Beulich
2022-04-08 7:42 ` Alan Modra
2022-04-08 21:26 ` Fangrui Song
2022-04-08 23:01 ` Fangrui Song
2022-04-08 23:06 ` Fangrui Song
2022-04-09 1:12 ` Alan Modra
2022-04-09 0:43 ` Alan Modra
2022-04-09 4:29 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).