[PATCH] ld: aarch64: Fix scanning of GNU properties for AARCH64_FEATURE_1_AND

[Yury Khrustalev]

Hi Matthieu,

> > > This loop is run after _bfd_elf_link_setup_gnu_properties which should
> > > already have merged and sorted the GNU properties.
> > > 
> > > Consequently, GNU_PROPERTY_MEMORY_SEAL (=0x3) should be located before
> > > GNU_PROPERTY_AARCH64_FEATURE_1_AND (=0xc0000000), and the shortcut exit
> > > condition of the loop should still be valid.
> > 
> > Why do we need this condition in the first place?
> > 
> > ```
> > GNU_PROPERTY_AARCH64_FEATURE_1_AND <= p->property.pr_type
> > ```
> > 
> > Why do we need to stop the loop as soon as we reach the property with the
> > number more than `AARCH64_FEATURE_1_AND` (thus depending on the order of
> > properties)?
> > 
> 
> This exit condition is only an optimization.

I think we should not use this condition in the loop as it's not likely to
help but is error-prone. We should remove it anyway.


> Since the collection of GNU
> properties is assumed to be sorted at this stage, it makes sense to apply a
> shortcut if the current properties is greater than
> GNU_PROPERTY_AARCH64_FEATURE_1_AND. Otherwise what is the point of sorting
> the GNU properties in the first place ?

I don't know why we do this. Given that we search by looping over the list
anyway, it seems redundant.

> The issue you experienced is a symptom of a bug in the sorting of the GNU
> properties.


I'm happy to fix this here, however I will also drop the condition in the
search loop.

> Minor note: replacing this for-loop by a find that returns a pointer on the
> GNU property would make the code more explicit regarding the intent. I am
> sure there are plenty of places where we use a raw for-loop search, and mix
> other unrelated logic into it. It would be nice to clean this up.

Agreed, but this is for another time as it's not related to the bug fix.

Kind regards,
Yury