Re: [PATCH] PR ld/21382: Handle symbol defined in IR and referenced in DSO

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Tue, Apr 18, 2017 at 5:48 PM, Alan Modra <amodra@gmail.com> wrote:
> On Fri, Apr 14, 2017 at 08:44:56AM -0700, H.J. Lu wrote:
>
>> +++ b/bfd/elflink.c
>> +           /* Preserve re_def_dynamic_ref so that this symbol will
>
> Comment typo.
>
>> +++ b/include/bfdlink.h
>> +  /* Symbol is defined in an IR object and referenced in a dynamic
>> +     object.  */
>> +  unsigned int ir_def_dynamic_ref : 1;
>
> The comment here isn't completely true.  The flag will only be set if
> the dynamic object is linked *after* the IR object, won't it?  I think
> it is worth stating that, or fixing your plugin_notice change to also
> cover the case where the IR object is linked after a dynamic object.
>

Here is the updated patch.  I updated comments and renamed
ir_def_dynamic_ref to dynamic_ref_after_ir_def.


-- 
H.J.

From 2949ed612849ff9f936c5fb2570f67cf8a5cc553 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 13 Apr 2017 12:15:36 -0700
Subject: [PATCH] Handle symbol defined in IR and referenced in DSO

We need to make an IR symbol visible if it is defined in an IR object
and referenced in a dynamic object.  When --as-needed is used, since
linker removes the IR symbol reference of the dynamic object if the
dynamic object isn't needed in the first pass, the IR definition isn't
visible to the dynamic object even if the dynamic object becomes needed
in the second pass.  Add dynamic_ref_after_ir_def to bfd_link_hash_entry
to track IR symbol which is defined in an IR object and later referenced
in a dynamic object.  dynamic_ref_after_ir_def is preserved when restoring
the symbol table for unneeded dynamic object.

bfd/

	PR ld/21382
	* elflink.c (elf_link_add_object_symbols): Preserve
	re_def_dynamic_ref when restoring the symbol table for unneeded
	dynamic object.

include/

	PR ld/21382
	* bfdlink.h (bfd_link_hash_entry): Add dynamic_ref_after_ir_def.

ld/

	PR ld/21382
	* plugin.c (is_visible_from_outside): Symbol may be visible
	from outside if dynamic_ref_after_ir_def is set.
	(plugin_notice): Set dynamic_ref_after_ir_def if the symbol is
	defined in an IR object and referenced in a dynamic object.
	* testsuite/ld-plugin/lto.exp: Run PR ld/21382 tests.
	* testsuite/ld-plugin/pr21382a.c: New file.
	* testsuite/ld-plugin/pr21382b.c: Likewise.
---
 bfd/elflink.c                     |  6 ++++++
 include/bfdlink.h                 |  4 ++++
 ld/plugin.c                       | 23 +++++++++++++++++------
 ld/testsuite/ld-plugin/lto.exp    |  9 +++++++++
 ld/testsuite/ld-plugin/pr21382a.c | 17 +++++++++++++++++
 ld/testsuite/ld-plugin/pr21382b.c |  7 +++++++
 6 files changed, 60 insertions(+), 6 deletions(-)
 create mode 100644 ld/testsuite/ld-plugin/pr21382a.c
 create mode 100644 ld/testsuite/ld-plugin/pr21382b.c

diff --git a/bfd/elflink.c b/bfd/elflink.c
index 0add01b..972b6f8 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4932,6 +4932,7 @@ error_free_dyn:
 	  struct elf_link_hash_entry *h;
 	  bfd_size_type size;
 	  unsigned int alignment_power;
+	  unsigned int dynamic_ref_after_ir_def;
 
 	  for (p = htab->root.table.table[i]; p != NULL; p = p->next)
 	    {
@@ -4953,6 +4954,10 @@ error_free_dyn:
 		  size = 0;
 		  alignment_power = 0;
 		}
+	      /* Preserve dynamic_ref_after_ir_def so that this symbol
+		 will be exported when the dynamic lib becomes needed
+		 in the second pass.  */
+	      dynamic_ref_after_ir_def = h->root.dynamic_ref_after_ir_def;
 	      memcpy (p, old_ent, htab->root.table.entsize);
 	      old_ent = (char *) old_ent + htab->root.table.entsize;
 	      h = (struct elf_link_hash_entry *) p;
@@ -4969,6 +4974,7 @@ error_free_dyn:
 		  if (alignment_power > h->root.u.c.p->alignment_power)
 		    h->root.u.c.p->alignment_power = alignment_power;
 		}
+	      h->root.dynamic_ref_after_ir_def = dynamic_ref_after_ir_def;
 	    }
 	}
 
diff --git a/include/bfdlink.h b/include/bfdlink.h
index 18dea0c..cb4bad9 100644
--- a/include/bfdlink.h
+++ b/include/bfdlink.h
@@ -104,6 +104,10 @@ struct bfd_link_hash_entry
      IR object file.  */
   unsigned int non_ir_ref : 1;
 
+  /* Symbol is referenced in a dynamic object after it has been defined
+     in an IR object.  */
+  unsigned int dynamic_ref_after_ir_def : 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 03e2e69..164b5db 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -629,7 +629,9 @@ is_visible_from_outside (struct ld_plugin_symbol *lsym,
 
   if (bfd_link_relocatable (&link_info))
     return TRUE;
-  if (link_info.export_dynamic || bfd_link_dll (&link_info))
+  if (blhe->dynamic_ref_after_ir_def
+      || link_info.export_dynamic
+      || bfd_link_dll (&link_info))
     {
       /* Check if symbol is hidden by version script.  */
       if (bfd_hide_sym_by_version (link_info.version_info,
@@ -1316,12 +1318,21 @@ plugin_notice (struct bfd_link_info *info,
       /* If this is a ref, set non_ir_ref.  */
       else if (bfd_is_und_section (section))
 	{
+	   if (h->type == bfd_link_hash_defweak
+	       || h->type == bfd_link_hash_defined)
+	     {
+	       /* Check if the symbol is referenced in a dynamic object
+		  after it has been defined in an IR object.  */
+	       if ((abfd->flags & DYNAMIC) != 0
+		   && is_ir_dummy_bfd (h->u.def.section->owner))
+		 h->dynamic_ref_after_ir_def = TRUE;
+	     }
 	  /* Replace the undefined dummy bfd with the real one.  */
-	  if ((h->type == bfd_link_hash_undefined
-	       || h->type == bfd_link_hash_undefweak)
-	      && (h->u.undef.abfd == NULL
-		  || (h->u.undef.abfd->flags & BFD_PLUGIN) != 0))
-	    h->u.undef.abfd = abfd;
+	   else if ((h->type == bfd_link_hash_undefined
+		     || h->type == bfd_link_hash_undefweak)
+		    && (h->u.undef.abfd == NULL
+			|| (h->u.undef.abfd->flags & BFD_PLUGIN) != 0))
+	     h->u.undef.abfd = abfd;
 	  h->non_ir_ref = TRUE;
 	}
 
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index 80c084b..f0bc345 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -289,6 +289,12 @@ set lto_link_elf_tests [list \
   [list "PR ld/14918" \
    "-flto" "-flto" \
    {pr14918.c} {{"readelf" {-d --wide} "pr14918.d"}} "pr14918.exe" "c"] \
+  [list "Build pr21382a.o" \
+   "" "-O2 -flto" \
+   {pr21382a.c} {} "" "c"] \
+  [list "Build pr21382.so" \
+   "-shared" "-O2 -fpic" \
+   {pr21382b.c} {} "pr21382.so" "c"] \
 ]
 
 # Check final symbols in executables.
@@ -387,6 +393,9 @@ set lto_run_elf_shared_tests [list \
   [list "LTO 7" \
    "-O2 -flto -fuse-linker-plugin tmpdir/lto-7b.o tmpdir/lto-7c.o tmpdir/lto-7a.o -Wl,--no-as-needed tmpdir/liblto-7.so" "" \
    {dummy.c} "lto-7.exe" "lto-7.out" "" "c"] \
+  [list "Run pr21382" \
+   "-O2 -flto -fuse-linker-plugin -Wl,--as-needed tmpdir/pr21382a.o tmpdir/pr21382.so" "" \
+   {dummy.c} "pr21382.exe" "pass.out" "" "c"] \
 ]
 
 # LTO run-time tests for ELF
diff --git a/ld/testsuite/ld-plugin/pr21382a.c b/ld/testsuite/ld-plugin/pr21382a.c
new file mode 100644
index 0000000..09b9d75
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr21382a.c
@@ -0,0 +1,17 @@
+#include <stdio.h>
+
+extern void y (void);
+
+void
+x (void)
+{
+  printf ("PASS\n");
+}
+
+
+int
+main (void)
+{
+  y ();
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr21382b.c b/ld/testsuite/ld-plugin/pr21382b.c
new file mode 100644
index 0000000..c5b74a9
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr21382b.c
@@ -0,0 +1,7 @@
+extern void x (void);
+
+void
+y (void)
+{
+  x ();
+}
-- 
2.9.3