aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFangrui Song <i@maskray.me>2022-04-04 08:43:50 -0700
committerFangrui Song <maskray@google.com>2022-04-04 08:43:50 -0700
commit867b8c308a40e79f90db4051cafc273f00f881fb (patch)
treea7c39c1860484bf1064db52fbdd2b340d2181036
parentedbc15e6c4f463173b25b1f8042346c2e1a78602 (diff)
downloadfsf-binutils-gdb-867b8c308a40e79f90db4051cafc273f00f881fb.zip
fsf-binutils-gdb-867b8c308a40e79f90db4051cafc273f00f881fb.tar.gz
fsf-binutils-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.c24
-rw-r--r--gas/testsuite/gas/elf/elf.exp1
-rw-r--r--gas/testsuite/gas/elf/size.d14
-rw-r--r--gas/testsuite/gas/elf/size.s23
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