PE-COFF: Prefer weak external with defined fallback over null fallback

When two PE COFF weak externals for the same symbol are linked (both
C_NT_WEAK with aux records), the generic linker takes no action on the
second one (NOACT: weak undef meets existing weak undef).  This means
the first file's fallback alias always wins regardless of whether it
points to a real definition or to NULL.

This causes a problem when a weak declaration (fallback = NULL,
i.e. "resolve to NULL if nothing provides it") is linked before a
weak definition (fallback = actual function body).  The symbol resolves
to address 0 at runtime, causing a crash when called.

Fix this by comparing the fallback targets when two weak externals
meet: if the incoming weak external's fallback resolves to a defined
symbol and the existing one does not (or points to NULL),
update the aux record to use the better fallback.

This matches the behavior of lld after:
  https://github.com/llvm/llvm-project/commit/7ca5698b4c3698d06065e0941df7f23d72913d23

bfd/
	* cofflink.c (coff_link_add_symbols): When two PE COFF weak
	externals meet, prefer the one whose fallback alias resolves
	to a defined symbol.

diff --git a/bfd/cofflink.c b/bfd/cofflink.c
index eba0fad4c3f321f8185edba2292b35b319f23d41..75b4c47eacf188ff8c533e81564938bc589b6928 100644 (file)
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -542,6 +542,88 @@ coff_link_add_symbols (bfd *abfd,
                      (*sym_hash)->aux = alloc;
                    }
                }
+
+             /* When two PE COFF weak externals meet (both with aux
+                records specifying fallback aliases), prefer the one
+                whose fallback resolves to a defined symbol over one
+                whose fallback is undefined or NULL.  This
+                handles the case where a weak declaration (with a
+                fallback of NULL) is seen before a weak
+                definition (with a fallback of the actual function
+                body).  */
+             else if (IS_WEAK_EXTERNAL (abfd, sym)
+                      && sym.n_numaux > 0
+                      && (*sym_hash)->root.type == bfd_link_hash_undefweak
+                      && (*sym_hash)->symbol_class == C_NT_WEAK
+                      && (*sym_hash)->numaux == 1)
+               {
+                 /* Parse the incoming aux to get the fallback tagndx.  */
+                 union internal_auxent new_aux;
+                 unsigned long new_tagndx;
+                 unsigned long old_tagndx;
+                 struct coff_link_hash_entry *h2_new = NULL;
+                 struct coff_link_hash_entry *h2_old = NULL;
+                 bool new_is_real_fallback;
+                 bool old_is_unresolved_fallback;
+
+                 bfd_coff_swap_aux_in (abfd, esym + symesz, sym.n_type,
+                                       sym.n_sclass, 0, sym.n_numaux,
+                                       &new_aux);
+                 new_tagndx = new_aux.x_sym.x_tagndx.u32;
+
+                 if (new_tagndx < obj_raw_syment_count (abfd))
+                   h2_new = obj_coff_sym_hashes (abfd)[new_tagndx];
+
+                 old_tagndx = (*sym_hash)->aux->x_sym.x_tagndx.u32;
+                 if (old_tagndx
+                     < obj_raw_syment_count ((*sym_hash)->auxbfd))
+                   h2_old = obj_coff_sym_hashes
+                     ((*sym_hash)->auxbfd)[old_tagndx];
+
+                 /* Update if the new fallback is a real definition but
+                    the old one is not.  A weak declaration with no
+                    definition uses the COFF null symbol as its fallback.
+                    In the hash table that fallback looks like a defined
+                    absolute symbol with value zero, so treat that case as
+                    unresolved here.  */
+                 new_is_real_fallback
+                   = (h2_new != NULL
+                      && (h2_new->root.type == bfd_link_hash_defined
+                          || h2_new->root.type == bfd_link_hash_defweak)
+                      && !(bfd_is_abs_section (h2_new->root.u.def.section)
+                           && h2_new->root.u.def.value == 0));
+                 old_is_unresolved_fallback
+                   = (h2_old == NULL
+                      || h2_old->root.type == bfd_link_hash_undefined
+                      || h2_old->root.type == bfd_link_hash_undefweak
+                      || (h2_old->root.type == bfd_link_hash_defined
+                          && bfd_is_abs_section (h2_old->root.u.def.section)
+                          && h2_old->root.u.def.value == 0));
+
+                 if (new_is_real_fallback && old_is_unresolved_fallback)
+                   {
+                     union internal_auxent *alloc;
+                     unsigned int i;
+                     bfd_byte *eaux;
+                     union internal_auxent *iaux;
+
+                     (*sym_hash)->symbol_class = sym.n_sclass;
+                     (*sym_hash)->auxbfd = abfd;
+                     (*sym_hash)->numaux = sym.n_numaux;
+                     alloc = bfd_hash_allocate (&info->hash->table,
+                                                (sym.n_numaux
+                                                 * sizeof (*alloc)));
+                     if (alloc == NULL)
+                       goto error_return;
+                     for (i = 0, eaux = esym + symesz, iaux = alloc;
+                          i < sym.n_numaux;
+                          i++, eaux += symesz, iaux++)
+                       bfd_coff_swap_aux_in (abfd, eaux, sym.n_type,
+                                             sym.n_sclass, (int) i,
+                                             sym.n_numaux, iaux);
+                     (*sym_hash)->aux = alloc;
+                   }
+               }
            }
 
          if (classification == COFF_SYMBOL_PE_SECTION