[PATCH v3 12/18] PR ld/24600: BFD: Add general linker support for mapless archives

[Maciej W. Rozycki]

On Tue, 9 Dec 2025, Jan Beulich wrote:

> On 20.11.2025 11:56, Maciej W. Rozycki wrote:
> > --- binutils-gdb.orig/bfd/archive.c
> > +++ binutils-gdb/bfd/archive.c
> > @@ -811,7 +811,10 @@ _bfd_generic_get_elt_at_index (bfd *abfd
> >    carsym *entry;
> >  
> >    entry = bfd_ardata (abfd)->symdefs + sym_index;
> > -  return _bfd_get_elt_at_filepos (abfd, entry->u.file_offset, NULL);
> > +  if (bfd_ardata (abfd)->symdef_use_bfd)
> > +    return entry->u.abfd;
> > +  else
> > +    return _bfd_get_elt_at_filepos (abfd, entry->u.file_offset, NULL);
> 
> If I counted right, there are 5 changes following this pattern. Should there
> be a helper macro or inline function?

 That's `_bfd_get_elt_from_symdef' now since you asked for it.  I'm not 
convinced it's a clear gain, but have nothing against it either.  I've 
left the single instance of the `alpha_ecoff_get_elt_at_filepos' variant 
open-coded.

> 
> > @@ -961,6 +963,84 @@ bfd_generic_archive_p (bfd *abfd)
> >    return _bfd_no_cleanup;
> >  }
> >  
> > +/* Given archive ARCH and symbol map MAP counting ORL_COUNT entries
> > +   load the symbols for use by the archive.  */
> 
> Was this comment copied from somewhere and then not edited sufficiently?
> There's nothing ARCH would refer to ...

 An archive BFD argument is referred to inconsistently as either ABFD or 
ARCH throughout the file and I reckon I decided to switch to ARCH for new 
additions when documenting them and then didn't update code accordingly, 
possibly owing to a distraction.  Fixed in v4.

> > +static bool
> > +_bfd_load_armap (bfd *abfd, unsigned int elength ATTRIBUTE_UNUSED,
> > +		 struct orl *map, unsigned int orl_count,
> > +		 int stridx ATTRIBUTE_UNUSED)
> 
> ... among the parameters, and "load" also doesn't look to describe what
> is being done here. (This "load" aspect then extends to the function's
> name as well.)

 I maintain "load" is correct usage here: an archive map in the output 
format is loaded into internal data structures of the requested archive 
BFD (compare the Z80 CPU's LD instruction).

> > +  ardata->symdefs = (struct carsym *) bfd_alloc (abfd, symdef_size);
> 
> I don't think new code should have such casts anymore.

 Fair enough.

> > +  for (counter = 0, set = ardata->symdefs;
> > +       counter < ardata->symdef_count;
> > +       counter++, set++)
> > +    {
> > +      bfd_size_type namelen = strlen (*(map[counter].name)) + 1;
> > +      char *name = bfd_alloc (abfd, namelen);
> > +
> > +      if (name == NULL)
> > +	{
> > +	  bfd_set_error (bfd_error_no_memory);
> > +	  goto release_symdefs;
> > +	}
> > +
> > +      memcpy (name, *(map[counter].name), namelen);
> > +      set->name = name;
> 
> Use bfd_strdup() here to simplify things?

 Umm, no, `bfd_release' does not free memory allocated by `bfd_strdup', so 
that'd be a memory leak.

> For *(map[counter].name) (another use a few lines up, other similar uses
> elsewhere), may I ask to omit the parentheses? They only hamper readability
> imo, without any benefit that I could see.

 OK.

> > @@ -6292,7 +6301,9 @@ elf_link_add_archive_symbols (bfd *abfd,
> >  
> >  	  if (included[i])
> >  	    continue;
> > -	  if (symdef->u.file_offset == last)
> > +	  if (bfd_ardata (abfd)->symdef_use_bfd
> > +	      ? symdef->u.abfd == last.abfd
> > +	      : symdef->u.file_offset == last.file_offset)
> 
> Such comparisons also look to repeat - same question again towards having
> a helper.

 OK.  Now `_bfd_elt_eq' in v4 along with `_bfd_elt_nil' to initialise.

  Maciej