[PATCH v3] LTO: Properly check wrapper symbol

[PATCH v3] LTO: Properly check wrapper symbol

[H.J. Lu]

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.

Note: Calling unwrap_hash_lookup to check for the wrapper symbol works
only when there is a definition for the wrapped symbol since references
to the wrapped symbol have been redirected to the wrapper symbol.

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                       | 11 +++--------
 ld/testsuite/ld-plugin/lto.exp    | 16 ++++++++++++++++
 ld/testsuite/ld-plugin/pr31956a.c |  3 +++
 ld/testsuite/ld-plugin/pr31956b.c | 23 +++++++++++++++++++++++
 6 files changed, 50 insertions(+), 8 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..03ee9880d10 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -777,14 +777,9 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
       if (syms[n].def != LDPK_UNDEF && syms[n].def != LDPK_WEAKUNDEF)
 	{
 	  blhe = h;
-	  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)
-		wrap_status = wrapper;
-	     }
+	  /* Check if a symbol is a wrapper symbol.  */
+	  if (blhe && blhe->wrapper_symbol)
+	    wrap_status = wrapper;
 	}
       else
 	{
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

Re: [PATCH v3] LTO: Properly check wrapper symbol

[Alan Modra]

On Tue, Jul 09, 2024 at 01:30:19AM -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.
> 
> Note: Calling unwrap_hash_lookup to check for the wrapper symbol works
> only when there is a definition for the wrapped symbol since references
> to the wrapped symbol have been redirected to the wrapper symbol.

I'm not suggesting you revert your patch, fewer hash lookups in good,
just showing an alternative here that works.

diff --git a/ld/plugin.c b/ld/plugin.c
index 8107841407a..7672fbc6e08 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -782,7 +782,7 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms,
 	      /* 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 (unwrap != h)
 		wrap_status = wrapper;
 	     }
 	}

-- 
Alan Modra