[PATCH] x86: Check PT_GNU_PROPERTY early
Noah Goldstein
goldstein.w.n@gmail.com
Mon Dec 11 18:18:19 GMT 2023
On Mon, Dec 11, 2023 at 10:38 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Dec 8, 2023 at 9:04 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > On Thu, Dec 7, 2023 at 3:05 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> > >
> > > The PT_GNU_PROPERTY segment is scanned before PT_NOTE. For binaries
> > > with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
> > > the PT_NOTE segment.
> > > ---
> > > sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
> > > 1 file changed, 80 insertions(+), 40 deletions(-)
> > >
> > > diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> > > index b2836f3009..d2c53c2182 100644
> > > --- a/sysdeps/x86/dl-prop.h
> > > +++ b/sysdeps/x86/dl-prop.h
> > > @@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
> > > #endif
> > > }
> > >
> > > +/* Check the GNU property and return its value. It returns:
> > > + -1: Skip this note.
> > > + 0: Stop checking.
> > > + 1: Continue to check.
> > > + */
> > > +static inline int
> > > +_dl_check_gnu_property (unsigned int type, unsigned int datasz,
> > > + void *ptr, unsigned int *feature_1_and,
> > > + unsigned int *needed_1,
> > > + unsigned int *isa_1_needed)
> > > +{
> > > + if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > > + || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > > + || type == GNU_PROPERTY_1_NEEDED)
> > > + {
> > > + /* The sizes of types which we are searching for are
> > > + 4 bytes. There is no point to continue if this
> > > + note is ill-formed. */
> > > + if (datasz != 4)
> > > + return -1;
> > > +
> > > + /* NB: Stop the scan only after seeing all types which
> > > + we are searching for. */
> > > + _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > > + > GNU_PROPERTY_X86_FEATURE_1_AND)
> > > + && (GNU_PROPERTY_X86_FEATURE_1_AND
> > > + > GNU_PROPERTY_1_NEEDED)),
> > > + "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > > + "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > > + "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > > + "GNU_PROPERTY_1_NEEDED");
> > > + if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > > + *feature_1_and = *(unsigned int *) ptr;
> > > + else if (type == GNU_PROPERTY_1_NEEDED)
> > > + *needed_1 = *(unsigned int *) ptr;
> > > + else
> > > + {
> > > + *isa_1_needed = *(unsigned int *) ptr;
> > > +
> > > + /* Keep searching for the next GNU property note
> > > + generated by the older linker. */
> > > + return 0;
> > > + }
> > > + }
> > > + else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > > + {
> > > + /* Stop the scan since property type is in ascending
> > > + order. */
> > > + return 0;
> > > + }
> > > +
> > > + return 1;
> > > +}
> > > +
> > > static inline void __attribute__ ((unused))
> > > _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> > > const ElfW(Addr) size, const ElfW(Addr) align)
> > > @@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> > >
> > > last_type = type;
> > >
> > > - if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > > - || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > > - || type == GNU_PROPERTY_1_NEEDED)
> > > - {
> > > - /* The sizes of types which we are searching for are
> > > - 4 bytes. There is no point to continue if this
> > > - note is ill-formed. */
> > > - if (datasz != 4)
> > > - return;
> > > -
> > > - /* NB: Stop the scan only after seeing all types which
> > > - we are searching for. */
> > > - _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > > - > GNU_PROPERTY_X86_FEATURE_1_AND)
> > > - && (GNU_PROPERTY_X86_FEATURE_1_AND
> > > - > GNU_PROPERTY_1_NEEDED)),
> > > - "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > > - "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > > - "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > > - "GNU_PROPERTY_1_NEEDED");
> > > - if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > > - feature_1_and = *(unsigned int *) ptr;
> > > - else if (type == GNU_PROPERTY_1_NEEDED)
> > > - needed_1 = *(unsigned int *) ptr;
> > > - else
> > > - {
> > > - isa_1_needed = *(unsigned int *) ptr;
> > > -
> > > - /* Keep searching for the next GNU property note
> > > - generated by the older linker. */
> > > - break;
> > > - }
> > > - }
> > > - else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > > - {
> > > - /* Stop the scan since property type is in ascending
> > > - order. */
> > > - break;
> > > - }
> > > + int result = _dl_check_gnu_property (type, datasz, ptr,
> > > + &feature_1_and,
> > > + &needed_1,
> > > + &isa_1_needed);
> > > + if (result == -1)
> > > + return; /* Skip this note. */
> > > + else if (result == 0)
> > > + break; /* Stop checking. */
> > >
> > > /* Check the next property item. */
> > > ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> > > @@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
> > > _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
> > > uint32_t datasz, void *data)
> > > {
> > > - return 0;
> > > + /* This is called on each GNU property. */
> > > + unsigned int needed_1 = 0;
> > > + unsigned int feature_1_and = 0;
> > > + unsigned int isa_1_needed = 0;
> > > + int result = _dl_check_gnu_property (type, datasz, data,
> > > + &feature_1_and, &needed_1,
> > > + &isa_1_needed);
> > > + if (needed_1 != 0)
> > > + l->l_1_needed = needed_1;
> > > + if (isa_1_needed != 0)
> > > + l->l_x86_isa_1_needed = isa_1_needed;
> > > + if (feature_1_and != 0)
> > > + l->l_x86_feature_1_and = feature_1_and;
> > > + if ((needed_1 | isa_1_needed | feature_1_and) != 0)
> > > + l->l_property = lc_property_valid;
> > > + else if (l->l_property == lc_property_unknown)
> > > + l->l_property = lc_property_none;
> > > + return result <= 0 ? 0 : result;
> > > }
> > >
> > > #endif /* _DL_PROP_H */
> > > --
> > > 2.43.0
> > >
> >
> > Is this the split patch? It seems to both have the refactor and new
> > functionality.
>
> This is a new functionality by adding _dl_check_gnu_property.
>
err i was thinking of splitting the refactor (creating `_dl_check_gnu_property`)
to a seperate patch but this is fine as is.
LGTM
Reviewed-by: Noah Goldstein <goldstein.w.n@gmail.com>
>
> --
> H.J.
More information about the Libc-alpha
mailing list