Re: avoiding undesirable undef[weak] syms after relocatable linking

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

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

On Sat, Apr 13, 2024 at 6:52 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Sat, Apr 13, 2024 at 3:15 AM Alexandre Oliva <oliva@adacore.com> wrote:
> >
> > Hello, Nick,
> >
> > On Apr 12, 2024, Nick Clifton <nickc@redhat.com> wrote:
> >
> > > Hi Alex,
> > >> And here's how to trigger the problem:
> > >> as t.s -o t.o && ar cr t.a t.o && ranlib t.a && as m.s -o m.o &&
> > >> ld m.o -o m -r && nm m | grep bar
> > >> w bar
> >
> > > Which version of the binutils are you using ?  And for which target ?
> >
> > Originally, 2.42 snapshots on multiple vxworks targets, but I'd tried on
> > x86_64-linux-gnu as well.  Unfortunately, I see now that I made a
> > mistake in the command posted by email: somehow I dropped t.a from the
> > link command, presumably while editing it to remove local artifacts.
> > Please accept my apologies.  Here's the correct command, now (not)
> > adjusted, so that it runs from the top of a binutils build tree.  The
> > (unmodified) [tm].s files are attached.
> >
> > $ gas/as-new t.s -o t.o && binutils/ar cr t.a t.o && binutils/ranlib t.a && gas/as-new m.s -o m.o && ld/ld-new m.o t.a -o m -r && binutils/nm-new m | grep bar
> >                  w bar
> > $ ld/ld-new --version
> > GNU ld (GNU Binutils) 2.42.50.20240413
> > [...]
> > $ git log
> > commit 4ad25f3bed6bc4c010962b11fde1c70ce8c22cae
> > [...]
> >
> >
>
> Please open a bug report.
>
> Thanks.
>

This should fix it.  I will submit a complete patch after the bug
report is opened.

-- 
H.J.

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 4904 bytes --]

diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h
index ef5dcb55e72..92a0287d40e 100644
--- a/bfd/elf-bfd.h
+++ b/bfd/elf-bfd.h
@@ -232,6 +232,8 @@ struct elf_link_hash_entry
      a strong defined symbol alias.  U.ALIAS points to a list of aliases,
      the definition having is_weakalias clear.  */
   unsigned int is_weakalias : 1;
+  /* Symbol has a relocation.  */
+  unsigned int has_reloc : 1;
 
   /* String table index in .dynstr if this is a dynamic symbol.  */
   unsigned long dynstr_index;
diff --git a/bfd/elf-vxworks.c b/bfd/elf-vxworks.c
index 4c172cd4115..36e5540f9c0 100644
--- a/bfd/elf-vxworks.c
+++ b/bfd/elf-vxworks.c
@@ -172,35 +172,39 @@ elf_vxworks_emit_relocs (bfd *output_bfd,
 	   irela += bed->s->int_rels_per_ext_rel,
 	     hash_ptr++)
 	{
-	  if (*hash_ptr
-	      && (*hash_ptr)->def_dynamic
-	      && !(*hash_ptr)->def_regular
-	      && ((*hash_ptr)->root.type == bfd_link_hash_defined
-		  || (*hash_ptr)->root.type == bfd_link_hash_defweak)
-	      && (*hash_ptr)->root.u.def.section->output_section != NULL)
+	  if (*hash_ptr)
 	    {
-	      /* This is a relocation from an executable or shared
-		 library against a symbol in a different shared
-		 library.  We are creating a definition in the output
-		 file but it does not come from any of our normal (.o)
-		 files. ie. a PLT stub.  Normally this would be a
-		 relocation against against SHN_UNDEF with the VMA of
-		 the PLT stub.  This upsets the VxWorks loader.
-		 Convert it to a section-relative relocation.  This
-		 gets some other symbols (for instance .dynbss), but
-		 is conservatively correct.  */
-	      for (j = 0; j < bed->s->int_rels_per_ext_rel; j++)
+	      (*hash_ptr)->has_reloc = 1;
+	      if ((*hash_ptr)->def_dynamic
+		  && !(*hash_ptr)->def_regular
+		  && ((*hash_ptr)->root.type == bfd_link_hash_defined
+		      || (*hash_ptr)->root.type == bfd_link_hash_defweak)
+		  && (*hash_ptr)->root.u.def.section->output_section != NULL)
 		{
-		  asection *sec = (*hash_ptr)->root.u.def.section;
-		  int this_idx = sec->output_section->target_index;
-
-		  irela[j].r_info
-		    = ELF32_R_INFO (this_idx, ELF32_R_TYPE (irela[j].r_info));
-		  irela[j].r_addend += (*hash_ptr)->root.u.def.value;
-		  irela[j].r_addend += sec->output_offset;
+		  /* This is a relocation from an executable or shared
+		     library against a symbol in a different shared
+		     library.  We are creating a definition in the output
+		     file but it does not come from any of our normal (.o)
+		     files. ie. a PLT stub.  Normally this would be a
+		     relocation against against SHN_UNDEF with the VMA of
+		     the PLT stub.  This upsets the VxWorks loader.
+		     Convert it to a section-relative relocation.  This
+		     gets some other symbols (for instance .dynbss), but
+		     is conservatively correct.  */
+		  for (j = 0; j < bed->s->int_rels_per_ext_rel; j++)
+		    {
+		      asection *sec = (*hash_ptr)->root.u.def.section;
+		      int this_idx = sec->output_section->target_index;
+
+		      irela[j].r_info
+			= ELF32_R_INFO (this_idx,
+					ELF32_R_TYPE (irela[j].r_info));
+		      irela[j].r_addend += (*hash_ptr)->root.u.def.value;
+		      irela[j].r_addend += sec->output_offset;
+		    }
+		  /* Stop the generic routine adjusting this entry.  */
+		  *hash_ptr = NULL;
 		}
-	      /* Stop the generic routine adjusting this entry.  */
-	      *hash_ptr = NULL;
 	    }
 	}
     }
diff --git a/bfd/elflink.c b/bfd/elflink.c
index 321e3d5e2ff..6558de82fa2 100644
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -2942,8 +2942,7 @@ _bfd_elf_link_output_relocs (bfd *output_bfd,
 			     asection *input_section,
 			     Elf_Internal_Shdr *input_rel_hdr,
 			     Elf_Internal_Rela *internal_relocs,
-			     struct elf_link_hash_entry **rel_hash
-			       ATTRIBUTE_UNUSED)
+			     struct elf_link_hash_entry **rel_hash)
 {
   Elf_Internal_Rela *irela;
   Elf_Internal_Rela *irelaend;
@@ -2986,9 +2985,12 @@ _bfd_elf_link_output_relocs (bfd *output_bfd,
 		      * bed->s->int_rels_per_ext_rel);
   while (irela < irelaend)
     {
+      if (*rel_hash)
+	(*rel_hash)->has_reloc = 1;
       (*swap_out) (output_bfd, irela, erel);
       irela += bed->s->int_rels_per_ext_rel;
       erel += input_rel_hdr->sh_entsize;
+      rel_hash++;
     }
 
   /* Bump the counter, so that we know where to add the next set of
@@ -10737,8 +10739,9 @@ elf_link_output_extsym (struct bfd_hash_entry *bh, void *data)
   /* We don't want to output symbols that have never been mentioned by
      a regular file, or that we have been told to strip.  However, if
      h->indx is set to -2, the symbol is used by a reloc and we must
-     output it.  */
-  strip = false;
+     output it.  Always strip undefined weak symbols if they don't
+     have relocation.  */
+  strip = h->root.type == bfd_link_hash_undefweak && !h->has_reloc;
   if (h->indx == -2)
     ;
   else if ((h->def_dynamic