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.
This commit is contained in:
Fangrui Song 2022-04-04 08:43:50 -07:00 committed by Fangrui Song
parent edbc15e6c4
commit 867b8c308a
4 changed files with 48 additions and 14 deletions

View File

@ -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))));

View File

@ -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

View File

@ -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

View File

@ -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