Re: [PATCH] PR ld/20276: Set non_ir_ref on common symbol

[Alan Modra <amodra at gmail dot com>]

On Sat, Jun 18, 2016 at 06:45:13PM -0700, H.J. Lu wrote:
> Linker backend sets non_ir_ref during relocation scan.  When relocations
> are scanned after opening all input files, get_symbols in plugin.c
> returns the incorrect symbol resulotion since non_ir_ref isn't set on
> common symbols.  plugin_notice should set non_ir_ref on common symbols.
> Also, we shouldn't check alignment on symbol from plugin dummy input.

I've committed this slightly different patch for you, which fixes what
looks like merge problems in the testsuite, and rearranges the
plugin.c patch plus gives a better explanatory comment.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index 187a049..8ab9a1f 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2016-06-20  H.J. Lu  <hongjiu.lu@intel.com>
+
+	PR ld/20276
+	* elflink.c (elf_link_add_object_symbols): Don't check alignment
+	on symbol from plugin dummy input.
+
 2016-06-18  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* bfd.c (bfd_plugin_format): Rename bfd_plugin_uknown to
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 6d591de..d4e8db6 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -4556,7 +4556,8 @@ error_free_dyn:
 
 	      symbol_align = ffs (h->root.u.def.value) - 1;
 	      if (h->root.u.def.section->owner != NULL
-		  && (h->root.u.def.section->owner->flags & DYNAMIC) == 0)
+		  && (h->root.u.def.section->owner->flags
+		       & (DYNAMIC | BFD_PLUGIN)) == 0)
 		{
 		  normal_align = h->root.u.def.section->alignment_power;
 		  if (normal_align > symbol_align)
diff --git a/ld/ChangeLog b/ld/ChangeLog
index b9b9c3e..cd161ab 100644
--- a/ld/ChangeLog
+++ b/ld/ChangeLog
@@ -1,3 +1,15 @@
+2016-06-20  H.J. Lu  <hongjiu.lu@intel.com>
+	    Alan Modra  <amodra@gmail.com>
+
+	PR ld/20276
+	* plugin.c (plugin_notice): Set non_ir_ref on common symbols.
+	* testsuite/ld-plugin/lto.exp (lto_link_tests): Add test for
+	PR ld/20276.
+	(lto_run_tests): Likewise.
+	* testsuite/ld-plugin/pass.out: New file.
+	* testsuite/ld-plugin/pr20276a.c: Likewise.
+	* testsuite/ld-plugin/pr20276b.c: Likewise.
+
 2016-06-18  H.J. Lu  <hongjiu.lu@intel.com>
 
 	* plugin.c (plugin_object_p): Replace bfd_plugin_uknown
diff --git a/ld/plugin.c b/ld/plugin.c
index cb61318..4c161d1 100644
--- a/ld/plugin.c
+++ b/ld/plugin.c
@@ -1325,20 +1325,30 @@ plugin_notice (struct bfd_link_info *info,
 	  h->non_ir_ref = TRUE;
 	}
 
-      /* Otherwise, it must be a new def.  Ensure any symbol defined
-	 in an IR dummy BFD takes on a new value from a real BFD.
-	 Weak symbols are not normally overridden by a new weak
-	 definition, and strong symbols will normally cause multiple
-	 definition errors.  Avoid this by making the symbol appear
-	 to be undefined.  */
-      else if (((h->type == bfd_link_hash_defweak
-		 || h->type == bfd_link_hash_defined)
-		&& is_ir_dummy_bfd (sym_bfd = h->u.def.section->owner))
-	       || (h->type == bfd_link_hash_common
-		   && is_ir_dummy_bfd (sym_bfd = h->u.c.p->section->owner)))
+      /* Otherwise, it must be a new def.  */
+      else
 	{
-	  h->type = bfd_link_hash_undefweak;
-	  h->u.undef.abfd = sym_bfd;
+	  /* A common symbol should be merged with other commons or
+	     defs with the same name.  In particular, a common ought
+	     to be overridden by a def in a -flto object.  In that
+	     sense a common is also a ref.  */
+	  if (bfd_is_com_section (section))
+	    h->non_ir_ref = TRUE;
+
+	  /* Ensure any symbol defined in an IR dummy BFD takes on a
+	     new value from a real BFD.  Weak symbols are not normally
+	     overridden by a new weak definition, and strong symbols
+	     will normally cause multiple definition errors.  Avoid
+	     this by making the symbol appear to be undefined.  */
+	  if (((h->type == bfd_link_hash_defweak
+		|| h->type == bfd_link_hash_defined)
+	       && is_ir_dummy_bfd (sym_bfd = h->u.def.section->owner))
+	      || (h->type == bfd_link_hash_common
+		  && is_ir_dummy_bfd (sym_bfd = h->u.c.p->section->owner)))
+	    {
+	      h->type = bfd_link_hash_undefweak;
+	      h->u.undef.abfd = sym_bfd;
+	    }
 	}
     }
 
diff --git a/ld/testsuite/ld-plugin/lto.exp b/ld/testsuite/ld-plugin/lto.exp
index 7a13abb..7743719 100644
--- a/ld/testsuite/ld-plugin/lto.exp
+++ b/ld/testsuite/ld-plugin/lto.exp
@@ -183,6 +183,12 @@ set lto_link_tests [list \
   [list "PR ld/19317 (1)" \
    "$plug_opt" "-flto $lto_no_fat" \
    {pr19317.c} {} "libpr19317.a"] \
+  [list "Build pr20276a.o" \
+   "" "-fno-lto" \
+   {pr20276a.c}] \
+  [list "Build pr20276b.o" \
+   "$plug_opt" "-flto $lto_no_fat" \
+   {pr20276b.c}] \
 ]
 
 if { [at_least_gcc_version 4 7] } {
@@ -332,6 +338,9 @@ set lto_run_tests [list \
   [list "PR ld/19317 (3)" \
    "-O2 -flto tmpdir/pr19317-r.o" "" \
    {dummy.c} "pr19317.exe" "pr19317.out" "-flto -O2" "c"] \
+  [list "Run pr20276" \
+   "-O2 -flto tmpdir/pr20276a.o tmpdir/pr20276b.o" "" \
+   {dummy.c} "pr20276" "pass.out" "-flto -O2" "c"] \
 ]
 
 if { [at_least_gcc_version 4 7] } {
diff --git a/ld/testsuite/ld-plugin/pass.out b/ld/testsuite/ld-plugin/pass.out
new file mode 100644
index 0000000..7ef22e9
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pass.out
@@ -0,0 +1 @@
+PASS
diff --git a/ld/testsuite/ld-plugin/pr20276a.c b/ld/testsuite/ld-plugin/pr20276a.c
new file mode 100644
index 0000000..0b37bc7
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr20276a.c
@@ -0,0 +1,12 @@
+#include <stdio.h>
+
+int global_var;
+extern void abort ();
+
+int main(void)
+{
+  if (global_var != 20)
+    abort ();
+  printf ("PASS\n");
+  return 0;
+}
diff --git a/ld/testsuite/ld-plugin/pr20276b.c b/ld/testsuite/ld-plugin/pr20276b.c
new file mode 100644
index 0000000..2ecbc2c
--- /dev/null
+++ b/ld/testsuite/ld-plugin/pr20276b.c
@@ -0,0 +1 @@
+int global_var = 20;

-- 
Alan Modra
Australia Development Lab, IBM