[PATCH v2] LTO: Properly check wrapper symbol

[H.J. Lu]

On Tue, Jul 9, 2024, 3:21 PM Alan Modra <amodra@gmail.com> wrote:

> On Mon, Jul 08, 2024 at 12:59:20AM -0700, H.J. Lu wrote:
> > Add wrapper_symbol to bfd_link_hash_entry and set it to true for wrapper
> > symbol. Set wrap_status to wrapper if wrapper_symbol is true in LTO.
> >
> > bfd/
> >
> >       PR ld/31956
> >       * linker.c (bfd_wrapped_link_hash_lookup): Set wrapper_symbol
> >       for wrapper symbol.
> >
> > include/
> >
> >       PR ld/31956
> >       * bfdlink.h (bfd_link_hash_entry): Add wrapper_symbol.
> >
> > ld/
> >
> >       PR ld/31956
> >       * plugin.c (get_symbols): Set wrap_status to wrapper if
> >       wrapper_symbol is set.
> >       * testsuite/ld-plugin/lto.exp: Run PR ld/31956 tests.
> >       * testsuite/ld-plugin/pr31956a.c: New file.
> >       * testsuite/ld-plugin/pr31956b.c: Likewise.
> >
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> > ---
> >  bfd/linker.c                      |  2 ++
> >  include/bfdlink.h                 |  3 +++
> >  ld/plugin.c                       |  4 +---
> >  ld/testsuite/ld-plugin/lto.exp    | 16 ++++++++++++++++
> >  ld/testsuite/ld-plugin/pr31956a.c |  3 +++
> >  ld/testsuite/ld-plugin/pr31956b.c | 23 +++++++++++++++++++++++
> >  6 files changed, 48 insertions(+), 3 deletions(-)
> >  create mode 100644 ld/testsuite/ld-plugin/pr31956a.c
> >  create mode 100644 ld/testsuite/ld-plugin/pr31956b.c
> >
> > diff --git a/bfd/linker.c b/bfd/linker.c
> > index 111deecf55d..21009a838bc 100644
> > --- a/bfd/linker.c
> > +++ b/bfd/linker.c
> > @@ -573,6 +573,8 @@ bfd_wrapped_link_hash_lookup (bfd *abfd,
> >         strcat (n, WRAP);
> >         strcat (n, l);
> >         h = bfd_link_hash_lookup (info->hash, n, create, true, follow);
> > +       if (h != NULL)
> > +         h->wrapper_symbol = true;
> >         free (n);
> >         return h;
> >       }
> > diff --git a/include/bfdlink.h b/include/bfdlink.h
> > index 015370d268f..f802ec627ef 100644
> > --- a/include/bfdlink.h
> > +++ b/include/bfdlink.h
> > @@ -117,6 +117,9 @@ struct bfd_link_hash_entry
> >    /* The symbol, SYM, is referenced by __real_SYM in an object file.  */
> >    unsigned int ref_real : 1;
> >
> > +  /* The symbol is a wrapper symbol, __wrap_SYM.  */
> > +  unsigned int wrapper_symbol : 1;
> > +
> >    /* Symbol is a built-in define.  These will be overridden by PROVIDE
> >       in a linker script.  */
> >    unsigned int linker_def : 1;
> > diff --git a/ld/plugin.c b/ld/plugin.c
> > index 8107841407a..6990fcc61b1 100644
> > --- a/ld/plugin.c
> > +++ b/ld/plugin.c
> > @@ -780,9 +780,7 @@ get_symbols (const void *handle, int nsyms, struct
> ld_plugin_symbol *syms,
> >         if (blhe && link_info.wrap_hash != NULL)
> >           {
> >             /* Check if a symbol is a wrapper symbol.  */
> > -           struct bfd_link_hash_entry *unwrap
> > -             = unwrap_hash_lookup (&link_info, (bfd *) abfd, blhe);
> > -           if (unwrap && unwrap != h)
> > +           if (h->wrapper_symbol)
> >               wrap_status = wrapper;
> >            }
> >       }
>
> There is no need to check wrap_hash here.  This could be
>           if (blhe && blhe->wrapper_symbol)
>             wrap_status = wrapper;
>

Fixed in v3.


> The patch is OK, but please do add to the git log an explanation as to
> why using unwrap_hash_lookup fails.
>

Added a note in v3 which I am going to
check in.

Thanks.


> > diff --git a/ld/testsuite/ld-plugin/lto.exp
> b/ld/testsuite/ld-plugin/lto.exp
> > index 7b4b3492a2f..9476caf7ab4 100644
> > --- a/ld/testsuite/ld-plugin/lto.exp
> > +++ b/ld/testsuite/ld-plugin/lto.exp
> > @@ -530,6 +530,22 @@ set lto_link_elf_tests [list \
> >     {} \
> >     "pr29086" \
> >    ] \
> > +  [list \
> > +   "PR ld/31956 (a)" \
> > +   "-Wl,--wrap=parse_line" \
> > +   "-O2 -flto" \
> > +   {pr31956a.c pr31956b.c} \
> > +   {} \
> > +   "pr31956a" \
> > +  ] \
> > +  [list \
> > +   "PR ld/31956 (b)" \
> > +   "-Wl,--wrap=parse_line" \
> > +   "-O2 -flto" \
> > +   {pr31956b.c pr31956a.c} \
> > +   {} \
> > +   "pr31956b" \
> > +  ] \
> >    [list \
> >     "Build pr30281.so" \
> >     "-shared -Wl,--version-script,pr30281.t \
> > diff --git a/ld/testsuite/ld-plugin/pr31956a.c
> b/ld/testsuite/ld-plugin/pr31956a.c
> > new file mode 100644
> > index 00000000000..48df8c537fb
> > --- /dev/null
> > +++ b/ld/testsuite/ld-plugin/pr31956a.c
> > @@ -0,0 +1,3 @@
> > +extern void parse_line(void);
> > +void _cmocka_run_group_tests(void *) {}
> > +void argv_parse_cmd(void) { parse_line(); }
> > diff --git a/ld/testsuite/ld-plugin/pr31956b.c
> b/ld/testsuite/ld-plugin/pr31956b.c
> > new file mode 100644
> > index 00000000000..1b069318257
> > --- /dev/null
> > +++ b/ld/testsuite/ld-plugin/pr31956b.c
> > @@ -0,0 +1,23 @@
> > +struct CMUnitTest {
> > +  void *test_func;
> > +};
> > +
> > +extern void _cmocka_run_group_tests(void *);
> > +
> > +extern void argv_parse_cmd(void);
> > +void __wrap_parse_line(void) {};
> > +
> > +void foo (void) {
> > +  argv_parse_cmd();
> > +}
> > +
> > +struct CMUnitTest main_tests = {
> > +   foo
> > +};
> > +
> > +int
> > +main (void)
> > +{
> > +  _cmocka_run_group_tests (&main_tests);
> > +  return 0;
> > +}
> > --
> > 2.45.2
>
> --
> Alan Modra
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20240709/8e6796e5/attachment-0001.htm>