[PATCH v2 1/2] ld: aarch64: Fix scanning of GNU properties for AARCH64_FEATURE_1_AND

[Richard Earnshaw (lists)]

On 01/04/2025 16:31, Yury Khrustalev wrote:
> On Tue, Apr 01, 2025 at 04:23:27PM +0100, Richard Earnshaw (lists) wrote:
>>> @@ -936,8 +936,7 @@ _bfd_aarch64_elf_link_setup_gnu_properties (struct bfd_link_info *info)
>>>      {
>>>        /* The property list is sorted in order of type.  */
>>
>> So this comment is wrong and should be corrected, I presume.
> 
> No, the comment is correct, the properties are sorted by pr_type in
> ascending order, however since right now we are interested in only
> one property, we don't rely on or use this sorting in any way.
> 
> Also see Matthieu's comment in [1].
> 
> [1] https://inbox.sourceware.org/binutils/96953853-b125-485d-8499-9a5445f58849@arm.com/
> 
> Kind regards,
> Yury
> 

So, as just discussed face to face, I think this whole chunk of code is micro-optimized in the wrong direction.  We will only have a small number of properties on an object file and if it grew we'd probably want a completely different structure, like a hash table, to hold it.

It would make much more sense to have a function that will search and find a specific property, so that we
can simply ask if that existed.  The code here would then look like

  if ((p = get_property (pbfd, GNU_PROPERTY_AARCH64_FEATURE_1_AND)) != NULL)
    outprop = (p->property.u.number
	       & (GNU_PROPERTY_AARCH64_FEATURE_1_BTI
		  | GNU_PROPERTY_AARCH64_FEATURE_1_PAC
		  | GNU_PROPERTY_AARCH64_FEATURE_1_GCS));

and then repeat this style for as many properties that you need to handle.

get_property () is then free to do some micro-optimizations if it makes sense, but the change is much more 
self contained if updates are needed in the future.

R.