[PATCH v9 12/19] Merge of Object Attributes v2 during linkage (generic logic)
Matthieu Longo
matthieu.longo@arm.com
Thu Nov 13 17:49:32 GMT 2025
On 31/10/2025 11:44, Jan Beulich wrote:
> On 01.09.2025 18:56, Matthieu Longo wrote:
>> --- a/bfd/elf-attrs.c
>> +++ b/bfd/elf-attrs.c
>> @@ -18,6 +18,108 @@
>> Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
>> MA 02110-1301, USA. */
>>
>> +/* Design note regarding the merge of Object Attributes v2 during linkage
>> +
>> + Entry point: _bfd_elf_link_setup_build_attributes
>> +
>> + This patch adds all the generic logic to the linker to process OAv2.
>> + The linker is an "advanced" consumer of OAv2. After parsing, it deduplicates
>> + them, merge them, detect any compatibility issues, and finally translate them
>> + to GNU properties.
>
> Nit: "merges", "detects", "translates" (also in the commit message).
>
Fixed.
> As to the translation to GNU properties: Is the same information then recorded
> in two different forms in the final binary? Or are the attributes dropped, and
> only the properties kept?
>
The same information is recorded in two different forms, if the GNU
property has a OAv2 equivalence, or if a OAv2 has a GNU property
equivalence.
GNU properties are used by the runtime linker and there is no plan to
migrate to OAv2 in my understanding because:
1. the look-up is faster.
2. backward compatibility.
> Could this translation, at the very least, be split off of this
> overly large patch?
>
Done, but in my opinion, the benefit of it for the review is really
minimal, and it makes the commit message even worse, because now,
instead of having a whole description in one place, it is split in two.
>> + ** Overall design
>> +
>> + The OAv2 processing pipeline follows a map-reduce pattern. Obviously, the
>> + actual processing in GNU ld is not multi-threaded, and the operations are not
>> + necessarily executed directly one after another.
>> +
>> + * Phase 1, map: successive per-file operations applied on the list of
>> + compatible input objects.
>> + 1. Parsing of the OAv2 section's data (also used by objcopy).
>> + 2. Translation of relevant GNU properties to OAv2. This is required for the
>> + backward-compatibility with input objects only marked using GNU
>> + properties.
>> + 3. Sorting of the subsections and object attributes. Further operations
>> + rely on the ordering to perform some optimization in the processing of
>> + the data.
>> + 4. Deduplication of subsections and object attributes, and detection of any
>> + conflict between duplicated subsections or tags.
>> + 5. Translation of relevant OAv2 to GNU properties for a forward
>> + -compatibility with the GNU properties merge.
>> +
>> + * Phase 2, reduce: OAv2 in input objects are merged together.
>> + 1. Gathering of "frozen" values (=coming from the command-line arguments)
>> + into a virtual read-only list of subsections and attributes.
>> + 2. Merging of OAv2 from an input file and the frozen input.
>> + 3. Merging of the results of step 2 together. Since the OAv2 merge is
>> + commutative and associative, it can be implemented as a reduce.
>> + However, GNU ld implements it as an accumulate because it does not
>> + support multithreading.
>> + Notes: the two merge phases also perform a marking of unsupported/invalid
>> + subsections and attributes. This marking can be used for debugging, and
>> + also more practically to drop unsupported optional subsections from the
>> + output.
>> +
>> + * Phase 3, finalization of the output.
>> + 1. Pruning of the unsupported/invalid subsections and attributes.
>> + 2. Serialization of OAv2 data (also used by objcopy).
>> + Notes:
>> + - There is no translation of the merged OAv2 to GNU properties at this
>> + stage, as the GNU properties merge has already all the information that
>> + were translated in step 5 of stage 1.
>> + - The GNU properties are currently required as the runtime linker does
>> + not understand OAv2 yet.
>> + - Phase 3 should also include a compatibility check between the final
>> + merge result of the current link unit and input shared objects. I opted
>> + for postponing this compatibility check, and GNU properties merge will
>> + take care of it as it already does.
>
> The splitting into three phases also looks as if they could be boundaries at
> which the patch could be split.
>
I am really not convinced about this split.
Unless you consider it mandatory to move forward, I prefer to abstain
from it.
>> @@ -424,6 +526,100 @@ bfd_elf_set_obj_attr_contents (bfd *abfd, bfd_byte *buffer, bfd_vma size)
>> abort ();
>> }
>>
>> +/* Structure storing the result of a search in the list of input BFDs.
>> + - the pointer to the BFD.
>> + - the pointer to the section containing the object attributes. */
>> +typedef struct
>> +{
>> + bfd *pbfd;
>> + bool has_build_attributes;
>> + asection *sec;
>> +} bfd_search_result_t;
>> +
>> +/* Checks whether a BFD contains object attributes, and if so search for the
>> + relevant section storing them. */
>> +static bool
>> +bfd_has_build_attributes (bfd *abfd, bfd_search_result_t *res)
>> +{
>> + if (elf_obj_attr_subsections (abfd).size == 0)
>> + return false;
>> + res->has_build_attributes = true;
>> +
>> + const char *sec_name = get_elf_backend_data (abfd)->obj_attrs_section;
>> + if ((res->sec = bfd_get_section_by_name (abfd, sec_name)) == NULL)
>> + return false;
>> + return true;
>> +}
>
> This is an odd interface: The sole caller sets res->pbfd, just to pass in the
> same pointer. Why would the filling of the structure not be done solely here?
>
Indeed I can move the filling of the structure inside
bfd_has_build_attributes(). I moved res->pbfd inside
bfd_has_build_attributes and added comments to explain better what this
boolean means.
/* Search for the first input object file containing object attributes.
If no such object is found, PBFD points to the last object file that
could have contained object attributes. HAS_OBJECT_ATTRIBUTES allows
to distinguish the cases when PBFD contains or does not contain object
attributes. If no candidate file is found, PBFD will stay NULL. */
static bfd_search_result_t
bfd_linear_find_first_with_obj_attrs (const struct bfd_link_info *info)
...
> Further, is has_build_attributes actually necessary as a separate field? Can't
> pbfd (being NULL or non-NULL) fulfill its purpose?
>
When pbfd is NULL, it means that no candidate file to host an OAv2
section was found.
Has_object_attributes means that the file contains object attributes
that are not necessarily part of an OAv2 section (sec might be NULL).
For instance, they could have been added by the translation of GNU
properties to OAv2.
> Finally (I think I had mentioned this before, but I may be misremembering and
> it was in another context): Going solely be section name is, imo, problematic.
> ELF has section types for a reason.
>
1. How does GNU Properties handle the case of several sections with the
expected type, but with the same expected name, or different names ?
Regarding the parsing of GNU properties, if several sections with
NT_GNU_PROPERTY_TYPE_0 are met, all of them are deserialized and their
content is appended to the list of GNU properties for this BFD input.
Regarding the merge of those properties,
_bfd_elf_link_setup_gnu_properties() does not seem to harmonize them
before merging them. However, elf_merge_gnu_property_list() seems to
handle their merge. I might be wrong, I find this code quite difficult
to follow to be honest.
In _bfd_elf_link_setup_gnu_properties(), the GNU properties section is
searched only by name. The section type is never checked. If it does not
find one with the expected name, it creates a new one.
2. What about Object Attributes ?
There was no mention of such cases in the Object Attributes
specification, so I asked the author.
He confirmed that he could not find such cases in either the 32-bit or
64-bit ABI that says anything about multiple .ARM.attributes sections.
The tools are expected to only ever produced one section so this case
never came up in practice. The closest thing the spec does say is that
multiple instances of the same attribute within a subsection are not
permitted. GNU as already handle this case to detect such duplication,
check whether the attribute values are contradictory and emit an error,
or if they match, deduplicate the value.
When it comes down to intent. Build Attributes are defined per
relocatable object so there is no need, or benefit, for there to be more
than one. If attributes applied only to an individual section, then it
could make sense to have a case where we have multiple .ARM.attributes
sections, each describing the attributes for just one .text section. But
this case is not supported by the current specification.
3. What will LLVM handle such an exotic object ?
LLD will parse every .ARM.attributes sections it sees, but it will only
"process" the last one, earlier ones will get ignored. This is more a
quirk of the implementation rather than intent. In truth it is assumed
that there would always be one .ARM.attributes section.
4. What does the author propose for such a case ?
Each object file can contain at most one .ARM.attributes section. What a
tool does when it encounters more than one in a single object is Q-o-I.
For example it could give an error message.
Making an object with more than one .ARM.attributes sections would be
hard with GNU and LLVM as. Both GNU and Clang integrated assembler
directives will only produce 1 section, and ld is expected to produce 1
"merged" output. For this reason, it could make it awkward to test such
cases.
https://github.com/ARM-software/abi-aa/pull/230/commits/226364b3f3db6a9091a9cecab38cce668c09433d
I propose the following change to the existing implementation to clarify
this situation, and document what GNU ld does.
Please let me know if you think that it is clear enough.
diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
index 0bc82fe02cd..f65570a96d2 100644
--- a/bfd/elf-attrs.c
+++ b/bfd/elf-attrs.c
@@ -542,9 +542,15 @@ typedef struct
} bfd_search_result_t;
/* Checks whether a BFD contains object attributes, and if so search
for the
- relevant section storing them. */
+ relevant section storing them. The name and type of the section have to
+ match with what the backend expects, i.e.
elf_backend_obj_attrs_section and
+ elf_backend_obj_attrs_section_type, otherwise the object attributes
section
+ won't be recognized as such, and will be skipped.
+ Return True if an object attribute section is found, False
otherwise. */
static bool
-bfd_has_object_attributes (bfd *abfd, bfd_search_result_t *res)
+bfd_has_object_attributes (const struct bfd_link_info *info,
+ bfd *abfd,
+ bfd_search_result_t *res)
{
/* The file may contain object attributes. Save this candidate. */
res->pbfd = abfd;
@@ -554,10 +560,20 @@ bfd_has_object_attributes (bfd *abfd,
bfd_search_result_t *res)
res->has_object_attributes = true;
+ uint32_t sec_type = get_elf_backend_data (abfd)->obj_attrs_section_type;
const char *sec_name = get_elf_backend_data (abfd)->obj_attrs_section;
- if ((res->sec = bfd_get_section_by_name (abfd, sec_name)) == NULL)
- return false;
- return true;
+ res->sec = bfd_get_section_by_name (abfd, sec_name);
+ if (res->sec != NULL)
+ {
+ if (elf_section_type (res->sec) != sec_type)
+ {
+ info->callbacks->minfo
+ (_("%X%pB: warning: ignoring section '%s' with unexpected
type 0x%x\n"),
+ abfd, elf_section_type (res->sec), sec_name);
+ res->sec = NULL;
+ }
+ }
+ return (res->sec != NULL);
}
/* Returns True if the given BFD is an ELF object with the target backend
@@ -597,7 +613,7 @@ bfd_linear_find_first_with_obj_attrs (const struct
bfd_link_info *info)
for (bfd *abfd = info->input_bfds; abfd != NULL; abfd = abfd->link.next)
{
if (elf_may_contain_obj_attrs (info, abfd)
- && bfd_has_object_attributes (abfd, &res))
+ && bfd_has_object_attributes (info, abfd, &res))
break;
}
return res;
@@ -1807,7 +1823,15 @@ oav2_subsection_destructive_merge (const bfd *abfd,
/* Merge duplicated subsections and object attributes inside a same object
file. After a call to this function, the subsections and object
attributes
- are sorted. */
+ are sorted.
+ Note: this function allows to handle in a best effort exoctic objects
+ produced by a non-GNU assembler. Duplicated subsections could come
from the
+ same section, or different ones. Indeed, the deserializer
deserializes the
+ content of a section if its type matches the object attributes type
specified
+ by the backend, regardless of the section name. The behavior for
such cases
+ is not specified by the Object Attributes specification, and are a
question
+ of implementation. Non-GNU linkers might have a different behavior
with such
+ exotic objects. */
static bool
oav2_file_scope_merge_subsections (const bfd *abfd)
{
@@ -2049,9 +2073,18 @@ _bfd_elf_link_setup_object_attributes (struct
bfd_link_info *info)
if (res.sec == NULL)
{
- /* This input object has no object attribute section, so the object
- attribute version was never set by the deserializer.
- Set it to the backend recommended value. */
+ /* This input object has no object attribute section matching the
name and
+ type specified by the backend, i.e.
elf_backend_obj_attrs_section and
+ elf_backend_obj_attrs_section_type.
+ One of the two following cases is possible:
+ 1. No object attribute were found in this file, so the object
attribute
+ version was never set by the deserializer.
+ 2. The deserializer might have found attributes in another
section with
+ the correct type but the wrong name. The object attribute
version
+ should have been set correctly in this case.
+ Whatever of those two cases, we set the object attribute
version to the
+ backend's recommended value, and create a new section with the
expected
+ name and type. */
elf_obj_attr_version (res.pbfd)
= get_elf_backend_data (res.pbfd)->default_obj_attr_version;
res.sec = create_object_attributes_section (info, res.pbfd);
>> +/* Returns True if the given BFD is an ELF object with the target backend
>> + machine code, non-dynamic (i.e. not a shared library), non-executable, and
>> + has sections. False otherwise.
>> + Note: this function is a convenient encapsulation of the predicate used to
>> + search for objects containing object attributes in the list of BFDs. */
>> +static bool
>> +elf_may_contain_obj_attrs (struct bfd_link_info *info,
>> + bfd *bed)
>
> "bed" stands for "backend data", like e.g. used ...
>
Renamed to abfd in the new revision.
>> +{
>> + const struct elf_backend_data *output_bed
>> + = get_elf_backend_data (info->output_bfd);
>
> ... here. Please let's not use misleading variable names.
>
and output_bed to output_bfd_bed.
>> + unsigned int elfclass = output_bed->s->elfclass;
>> + int elf_machine_code = output_bed->elf_machine_code;
>> + return (bfd_get_flavour (bed) == bfd_target_elf_flavour
>> + && bed->section_count != 0
>> + && (bed->flags & (DYNAMIC | EXEC_P)) == 0
>> + && elf_machine_code == get_elf_backend_data (bed)->elf_machine_code
>> + && elfclass == get_elf_backend_data (bed)->s->elfclass);
>> +}
>> +
>> +/* Search for the first input object file containing object attributes. */
>> +static bfd_search_result_t
>> +bfd_linear_find_first_with_obj_attrs (struct bfd_link_info *info)
>> +{
>> + bfd_search_result_t res = {
>> + .pbfd = NULL,
>> + .has_build_attributes = false,
>> + .sec = NULL,
>> + };
>> +
>> + for (bfd *abfd = info->input_bfds; abfd != NULL; abfd = abfd->link.next)
>> + if (elf_may_contain_obj_attrs (info, abfd))
>> + {
>> + res.pbfd = abfd;
>> + if (bfd_has_build_attributes (abfd, &res))
>> + break;
>> + }
>> + return res;
>> +}
>> +
>> +/* Create a build attributes section for the given bfd input. */
>> +static asection *
>> +create_build_attributes_section (struct bfd_link_info *info,
>> + bfd *ebfd)
>
> While not as bad here, I still wonder why it's "ebfd" when the common naming is
> "abfd".
>
This is a shortcut taken from elf-properties.c.
I shortened "elf_bfd" to "ebfd", but I agree this is not very consistent
with others parts of the code.
Changed in the next revision.
>> +{
>> + asection *sec;
>> + const char *sec_name = get_elf_backend_data (ebfd)->obj_attrs_section;
>> + sec = bfd_make_section_with_flags (ebfd,
>> + sec_name,
>> + (SEC_READONLY
>> + | SEC_HAS_CONTENTS
>> + | SEC_DATA));
>> + if (sec == NULL)
>> + info->callbacks->fatal (_("%P: failed to create %s section\n"), sec_name);
>> +
>> + unsigned align
>> + = (get_elf_backend_data (info->output_bfd)->s->elfclass == ELFCLASS64
>> + ? 3
>> + : 2);
>
> What in the format requires different alignment for 32- vs 64-bit ELF?
>
This is a copy-paste from _bfd_elf_link_create_gnu_property_sec() in
bfd/elf-properties.c It is part of the original patch that introduced
the feature.
v1:
https://inbox.sourceware.org/binutils/20210620225029.390239-1-hjl.tools@gmail.com/
v2:
https://inbox.sourceware.org/binutils/20210622235715.2813205-1-hjl.tools@gmail.com/
v3:
https://inbox.sourceware.org/binutils/20210624132411.1993105-1-hjl.tools@gmail.com/
I don't know the original reason for it, and there was no discussion on
this specific detail.
>> + if (!bfd_set_section_alignment (sec, align))
>> + info->callbacks->fatal (_("%pA: failed to align section\n"), sec);
>> +
>> + elf_section_type (sec) = get_elf_backend_data (ebfd)->obj_attrs_section_type;
>> +
>> + bfd_set_section_size (sec, bfd_elf_obj_attr_size (ebfd));
>> +
>> + return sec;
>> +}
>> +
>> /* The first two tags in gnu-testing namespace are known, and so have a name and
>> can be initialized to the default value ('0' or NULL) depending on the
>> encoding specified on the subsection. Any tags above 1 will be considered
>> @@ -563,6 +759,1309 @@ oav2_encoding_to_string (obj_attr_encoding_v2 encoding)
>> return (encoding == OA_ENC_ULEB128) ? "ULEB128" : "NTBS";
>> }
>>
>> +/* Initialize the given ATTR with its default value coming from the known tag
>> + registry. */
>> +static void
>> +oav2_attr_overwrite_with_default (struct bfd_link_info *info,
>> + obj_attr_subsection_v2 *subsec,
>
> subsec is only and input, isn't it? (info looks to be, too, but uses down the
> call tree may require it to be pointer-to-non-const, as sadly libbfd still is
> pretty far from being const-correct.)
>
> Again, constification of pointer targets wants to be done throughout the series,
> wherever possible.
>
Addressed the constification here and in others places in this patch.
>> + obj_attr_v2 *attr)
>> +{
>> + const struct elf_backend_data *bed = get_elf_backend_data (info->output_bfd);
>> +
>> + const obj_attr_info_t *attr_info
>> + = obj_attr_v2_find_known_by_tag (bed, subsec->name, attr->tag);
>> + if (attr_info == NULL)
>> + {
>> + attr->status = obj_attr_v2_unknown;
>> + if (subsec->encoding == OA_ENC_ULEB128)
>> + attr->val.uint_val = 0;
>> + else
>> + attr->val.string_val = NULL;
>> + return;
>> + }
>> +
>> + if (bed->obj_attr_v2_default_value != NULL
>> + && bed->obj_attr_v2_default_value (info, attr_info, subsec, attr))
>> + {}
>> + else if (subsec->encoding == OA_ENC_NTBS)
>> + {
>> + if (attr->val.string_val != NULL)
>> + {
>> + free ((void *) attr->val.string_val);
>> + attr->val.string_val = NULL;
>> + }
>> + if (attr_info->default_value.string_val != NULL)
>> + attr->val.string_val = xstrdup (attr_info->default_value.string_val);
>> + }
>> + else
>> + attr->val.uint_val = attr_info->default_value.uint_val;
>> +}
>> +
>> +/* Create a new attribute with the same key (=tag) as ATTR, and initialized with
>> + its default value from the known tag registry. */
>> +static obj_attr_v2 *
>> +oav2_attr_default (struct bfd_link_info *info,
>> + obj_attr_subsection_v2 *subsec,
>> + obj_attr_v2 *attr)
>> +{
>> + obj_attr_v2 *new_attr = _bfd_elf_obj_attr_v2_copy (attr, subsec->encoding);
>> + oav2_attr_overwrite_with_default (info, subsec, new_attr);
>> + return new_attr;
>> +}
>> +
>> +/* The currently supported merge policy in the testing GNU namespace.
>> + - bitwise AND: apply bitwise AND.
>> + - bitwise OR: apply bitwise OR.
>> + - String-ADD: concatenates strings together with a '+' in-between.
>> + Note: Such policies should only be used for testing. */
>> +typedef enum {
>> + SUBSECTION_TESTING_MERGE_UNSUPPORTED = 0,
>> + SUBSECTION_TESTING_MERGE_AND_POLICY = 1,
>> + SUBSECTION_TESTING_MERGE_OR_POLICY = 2,
>> + SUBSECTION_TESTING_MERGE_ADD_POLICY = 3,
>> +} gnu_testing_merge_policy;
>> +
>> +/* Determine which merge policy will be applied to SUBSEC. The GNU policy are
>> + detected from the name of the subsection. It should follow the following
>> + pattern: "gnu-testing-XXXXXX-MERGE-<POLICY>".
>> + Return one of the known merge policy if recognised, UNSUPPORTED otherwise. */
>> +static gnu_testing_merge_policy
>> +gnu_testing_merge_subsection (const char *subsec_name)
>> +{
>> + if (! gnu_testing_namespace (subsec_name))
>> + return SUBSECTION_TESTING_MERGE_UNSUPPORTED;
>> +
>> + size_t subsec_name_len = strlen (subsec_name);
>> + if (strcmp ("-MERGE-AND", subsec_name + subsec_name_len - 10) == 0)
>> + return SUBSECTION_TESTING_MERGE_AND_POLICY;
>> + else if (strcmp ("-MERGE-OR", subsec_name + subsec_name_len - 9) == 0)
>> + return SUBSECTION_TESTING_MERGE_OR_POLICY;
>> + else if (strcmp ("-MERGE-ADD", subsec_name + subsec_name_len - 10) == 0)
>> + return SUBSECTION_TESTING_MERGE_ADD_POLICY;
>> + else
>> + return SUBSECTION_TESTING_MERGE_UNSUPPORTED;
>> +}
>> +
>> +/* Merge policy Integer-AND: apply bitwise AND between REF and RHS. */
>> +obj_attr_v2_merge_result
>> +obj_attr_v2_merge_policy_AND (struct bfd_link_info *info ATTRIBUTE_UNUSED,
>
> I agree with the use of the term "policy" further up, but the functions are
> actors, not policies, so I don't think their name should include the word.
>
I removed "_policy" from the functions name.
> As a non-static function this may also again want to gain _bfd_ or bfd_ as
> a prefix? (Why is it non-static anyway? The sole caller looks to liver further
> down in this same file.)
>
The caller is in bfd/elfxx-aarch64.c
See patch 14/19.
I disagree on adding the prefix _bfd_ to this function, as it is
internal to bfd (i.e. either used in elf-attr.c or a backend handler).
However, splitting elf-bfd.h between public and private functions might
help to clarify the situation, but this is out of scope of this patch
series.
>> + bfd *abfd ATTRIBUTE_UNUSED,
>> + obj_attr_subsection_v2 *subsec,
>> + obj_attr_v2 *ref, obj_attr_v2 *rhs,
>> + obj_attr_v2 *frozen ATTRIBUTE_UNUSED)
>> +{
>> + BFD_ASSERT (subsec->encoding == OA_ENC_ULEB128);
>> +
>> + obj_attr_v2_merge_result res = {
>> + .merge = true,
>> + .val.uint_val = 0,
>
> Pointless initializer - it's overwritten unconditionally below, and even if
> it wasn't the use of designated initializers for the other fields would have
> it obtain value 0.
>
Removed in the next revision.
>> + .reason = MERGE_OK,
>> + };
>> +
>> + uint32_t original_value = ref->val.uint_val;
>> + res.val.uint_val = (ref->val.uint_val & rhs->val.uint_val);
>> + res.merge = (res.val.uint_val != original_value);
>> + if (!res.merge)
>> + res.reason = SAME_VALUE_AS_REF;
>> +
>> + return res;
>> +}
>> +
>> +/* Merge policy Integer-OR: apply bitwise OR between REF and RHS. */
>> +static obj_attr_v2_merge_result
>> +obj_attr_v2_merge_policy_OR (struct bfd_link_info *info ATTRIBUTE_UNUSED,
>> + bfd *abfd ATTRIBUTE_UNUSED,
>> + obj_attr_subsection_v2 *subsec,
>> + obj_attr_v2 *ref, obj_attr_v2 *rhs,
>> + obj_attr_v2 *frozen ATTRIBUTE_UNUSED)
>> +{
>> + BFD_ASSERT (subsec->encoding == OA_ENC_ULEB128);
>> +
>> + obj_attr_v2_merge_result res = {
>> + .merge = true,
>> + .val.uint_val = 0,
>> + .reason = MERGE_OK,
>> + };
>> +
Same here, removed the initialization.
>> + uint32_t original_value = ref->val.uint_val;
>> + res.val.uint_val = (ref->val.uint_val | rhs->val.uint_val);
>> + res.merge = (res.val.uint_val != original_value);
>> + if (res.val.uint_val == original_value)
>> + res.reason = SAME_VALUE_AS_REF;
>> +
>> + return res;
>> +}
>> +
>> +/* Merge policy String-ADD: concatenates strings from REF and RHS together
>> + adding a '+' character in-between. */
>> +static obj_attr_v2_merge_result
>> +obj_attr_v2_merge_policy_ADD (struct bfd_link_info *info ATTRIBUTE_UNUSED,
>> + bfd *abfd ATTRIBUTE_UNUSED,
>> + obj_attr_subsection_v2 *subsec,
>> + obj_attr_v2 *ref, obj_attr_v2 *rhs,
>> + obj_attr_v2 *frozen)
>> +{
>> + BFD_ASSERT (subsec->encoding == OA_ENC_NTBS);
>> +
>> + obj_attr_v2_merge_result res = {
>> + .merge = false,
>> + .val.uint_val = 0,
>
> That's the wrong field here, isn't it?
>
Yes, fixed in the next revision.
>> + .reason = MERGE_OK,
>> + };
>> +
>> + if (ref->val.string_val && rhs->val.string_val)
>> + {
>> + res.merge = true;
>> + size_t ref_s_size = strlen (ref->val.string_val);
>> + size_t rhs_s_size = strlen (rhs->val.string_val);
>> + char *buffer = xmalloc (ref_s_size + 1 + rhs_s_size + 1);
>> + res.val.string_val = buffer;
>> + memcpy (buffer, ref->val.string_val, ref_s_size);
>> + buffer += ref_s_size;
>> + *buffer = '+';
>> + ++buffer;
>> + memcpy (buffer, rhs->val.string_val, rhs_s_size + 1);
>> + }
>> + else if (ref->val.string_val)
>> + {
>> + /* Nothing to do, frozen (if not NULL) should already be merged with
>> + it. */
>> + res.reason = SAME_VALUE_AS_REF;
>
> No was to assert what the comment says?
>
I improved the comments, ...
>> + }
>> + else if (rhs->val.string_val)
>> + {
>> + res.merge = true;
>> +
>> + size_t frozen_s_size
>> + = (frozen && frozen->val.string_val
>> + ? strlen (frozen->val.string_val)
>> + : 0);
>
> The purpose of "frozen" could do with commenting on anyway.
>
And removed "frozen" from this merge, as it was already merged.
>> + if (frozen_s_size == 0)
>> + {
>> + res.val.string_val = rhs->val.string_val;
>> + rhs->val.string_val = NULL;
>
> That is, the caller's input variable is altered, and the caller needs to be
> aware that it cannot be used again subsequently?
>
I also removed this optimization that caused more complexity than
benefit, and prevented me from constifying RHS before.
Version for the next revision:
/* Merge policy String-ADD: concatenates strings from REF and RHS together
adding a '+' character in-between. */
static obj_attr_v2_merge_result
obj_attr_v2_merge_ADD (const struct bfd_link_info *info ATTRIBUTE_UNUSED,
const bfd *abfd ATTRIBUTE_UNUSED,
const obj_attr_subsection_v2 *subsec,
const obj_attr_v2 *ref, const obj_attr_v2 *rhs,
const obj_attr_v2 *frozen ATTRIBUTE_UNUSED)
{
BFD_ASSERT (subsec->encoding == OA_ENC_NTBS);
obj_attr_v2_merge_result res = {
.merge = false,
.val.string = NULL,
.reason = MERGE_OK,
};
/* Note: FROZEN is unused for this "concatenating" merge because it is
either passed as RHS when coming from oav2_subsections_merge_frozen(),
or passed as FROZEN when coming from oav2_subsections_merge() and was
already merged. */
/* REF and RHS have both a value. Concatenate RHS to REF. */
if (ref->val.string && rhs->val.string)
{
res.merge = true;
size_t ref_s_size = strlen (ref->val.string);
size_t rhs_s_size = strlen (rhs->val.string);
char *buffer = xmalloc (ref_s_size + 1 + rhs_s_size + 1);
res.val.string = buffer;
memcpy (buffer, ref->val.string, ref_s_size);
buffer += ref_s_size;
*buffer = '+';
++buffer;
memcpy (buffer, rhs->val.string, rhs_s_size + 1);
}
/* REF has a value, but RHS does not. Nothing to do. */
else if (ref->val.string)
res.reason = SAME_VALUE_AS_REF;
/* No previous value in REF. RHS is the new value. */
else if (rhs->val.string)
{
/* This case could be optimized to avoid the dynamic allocation
by moving
the value from RHS to RES, but then, we need to distinguish the case
when RHS and FROZEN are the same, and in this case, the value should
not be moved but copied. The little benefit is not worth the added
complexity. */
res.merge = true;
size_t rhs_s_size = strlen (rhs->val.string);
char *buffer = xmalloc (rhs_s_size + 1);
res.val.string = buffer;
memcpy (buffer, rhs->val.string, rhs_s_size + 1);
}
return res;
}
>> + }
>> + else
>> + {
>> + size_t rhs_s_size = strlen (rhs->val.string_val);
>> + char *buffer = xmalloc (frozen_s_size + 1 + rhs_s_size + 1);
>> + res.val.string_val = buffer;
>> + memcpy (buffer, frozen->val.string_val, frozen_s_size);
>> + buffer += frozen_s_size;
>> + *buffer = '+';
>> + ++buffer;
>> + memcpy (buffer, rhs->val.string_val, rhs_s_size + 1);
>> + }
>> + }
>> + return res;
>> +}
>> +
>> +/* Return the merge result between attributes LHS, RHS and FROZEN. */
>> +static obj_attr_v2_merge_result
>> +oav2_attr_merge (struct bfd_link_info *info,
>> + bfd *abfd,
>> + obj_attr_subsection_v2 *subsec,
>> + obj_attr_v2 *lhs, obj_attr_v2 *rhs,
>> + obj_attr_v2 *frozen, bool frozen_as_abfd)
>> +{
>> + obj_attr_v2_merge_result res = {
>> + .merge = false,
>> + .val.uint_val = 0,
>> + .reason = MERGE_OK,
>> + };
>> +
>> + gnu_testing_merge_policy policy;
>> +
>> + if (get_elf_backend_data (abfd)->obj_attr_v2_tag_merge != NULL)
>> + {
>> + if (frozen_as_abfd)
>> + {
>> + obj_attr_v2 *tmp = lhs;
>> + lhs = rhs;
>> + rhs = tmp;
>> + }
>> + res = get_elf_backend_data (abfd)->obj_attr_v2_tag_merge (info, abfd,
>> + subsec, lhs, rhs, frozen);
>
> Again it's entirely unclear to me what "frozen" and "frozen_as_abfd" are about.
>
I clarified the meaning of "frozen_as_abfd" by adding the following
comments. I also renamed "frozen_as_abfd" as "frozen_is_abfd" because
this "as" was making things even more obscur.
diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
index a42747bb454..80e1aa0a906 100644
--- a/bfd/elf-attrs.c
+++ b/bfd/elf-attrs.c
@@ -976,13 +976,17 @@ obj_attr_v2_merge_ADD (const struct bfd_link_info
*info ATTRIBUTE_UNUSED,
return res;
}
-/* Return the merge result between attributes LHS, RHS and FROZEN. */
+/* Return the merge result between attributes LHS, RHS and FROZEN.
+ Note: FROZEN_IS_RHS indicates that FROZEN is in RHS's position.
This happens
+ when this function is called from oav2_subsections_merge_frozen().
When this
+ boolean is true, arguments are swapped to get the correct diagnostic
messages
+ in case of issues. */
static obj_attr_v2_merge_result
oav2_attr_merge (const struct bfd_link_info *info,
const bfd *abfd,
const obj_attr_subsection_v2 *subsec,
const obj_attr_v2 *lhs, const obj_attr_v2 *rhs,
- const obj_attr_v2 *frozen, bool frozen_as_abfd)
+ const obj_attr_v2 *frozen, bool frozen_is_rhs)
{
obj_attr_v2_merge_result res = {
.merge = false,
@@ -994,7 +998,12 @@ oav2_attr_merge (const struct bfd_link_info *info,
if (get_elf_backend_data (abfd)->obj_attr_v2_tag_merge != NULL)
{
- if (frozen_as_abfd)
+ /* If FROZEN is RHS (i.e. called from
oav2_subsections_merge_frozen()), it
+ means that the merged value of LHS (the REF) and FROZEN is going to
+ be merged by the caller into LHS. However, since diagnostic
messages
+ always point to RHS, we need to swap LHS and RHS, so that ABFD
is not
+ associated wrongly to FROZEN. */
+ if (frozen_is_rhs)
{
const obj_attr_v2 *tmp = lhs;
lhs = rhs;
@@ -1202,7 +1211,10 @@ handle_optional_subsection_merge (const struct
bfd_link_info *info,
const obj_attr_subsection_v2 *s_frozen)
{
(void) info;
- bool frozen_as_abfd = (ref_bfd == abfd);
+ /* REF_BFD and ABFD are the same only when the call originated from
+ oav2_subsections_merge_frozen(), when FROZEN is merged into ABFD
+ (the future REF_BFD). See detailed explanation in
oav2_attr_merge(). */
+ bool frozen_is_abfd = (ref_bfd == abfd);
obj_attr_v2 *a_ref = s_ref->first;
obj_attr_v2 *a_abfd = s_abfd->first;
obj_attr_v2 *a_frozen_first = (s_frozen != NULL) ? s_frozen->first :
NULL;
@@ -1222,7 +1234,7 @@ handle_optional_subsection_merge (const struct
bfd_link_info *info,
obj_attr_v2 *a_default = oav2_attr_default (info, s_abfd, a_ref);
obj_attr_v2_merge_result res
= oav2_attr_merge (info, abfd, s_ref, a_ref, a_default,
a_frozen,
- frozen_as_abfd);
+ frozen_is_abfd);
_bfd_elf_obj_attr_v2_free (a_default, s_ref->encoding);
if (res.merge)
a_ref->val = res.val;
@@ -1235,7 +1247,7 @@ handle_optional_subsection_merge (const struct
bfd_link_info *info,
obj_attr_v2 *a_default = oav2_attr_default (info, s_ref, a_abfd);
obj_attr_v2_merge_result res
= oav2_attr_merge (info, abfd, s_ref, a_default, a_abfd,
a_frozen,
- frozen_as_abfd);
+ frozen_is_abfd);
if (res.merge || res.reason == SAME_VALUE_AS_REF)
{
a_default->val = res.val;
@@ -1249,7 +1261,7 @@ handle_optional_subsection_merge (const struct
bfd_link_info *info,
{
obj_attr_v2_merge_result res
= oav2_attr_merge (info, abfd, s_ref, a_ref, a_abfd, a_frozen,
- frozen_as_abfd);
+ frozen_is_abfd);
if (res.merge)
a_ref->val = res.val;
else if (res.reason == UNSUPPORTED)
@@ -1282,7 +1294,7 @@ handle_optional_subsection_merge (const struct
bfd_link_info *info,
obj_attr_v2 *a_default = oav2_attr_default (info, s_abfd, a_ref);
obj_attr_v2_merge_result res
= oav2_attr_merge (info, abfd, s_ref, a_ref, a_default, a_frozen,
- frozen_as_abfd);
+ frozen_is_abfd);
_bfd_elf_obj_attr_v2_free (a_default, s_ref->encoding);
if (res.merge)
a_ref->val = res.val;
@@ -1303,7 +1315,8 @@ handle_subsection_merge (const struct
bfd_link_info *info,
{
if (! s_ref->optional)
return oav2_subsection_perfect_match (info, ref_bfd, abfd, s_ref,
s_abfd);
- return handle_optional_subsection_merge (info, ref_bfd, abfd, s_ref,
s_abfd, s_frozen);
+ return handle_optional_subsection_merge
+ (info, ref_bfd, abfd, s_ref, s_abfd, s_frozen);
}
/* Merge case 2: S_ABFD does not exist, but S_REF does.
@@ -1421,11 +1434,12 @@ oav2_subsection_mismatching_params (const struct
bfd_link_info *info,
return mismatch;
}
-/* Merge object attributes from FROZEN into the object file REF_BFD.
+/* Merge object attributes from FROZEN into the object file ABFD.
Note: this function is called only once before starting the merge
process
- between the object files. REF_BFD is used to store the result of
the merge,
- but REF_BFD is also an input file, so any mismatch against FROZEN
should be
- raised before the values of REF_BFD be modified. */
+ between the object files. ABFD corresponds to the future REF_BFD,
and is
+ used to store the result of the merge. ABFD is also an input file,
so any
+ mismatch against FROZEN should be raised before the values of ABFD be
+ modified. */
static bool
oav2_subsections_merge_frozen (const struct bfd_link_info *info,
const bfd *abfd,
@@ -1436,6 +1450,11 @@ oav2_subsections_merge_frozen (const struct
bfd_link_info *info,
bool success = true;
+ /* Note: all the handle_subsection_* functions call oav2_attr_merge()
down the
+ stack. Passing ABFD as REF_BFD allows to detect that this function is
+ the caller. Then a special behavior in oav2_attr_merge() is
triggered for
+ this specific use case, so that we can obtain the right
diagnostics. */
+
obj_attr_subsection_v2 *s_abfd = elf_obj_attr_subsections (abfd).first;
while (s_frozen != NULL && s_abfd != NULL)
{
>> +/* Report missing required attribute with key TAG in subsection SREF. */
>> +static void
>> +report_missing_required_obj_attr (struct bfd_link_info *info,
>> + bfd *abfd,
>> + obj_attr_subsection_v2 *s_ref,
>> + obj_attr_tag_t tag)
>> +{
>> + const struct elf_backend_data *bed = get_elf_backend_data (abfd);
>> + const char *tag_s = obj_attr_v2_tag_to_string (bed, s_ref->name, tag);
>> + info->callbacks->einfo (
>> + _("%X%pB: error: missing required object attribute '%s' in subsection "
>> + "'%s'\n"), abfd, tag_s, s_ref->name);
>
> Imo better:
>
> info->callbacks->einfo (
> _("%X%pB: error: missing required object attribute '%s' in subsection '%s'\n"),
> abfd, tag_s, s_ref->name);
>
> Splitting format strings should be avoided, unless the lines get really long (but
> then the diagnostic text likely is too verbose anyway).
>
Ok, fixed in the next revision.
What is the general recommendation when the string goes beyond the limit
of 80 characters ?
>> +}
>> +
>> +/* Report required attribute A_ABFD mismatching with A_REF. */
>> +static void
>> +report_mismatching_required_obj_attr (struct bfd_link_info *info,
>> + bfd *ref_bfd,
>> + bfd *abfd,
>> + obj_attr_subsection_v2 *s_ref,
>> + obj_attr_v2 *a_ref,
>> + obj_attr_v2 *a_abfd)
>> +{
>> + const struct elf_backend_data *bed = get_elf_backend_data (abfd);
>> + const char* tag_s = obj_attr_v2_tag_to_string (bed, s_ref->name, a_ref->tag);
>> + if (s_ref->encoding == OA_ENC_ULEB128)
>> + {
>> + info->callbacks->einfo (
>> + _("%X%pB, %pB: error: mismatching values 0x%x and 0x%x for "
>
> %#x
>
See your previous comment below in v8.
https://inbox.sourceware.org/binutils/99e171d4-64e6-4cfe-9c77-f47791373eed@suse.com/
>>> +/* Report required attribute A_ABFD mismatching with A_REF. */
>>> +static void
>>> +report_mismatching_required_obj_attr (struct bfd_link_info *info,
>>> + bfd *ref_bfd,
>>> + bfd *abfd,
>>> + obj_attr_subsection_v2 *s_ref,
>>> + obj_attr_v2 *a_ref,
>>> + obj_attr_v2 *a_abfd)
>>> +{
>>> + const struct elf_backend_data *be = get_elf_backend_data (abfd);
>>> + const char* tag_s = obj_attr_v2_tag_to_string (be, s_ref->name,
a_ref->tag);
>>> + if (s_ref->encoding == OA_ENC_ULEB128)
>>> + {
>>> + if (tag_s)
>>> + info->callbacks->einfo (
>>> + _("%X%pB, %pB: error: mismatching values 0x%x and 0x%x for "
>>
>> In case I didn't say so elsewhere already: Please prefer %#x over 0x%x.
>>
>
> If I change it to %#x, it crashes due to a SEGFAULT with the stack below:
>
> (gdb) bt
> #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> #1 0x00007ffff7c8626a in __GI__IO_fputs (str=0x0, fp=0x7ffff7e044e0
> <_IO_2_1_stderr_>) at ./libio/iofputs.c:33
> #2 0x00005555555baf34 in vfinfo (fp=0x7ffff7e044e0 <_IO_2_1_stderr_>,
> fmt=0x55555596d243 "' in subsection '%s'\n", ap=0x7fffffffd630,
> is_warning=true) at ../../ld/ldmisc.c:527
> #3 0x00005555555bb36e in einfo (fmt=0x55555596d1f0 "%X%pB, %pB: error:
> mismatching values %#x and %#x for required object attribute '%s' in
> subsection '%s'\n") at ../../ld/ldmisc.c:608
> #4 0x000055555566dfc8 in report_mismatching_required_obj_attr
> (info=0x5555559d7ce0 <link_info>, ref_bfd=0x555555a14dc0,
> abfd=0x555555a17160, s_ref=0x555555a19590, a_ref=0x555555a19610,
> a_abfd=0x555555a1fdf0)
> at ../../bfd/elf-attrs.c:1077
> #5 0x000055555566e20c in oav2_subsection_perfect_match
> (info=0x5555559d7ce0 <link_info>, ref_bfd=0x555555a14dc0,
> abfd=0x555555a17160, s_ref=0x555555a19590, s_abfd=0x555555a1fd70) at
> ../../bfd/elf-attrs.c:1139
>
> It does not seem to me that the support for "#" was implemented in
vfinfo().
Hmm, I'm sorry for the bad request then. printf()-like functions which
aren't really printf()-like are problematic imo, but of course that's
entirely unrelated top your work here.
>> +/* Merge case 3: S_ABFD does not have a S_REF equivalent.
>> + 1. Create a new default-initialized S_REF subsection.
>> + 2. Merge S_ABFD into S_REF.
>> + 3. Insert S_REF into REF. */
>> +static bool
>> +handle_subsection_additional (struct bfd_link_info *info,
>> + bfd *ref_bfd, bfd *abfd,
>> + obj_attr_subsection_v2 *s_ref_next,
>> + obj_attr_subsection_v2 *s_abfd,
>> + obj_attr_subsection_v2 *s_frozen)
>
> There's nothing here that REF in the comment matches.
>
> (Giving up here.)
>
The comment starts by stating that there is not equivalent S_REF for
S_ABFD, i.e. S_REF does not appear in the input parameters.
What about the following phrasing ? Is it clearer ?
Merge case 3: S_ABFD does not have an equivalent subsection in the
current merge result.
1. Create a new default-initialized subsection S_REF from S_ABFD's
properties.
2. Merge S_ABFD into S_REF.
3. Insert S_REF into the current merge result right before S_REF_NEXT.
>> --- a/bfd/elf-attrs.h
>> +++ b/bfd/elf-attrs.h
>> @@ -55,6 +55,16 @@ typedef union obj_attr_value_v2 {
>> const char* string_val;
>> } obj_attr_value_v2;
>>
>> +typedef enum obj_attr_v2_status
>> +{
>> + /* An attribute that is unknown to the linker, and so cannot be merged. */
>> + obj_attr_v2_unknown = 0,
>> + /* An attribute that was reported as corrupted. */
>> + obj_attr_v2_corrupted,
>> + /* A valid attribute. */
>> + obj_attr_v2_ok,
>> +} obj_attr_v2_status;
>
> Iirc already on an earlier version I asked that typedef-s please either be
> omitted or be identified as such (by a _t suffix).
>
Do you mean only for the enums or everything, even structs ?
>> @@ -153,3 +179,36 @@ obj_attr_v2_find_known_by_tag (const struct elf_backend_data *,
>> extern const char *
>> obj_attr_v2_tag_to_string (const struct elf_backend_data *, const char*,
>> obj_attr_tag_t);
>> +
>> +enum obj_attr_v2_merge_result_reason
>> +{
>> + /* Default: everything is ok. */
>> + MERGE_OK = 0,
>> + /* The result value of the merge is the same as REF. */
>> + SAME_VALUE_AS_REF,
>> + /* No implementation of a merge for this attribute exists. */
>> + UNSUPPORTED,
>> + /* The merge failed, an error message should be logged. */
>> + ERROR,
>> +};
>
> These identifiers would be fine if they lived in a .c file. For them to live in
> a header, I think they need some disambiguating prefix. I also assume there's
> no dependency anywhere that would require MERGE_OK to explictly have value 0
> assigned (which it would get anyway if the "= 0" was dropped)?
>
What about OAv2_MERGE_STATUS_(OK, SAME_VALUE_AS_REF, UNSUPPORTED, ERROR) ?
>> +typedef struct {
>> + /* Should the merge be performed ? */
>> + bool merge;
>> + /* The merged value. */
>> + union obj_attr_value_v2 val;
>> + /* If the merge should not be performed, give the reason to differentiate
>> + error cases from normal cases. Typically, if REF already is set to the
>> + same value as the merged result, no merge is needed, and this is not an
>> + error. */
>> + enum obj_attr_v2_merge_result_reason reason;
>> +} obj_attr_v2_merge_result;
>> +
>> +/* Re-usable merge policies. */
>> +/* For now, only AND-merge is used by AArch64 backend. Additional policies
>> + (Integer-OR, String-ADD) are part of the GNU testing namespace. If they
>> + appear to be usefull for a backend at some point, they should be exposed
>> + to the backend here below. */
>> +extern obj_attr_v2_merge_result
>> +obj_attr_v2_merge_policy_AND (struct bfd_link_info *, bfd *,
>> + obj_attr_subsection_v2 *, obj_attr_v2 *,
>> + obj_attr_v2 *, obj_attr_v2 *);
>
> May want a _bfd_ or bfd_ (depending on where it's used) prefix again?
>
See previous answer. This is an internal function only used by bfd backends.
>> --- a/bfd/elf-bfd.h
>> +++ b/bfd/elf-bfd.h
>> @@ -1662,6 +1662,26 @@ struct elf_backend_data
>> /* The size of the array of known subsections. */
>> const size_t obj_attr_v2_known_subsections_size;
>>
>> + /* Translate the relevant GNU properties to object attributes v2. */
>> + void (*translate_relevant_gnu_props_to_obj_attrs) (bfd *,
>> + elf_property_list *);
>> +
>> + /* Translate the relevant object attributes v2 to GNU properties. */
>> + void (*translate_relevant_obj_attrs_to_gnu_props) (bfd *,
>> + obj_attr_subsection_v2 *);
>
> What is "relevant" intended to convey in the names?
>
"Relevant" in this context means object attributes that have GNU
properties equivalents.
What about this ? Is it clearer ?
/* Translate object attributes that have GNU property equivalents. */
And a similar phrasing for the reciprocal function:
/* Translate GNU properties that have object attributes v2 equivalents. */
>> + /* Check build attributes subsection v2 against expected properties. */
>> + bool (*obj_attr_subsection_v2_match_known) (struct bfd_link_info *, bfd *,
>> + obj_attr_subsection_v2 *);
>
> Comment says "expected" while the function name says "known". I think it's
> meant to be the latter?
>
Changed the comment to:
/* Check whether an object attributes subsection v2 is known by the
backend,
and if so, check whether the subsection's properties match the
expected
ones. */
>> + /* Get default value for an attribute. */
>> + bool (*obj_attr_v2_default_value) (struct bfd_link_info *,
>> + const obj_attr_info_t *, obj_attr_subsection_v2 *, obj_attr_v2 *);
>> +
>> + /* Merge a build attribute v2. */
>> + obj_attr_v2_merge_result (*obj_attr_v2_tag_merge) (struct bfd_link_info *,
>> + bfd *, obj_attr_subsection_v2 *, obj_attr_v2 *, obj_attr_v2 *, obj_attr_v2 *);
>
> "build attribute" in the comment?
>
That's a miss of when we changed the name to "object attributes".
I went through all the patch series and fixed the occurences here and
there in the next revision.
>> @@ -1683,6 +1703,9 @@ struct elf_backend_data
>> bool (*merge_gnu_properties) (struct bfd_link_info *, bfd *, bfd *,
>> elf_property *, elf_property *);
>>
>> + /* Set up build attributes. */
>> + bfd *(*setup_build_attributes) (struct bfd_link_info *);
>
> Hmm, more "build" here. Am I missing something?
>
See my previous answer.
>> --- a/bfd/elf-properties.c
>> +++ b/bfd/elf-properties.c
>> @@ -812,9 +812,9 @@ _bfd_elf_link_setup_gnu_properties (struct bfd_link_info *info)
>> return NULL;
>>
>> /* Merge .note.gnu.property sections. */
>> - info->callbacks->minfo (_("\n"));
>> + info->callbacks->minfo ("\n");
>> info->callbacks->minfo (_("Merging program properties\n"));
>> - info->callbacks->minfo (_("\n"));
>> + info->callbacks->minfo ("\n");
>
> Entirely unrelated change in an already overly large patch?
Reverted.
>
> Jan
Matthieu
More information about the Binutils
mailing list