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

[Matthieu Longo]

On 23/01/2026 09:31, Jan Beulich wrote:
> 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.
> 

I still would like to keep them together in the same patch.
Splitting such fixes does not bring much benefit.

>> 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 ...
> 

This is a mistake, and not needed.
I was to eager yesterday evening to copy paste my changes from the header to the C file.
LINKED_LIST_MUTATIVE_OPS_DECL can be kept as it is.
I will remove this change in the next revision.

>> --- 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).
> 

Fixed in the next revision.

> 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.
> 

The record is only used in gas, so moving this code inside BFD does not seem the right approach.

> 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

Here is the simplification. I will submit it in an independent patch.

Matthieu

diff --git a/gas/config/obj-elf-attr.c b/gas/config/obj-elf-attr.c
index fc354bfa7e9..a7b0675a140 100644
--- a/gas/config/obj-elf-attr.c
+++ b/gas/config/obj-elf-attr.c
@@ -1062,8 +1062,7 @@ obj_attr_v2_record (obj_attr_tag_t key, arg_t *arg_val)

    /* Go over the list of already recorded attributes and check for
       redefinitions (which are forbidden).  */
-  bool skip_recording = false;
-  obj_attr_v2_t *recorded_attr = bfd_obj_attr_v2_find_by_tag
+  const 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)
      {
@@ -1073,11 +1072,7 @@ obj_attr_v2_record (obj_attr_tag_t key, arg_t *arg_val)
               && strcmp (recorded_attr->val.string, obj_attr->val.string) != 0))
         as_bad (_("attribute '%" PRIu64 "' cannot be redefined"),
                 recorded_attr->tag);
-      skip_recording = true;
-    }

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