[PATCH 09/17] x86: Check PT_GNU_PROPERTY early

[Noah Goldstein]

On Wed, Dec 6, 2023 at 11:20 AM 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;
These seems like a seperate change from the refactor mentioned in the
commit message?
>  }
>
>  #endif /* _DL_PROP_H */
> --
> 2.43.0
>