[PATCH v1] bfd/ELF: fix BFD library build --enable-shared

[Jan Beulich]

On 22.01.2026 20:04, Matthieu Longo wrote:
> The patch series that added support for Object Attributes v2 introduced
> regressions when building the BFD library as a shared object.
> 
> Incorrect usages of ATTRIBUTE_HIDDEN caused the following link-time errors:
> 
> /usr/bin/ld: config/obj-elf-attr.o: in function `obj_attr_v2_record':
> obj-elf-attr.c: undefined reference to `bfd_elf_obj_attr_v2_init'
> obj-elf-attr.c: undefined reference to `_bfd_obj_attr_v2_find_by_tag'

The fixes for these two I agree with. If they were in a separate patch, they
could go in right away.

> obj-elf-attr.c: undefined reference to `obj_attr_v2_t_append'
> /usr/bin/ld: config/obj-elf-attr.o: in function `obj_attr_v2_subsection_record':
> obj-elf-attr.c: undefined reference to `obj_attr_subsection_v2_t_append'
> obj-elf-attr.c: undefined reference to `obj_attr_subsection_v2_t_remove'
> obj-elf-attr.c: undefined reference to `obj_attr_subsection_v2_t_append'

For these, however, I don't understand ...

> @@ -3214,8 +3214,21 @@ _bfd_obj_attr_v2_find_by_tag (const obj_attr_subsection_v2_t *subsec,
>     implemented with a merge sort.
>     See more details in libiberty/doubly-linked-list.h  */
>  
> -LINKED_LIST_MUTATIVE_OPS_DECL (obj_attr_subsection_v2_t,
> +LINKED_LIST_DEFN_APPEND(obj_attr_subsection_v2_t,
> +			obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_PREPEND(obj_attr_subsection_v2_t,
> +			 obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_INSERT_BEFORE(obj_attr_subsection_v2_t,
>  			       obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_POP_FRONT(obj_attr_subsection_v2_t,
> +			   obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_POP_BACK(obj_attr_subsection_v2_t,
> +			  obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_REMOVE(obj_attr_subsection_v2_t,
> +			obj_attr_v2_t, /* extern */)
> +LINKED_LIST_DEFN_SWAP(obj_attr_subsection_v2_t,
> +		      obj_attr_v2_t, /* extern */)

... why e.g. LINKED_LIST_MUTATIVE_OPS_DECL() now needs expanding here.
I do see ...

> --- a/bfd/elf-bfd.h
> +++ b/bfd/elf-bfd.h
> @@ -3214,17 +3214,29 @@ extern bool _bfd_elf_read_notes
>    (bfd *, file_ptr, bfd_size_type, size_t) ATTRIBUTE_HIDDEN;
>  
>  extern obj_attr_v2_t *bfd_elf_obj_attr_v2_init (obj_attr_tag_t,
> -  union obj_attr_value_v2) ATTRIBUTE_HIDDEN;
> +  union obj_attr_value_v2);
>  extern void _bfd_elf_obj_attr_v2_free (obj_attr_v2_t *, obj_attr_encoding_v2_t)
>    ATTRIBUTE_HIDDEN;
>  extern obj_attr_v2_t *_bfd_elf_obj_attr_v2_copy (const obj_attr_v2_t *,
>    obj_attr_encoding_v2_t) ATTRIBUTE_HIDDEN;
>  extern int _bfd_elf_obj_attr_v2_cmp (const obj_attr_v2_t *,
>    const obj_attr_v2_t *) ATTRIBUTE_HIDDEN;
> -extern obj_attr_v2_t * _bfd_obj_attr_v2_find_by_tag
> -  (const obj_attr_subsection_v2_t *, obj_attr_tag_t, bool) ATTRIBUTE_HIDDEN;
> -LINKED_LIST_MUTATIVE_OPS_PROTOTYPE (obj_attr_subsection_v2_t,
> -				    obj_attr_v2_t, ATTRIBUTE_HIDDEN);

... that here you go from "hidden" everywhere to ...

> +extern obj_attr_v2_t * bfd_obj_attr_v2_find_by_tag
> +  (const obj_attr_subsection_v2_t *, obj_attr_tag_t, bool);
> +LINKED_LIST_DECL_APPEND(obj_attr_subsection_v2_t,
> +			obj_attr_v2_t, extern);

... "hidden" everywhere except this one. Which, however, looks like a
conceptual or layering problem to me: Either manipulations of such linked
lists are intended to occur outside of libbfd (and then all of operations
should be exposed), or they aren't (in which case all the functions
should remain hidden).

Related to this there's also a naming issue (which I had hoped I would
have got across by several earlier comments): A function named
obj_attr_v2_t_append() shouldn't be exported. It ought to be
bfd_obj_attr_v2_t_append(), bfd_elf_obj_attr_v2_t_append(),
bfd_elf_oav2_t_append(), or some such. I understand this may be difficult
with the linked-list macro machinery, so perhaps wrapper functions may
need creating. At _that_ point, at least for the time being, it may then
be okay to create just the three(?) wrappers actually needed by gas (with
suitable commentary or patch description).

That said, I'm now looking at one of the callers, obj_attr_v2_record():
What is it that's gas-specific in there besides the use of stdoutput and
as_bad()? IOW why does this function not live in libbfd? Going over
obj_attr_v2_subsection_record(), thing look similar there. With the
functions moved, the problematic references would all go away afaict.

Tangential to this, why does obj_attr_v2_record() need the
"skip_recording" local variable? Imo

  /* Go over the list of already recorded attributes and check for
     redefinitions (which are forbidden).  */
  obj_attr_v2_t *recorded_attr = _bfd_obj_attr_v2_find_by_tag
    (elf_obj_attr_subsections (stdoutput).last, obj_attr->tag, false);
  if (recorded_attr != NULL)
    {
      if ((arg_val->vtype == VALUE_UNSIGNED_INTEGER
	   && recorded_attr->val.uint != obj_attr->val.uint)
	  || (arg_val->vtype == VALUE_STRING
	      && strcmp (recorded_attr->val.string, obj_attr->val.string) != 0))
	as_bad (_("attribute '%" PRIu64 "' cannot be redefined"), recorded_attr->tag);

      if (arg_val->vtype == VALUE_STRING)
	free ((void *) obj_attr->val.string);
      free (obj_attr);
      return;
    }

would be easier to follow. (Not to speak of recorded_attr wanting to be
pointer-to-const from all I can see.) Yet of course you may be having
future plans here ...

Jan