diff options
author | Fangrui Song <i@maskray.me> | 2022-04-04 08:43:50 -0700 |
---|---|---|
committer | Fangrui Song <maskray@google.com> | 2022-04-04 08:43:50 -0700 |
commit | 867b8c308a40e79f90db4051cafc273f00f881fb (patch) | |
tree | a7c39c1860484bf1064db52fbdd2b340d2181036 | |
parent | edbc15e6c4f463173b25b1f8042346c2e1a78602 (diff) | |
download | gdb-867b8c308a40e79f90db4051cafc273f00f881fb.zip gdb-867b8c308a40e79f90db4051cafc273f00f881fb.tar.gz gdb-867b8c308a40e79f90db4051cafc273f00f881fb.tar.bz2 |
gas: copy st_size only if unset
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 the compromise seems 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.
-rw-r--r-- | gas/config/obj-elf.c | 24 | ||||
-rw-r--r-- | gas/testsuite/gas/elf/elf.exp | 1 | ||||
-rw-r--r-- | gas/testsuite/gas/elf/size.d | 14 | ||||
-rw-r--r-- | gas/testsuite/gas/elf/size.s | 23 |
4 files changed, 48 insertions, 14 deletions
diff --git a/gas/config/obj-elf.c b/gas/config/obj-elf.c index b56f8aa..b218ff6 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 (destelf->size == NULL) - destelf->size = XNEW (expressionS); - *destelf->size = *srcelf->size; - } - else + /* 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) { - free (destelf->size); - destelf->size = NULL; + if (srcelf->size) + { + destelf->size = XNEW (expressionS); + *destelf->size = *srcelf->size; + } + S_SET_SIZE (dest, S_GET_SIZE (src)); } - 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 081ab4e..963730a 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 0000000..51bf4eb --- /dev/null +++ b/gas/testsuite/gas/elf/size.d @@ -0,0 +1,14 @@ +#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+ +3 +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+ +3 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar6 + +[0-9]+: 0+ +3 +NOTYPE +LOCAL +DEFAULT +[0-9]+ +bar7 +#pass diff --git a/gas/testsuite/gas/elf/size.s b/gas/testsuite/gas/elf/size.s new file mode 100644 index 0000000..9286556 --- /dev/null +++ b/gas/testsuite/gas/elf/size.s @@ -0,0 +1,23 @@ +.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 |