[PATCH v2] elf: Avoid possible duplicated local symbol names

[H.J. Lu]

On Wed, May 5, 2021 at 2:36 PM Fangrui Song <i@maskray.me> wrote:
>
> On 2021-05-05, H.J. Lu via Binutils wrote:
> >On Wed, May 5, 2021 at 11:31 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >>
> >> On Wed, May 5, 2021 at 8:48 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >> >
> >> > When appending ".COUNT" to duplicated local symbol names of "XXX",
> >> > increment COUNT if there is an existing local symbol name of "XXX.COUNT".
> >> >
> >> > bfd/
> >> >
> >> >         PR ld/27825
> >> >         * elflink.c (elf_link_output_symstrtab): Avoid conflicts with
> >> >         local symbol names of "XXX.COUNT".
> >> >
> >> > ld/
> >> >
> >> >         PR ld/27825
> >> >         * testsuite/ld-elf/pr27825.d: New file.
> >> >         * testsuite/ld-elf/pr27825a.s: Likewise.
> >> >         * testsuite/ld-elf/pr27825b.s: Likewise.
> >> > ---
> >> >  bfd/elflink.c                  | 31 ++++++++++++++++++++++++++++---
> >> >  ld/testsuite/ld-elf/pr27825.d  | 18 ++++++++++++++++++
> >> >  ld/testsuite/ld-elf/pr27825a.s |  7 +++++++
> >> >  ld/testsuite/ld-elf/pr27825b.s |  5 +++++
> >> >  4 files changed, 58 insertions(+), 3 deletions(-)
> >> >  create mode 100644 ld/testsuite/ld-elf/pr27825.d
> >> >  create mode 100644 ld/testsuite/ld-elf/pr27825a.s
> >> >  create mode 100644 ld/testsuite/ld-elf/pr27825b.s
> >> >
> >> > diff --git a/bfd/elflink.c b/bfd/elflink.c
> >> > index cb38a025349..ac46585f0ab 100644
> >> > --- a/bfd/elflink.c
> >> > +++ b/bfd/elflink.c
> >> > @@ -9845,22 +9845,47 @@ elf_link_output_symstrtab (void *finf,
> >> >                   /* Append ".COUNT" to duplicated local symbols.  */
> >> >                   size_t count_len;
> >> >                   size_t base_len = lh->size;
> >> > -                 char buf[30];
> >> > -                 sprintf (buf, "%lx", lh->count);
> >> > +                 char buf[20];
> >> > +                 if (snprintf (buf, sizeof (buf), "%lx", lh->count)
> >> > +                     >= (int) sizeof (buf))
> >> > +                   return 0;
> >> >                   if (!base_len)
> >> >                     {
> >> >                       base_len = strlen (name);
> >> >                       lh->size = base_len;
> >> >                     }
> >> >                   count_len = strlen (buf);
> >> > +                 /* NB: Allocate the extra suffix buffer for possible
> >> > +                    change.  */
> >> >                   versioned_name = bfd_alloc (flinfo->output_bfd,
> >> > -                                             base_len + count_len + 2);
> >> > +                                             base_len + sizeof (buf)
> >> > +                                             + 2);
> >> >                   if (versioned_name == NULL)
> >> >                     return 0;
> >> >                   memcpy (versioned_name, name, base_len);
> >> >                   versioned_name[base_len] = '.';
> >> >                   memcpy (versioned_name + base_len + 1, buf,
> >> >                           count_len + 1);
> >> > +                 do
> >> > +                   {
> >> > +                     /* Avoid conflicts with local symbol names of
> >> > +                        "XXX.COUNT".  */
> >> > +                     struct local_hash_entry *lvh
> >> > +                       = (struct local_hash_entry *) bfd_hash_lookup
> >> > +                       (&flinfo->local_hash_table, versioned_name,
> >> > +                        false, false);
> >> > +                     if (lvh == NULL)
> >> > +                       break;
> >> > +                     lh->count++;
> >> > +                     if (snprintf (buf, sizeof (buf), "%lx",
> >> > +                                   lh->count) >= (int) sizeof (buf))
> >> > +                       return 0;
> >> > +                     count_len = strlen (buf);
> >> > +                     /* NB: Use the existing suffix buffer.  */
> >> > +                     memcpy (versioned_name + base_len + 1, buf,
> >> > +                             count_len + 1);
> >> > +                   }
> >> > +                 while (1);
> >> >                 }
> >> >               lh->count++;
> >> >               break;
> >
> >> This scheme doesn't work.
> >>
> >
> >This one works.
> >
> >--
> >H.J.
>
> I don't read the implementation.
>
> If bar.2.1 already exists, can the implementation correctly rename bar.2 (to bar.2.2) ?

Is this what you have in mind?

[hjl@gnu-cfl-2 pr27825-2]$ cat pr27825a.s
.text
.globl _start
_start:
bar.2:
.nop
[hjl@gnu-cfl-2 pr27825-2]$ cat pr27825b.s
.text
bar.2:
.nop
[hjl@gnu-cfl-2 pr27825-2]$ cat pr27825c.s
.text
bar.1.1:
bar.2.1:
.nop
[hjl@gnu-cfl-2 pr27825-2]$ make
as   -o pr27825a.o pr27825a.s
as   -o pr27825b.o pr27825b.s
as   -o pr27825c.o pr27825c.s
./ld --emit-relocs -z unique-symbol -e _start -o x pr27825a.o
pr27825b.o pr27825c.o
./nm x
0000000000401002 t bar.1.1
0000000000401000 t bar.2
0000000000401001 t bar.2.1
0000000000401002 t bar.2.1.1
0000000000402000 B __bss_start
0000000000402000 D _edata
0000000000402000 B _end
0000000000401000 T _start
[hjl@gnu-cfl-2 pr27825-2]$


-- 
H.J.