[1/2] Initial support for GNU_PROPERTY_1_NEEDED

Message ID 20210803215914.4170913-1-hjl.tools@gmail.com
State Committed
Commit 1bd888d0b705fa5ab7b443724288f7ba12939533
Delegated to: Adhemerval Zanella Netto
Headers
Series [1/2] Initial support for GNU_PROPERTY_1_NEEDED |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

H.J. Lu Aug. 3, 2021, 9:59 p.m. UTC
  1. Add GNU_PROPERTY_1_NEEDED:

 #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO

to indicate the needed properties by the object file.
2. Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS:

 #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)

to indicate that the object file requires canonical function pointers and
cannot be used with copy relocation.
3. Scan GNU_PROPERTY_1_NEEDED property and store it in l_1_needed.
---
 elf/elf.h                  | 17 +++++++++++++++++
 sysdeps/generic/dl-prop.h  |  9 ++++++++-
 sysdeps/generic/link_map.h |  3 ++-
 sysdeps/x86/dl-prop.h      | 19 ++++++++++++++-----
 sysdeps/x86/link_map.h     |  2 ++
 5 files changed, 43 insertions(+), 7 deletions(-)
  

Comments

Adhemerval Zanella Oct. 7, 2021, 2:53 p.m. UTC | #1
On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote:
> 1. Add GNU_PROPERTY_1_NEEDED:
> 
>  #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO
> 
> to indicate the needed properties by the object file.
> 2. Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS:
> 
>  #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> 
> to indicate that the object file requires canonical function pointers and
> cannot be used with copy relocation.
> 3. Scan GNU_PROPERTY_1_NEEDED property and store it in l_1_needed.

It was added by binutils with commit 6320fd00dc374f74658, maybe add on
the commit the version expected to have this support.

LGTM, with a small suggestion below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/elf.h                  | 17 +++++++++++++++++
>  sysdeps/generic/dl-prop.h  |  9 ++++++++-
>  sysdeps/generic/link_map.h |  3 ++-
>  sysdeps/x86/dl-prop.h      | 19 ++++++++++++++-----
>  sysdeps/x86/link_map.h     |  2 ++
>  5 files changed, 43 insertions(+), 7 deletions(-)
> 
> diff --git a/elf/elf.h b/elf/elf.h
> index 4738dfa28f..50f87baceb 100644
> --- a/elf/elf.h
> +++ b/elf/elf.h
> @@ -1312,6 +1312,23 @@ typedef struct
>  /* No copy relocation on protected data symbol.  */
>  #define GNU_PROPERTY_NO_COPY_ON_PROTECTED	2
>  
> +/* A 4-byte unsigned integer property: A bit is set if it is set in all
> +   relocatable inputs.  */
> +#define GNU_PROPERTY_UINT32_AND_LO	0xb0000000
> +#define GNU_PROPERTY_UINT32_AND_HI	0xb0007fff
> +
> +/* A 4-byte unsigned integer property: A bit is set if it is set in any
> +   relocatable inputs.  */
> +#define GNU_PROPERTY_UINT32_OR_LO	0xb0008000
> +#define GNU_PROPERTY_UINT32_OR_HI	0xb000ffff
> +
> +/* The needed properties by the object file.  */
> +#define GNU_PROPERTY_1_NEEDED		GNU_PROPERTY_UINT32_OR_LO
> +
> +/* Set if the object file requires canonical function pointers and
> +   cannot be used with copy relocation.  */
> +#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> +
>  /* Processor-specific semantics, lo */
>  #define GNU_PROPERTY_LOPROC			0xc0000000
>  /* Processor-specific semantics, hi */

Ok.

> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> index eaee8052b6..207aadb35d 100644
> --- a/sysdeps/generic/dl-prop.h
> +++ b/sysdeps/generic/dl-prop.h
> @@ -47,7 +47,14 @@ 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;
> +  /* Continue until GNU_PROPERTY_1_NEEDED is found.  */
> +  if (type == GNU_PROPERTY_1_NEEDED)
> +    {
> +      if (datasz == 4)
> +	l->l_1_needed = *(unsigned int *) data;

Although the pointer seems to be aligned, I think it would be better to
use memcpy here:

   unsigned int t;
   memcpy (&t, data, sizeof (t));
   l->l_1_needed = t;

(the x86 header might use a more relax since the architecture always
support unaligned access)

> +      return 0;
> +    }
> +  return 1;
>  }
>  
>  #endif /* _DL_PROP_H */
> diff --git a/sysdeps/generic/link_map.h b/sysdeps/generic/link_map.h
> index a056184690..9f482b8c20 100644
> --- a/sysdeps/generic/link_map.h
> +++ b/sysdeps/generic/link_map.h
> @@ -1 +1,2 @@
> -/* No architecture specific definitions.  */
> +/* GNU_PROPERTY_1_NEEDED of this object.  */
> +unsigned int l_1_needed;
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index 56bd020b3c..385548fad3 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -97,6 +97,7 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>  
>    const ElfW(Addr) start = (ElfW(Addr)) note;
>  
> +  unsigned int needed_1 = 0;
>    unsigned int feature_1_and = 0;
>    unsigned int isa_1_needed = 0;
>    unsigned int last_type = 0;
> @@ -141,7 +142,8 @@ _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_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

Ok.

> @@ -151,12 +153,18 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>  
>  		  /* 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),
> +		  _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_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;

Ok.

> @@ -187,9 +195,10 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>      }
>  
>    /* We get here only if there is one or no GNU property note.  */
> -  if (isa_1_needed != 0 || feature_1_and != 0)
> +  if (needed_1 != 0 || isa_1_needed != 0 || feature_1_and != 0)
>      {
>        l->l_property = lc_property_valid;
> +      l->l_1_needed = needed_1;
>        l->l_x86_isa_1_needed = isa_1_needed;
>        l->l_x86_feature_1_and = feature_1_and;
>      }

Ok.

> diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h
> index 4c46a25f83..0c7e25dc96 100644
> --- a/sysdeps/x86/link_map.h
> +++ b/sysdeps/x86/link_map.h
> @@ -29,3 +29,5 @@ unsigned int l_x86_feature_1_and;
>  
>  /* GNU_PROPERTY_X86_ISA_1_NEEDED of this object.  */
>  unsigned int l_x86_isa_1_needed;
> +
> +#include <sysdeps/generic/link_map.h>
> 

Ok.
  
H.J. Lu Oct. 7, 2021, 3:11 p.m. UTC | #2
On Thu, Oct 7, 2021 at 7:53 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote:
> > 1. Add GNU_PROPERTY_1_NEEDED:
> >
> >  #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO
> >
> > to indicate the needed properties by the object file.
> > 2. Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS:
> >
> >  #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> >
> > to indicate that the object file requires canonical function pointers and
> > cannot be used with copy relocation.
> > 3. Scan GNU_PROPERTY_1_NEEDED property and store it in l_1_needed.
>
> It was added by binutils with commit 6320fd00dc374f74658, maybe add on
> the commit the version expected to have this support.
>
> LGTM, with a small suggestion below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> > ---
> >  elf/elf.h                  | 17 +++++++++++++++++
> >  sysdeps/generic/dl-prop.h  |  9 ++++++++-
> >  sysdeps/generic/link_map.h |  3 ++-
> >  sysdeps/x86/dl-prop.h      | 19 ++++++++++++++-----
> >  sysdeps/x86/link_map.h     |  2 ++
> >  5 files changed, 43 insertions(+), 7 deletions(-)
> >
> > diff --git a/elf/elf.h b/elf/elf.h
> > index 4738dfa28f..50f87baceb 100644
> > --- a/elf/elf.h
> > +++ b/elf/elf.h
> > @@ -1312,6 +1312,23 @@ typedef struct
> >  /* No copy relocation on protected data symbol.  */
> >  #define GNU_PROPERTY_NO_COPY_ON_PROTECTED    2
> >
> > +/* A 4-byte unsigned integer property: A bit is set if it is set in all
> > +   relocatable inputs.  */
> > +#define GNU_PROPERTY_UINT32_AND_LO   0xb0000000
> > +#define GNU_PROPERTY_UINT32_AND_HI   0xb0007fff
> > +
> > +/* A 4-byte unsigned integer property: A bit is set if it is set in any
> > +   relocatable inputs.  */
> > +#define GNU_PROPERTY_UINT32_OR_LO    0xb0008000
> > +#define GNU_PROPERTY_UINT32_OR_HI    0xb000ffff
> > +
> > +/* The needed properties by the object file.  */
> > +#define GNU_PROPERTY_1_NEEDED                GNU_PROPERTY_UINT32_OR_LO
> > +
> > +/* Set if the object file requires canonical function pointers and
> > +   cannot be used with copy relocation.  */
> > +#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> > +
> >  /* Processor-specific semantics, lo */
> >  #define GNU_PROPERTY_LOPROC                  0xc0000000
> >  /* Processor-specific semantics, hi */
>
> Ok.
>
> > diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> > index eaee8052b6..207aadb35d 100644
> > --- a/sysdeps/generic/dl-prop.h
> > +++ b/sysdeps/generic/dl-prop.h
> > @@ -47,7 +47,14 @@ 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;
> > +  /* Continue until GNU_PROPERTY_1_NEEDED is found.  */
> > +  if (type == GNU_PROPERTY_1_NEEDED)
> > +    {
> > +      if (datasz == 4)
> > +     l->l_1_needed = *(unsigned int *) data;
>
> Although the pointer seems to be aligned, I think it would be better to
> use memcpy here:
>
>    unsigned int t;
>    memcpy (&t, data, sizeof (t));
>    l->l_1_needed = t;
>
> (the x86 header might use a more relax since the architecture always
> support unaligned access)

The GNU property is guaranteed to be properly aligned:

 void
_dl_process_pt_gnu_property (struct link_map *l, int fd, const ElfW(Phdr) *ph)
{
  const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
  const ElfW(Addr) size = ph->p_memsz;
  const ElfW(Addr) align = ph->p_align;

  /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
     32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
     with incorrect alignment.  */
  if (align != (__ELF_NATIVE_CLASS / 8))
    return;

data passed to _dl_process_gnu_property should be properly aligned.

> > +      return 0;
> > +    }
> > +  return 1;
> >  }
> >
> >  #endif /* _DL_PROP_H */
> > diff --git a/sysdeps/generic/link_map.h b/sysdeps/generic/link_map.h
> > index a056184690..9f482b8c20 100644
> > --- a/sysdeps/generic/link_map.h
> > +++ b/sysdeps/generic/link_map.h
> > @@ -1 +1,2 @@
> > -/* No architecture specific definitions.  */
> > +/* GNU_PROPERTY_1_NEEDED of this object.  */
> > +unsigned int l_1_needed;
> > diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> > index 56bd020b3c..385548fad3 100644
> > --- a/sysdeps/x86/dl-prop.h
> > +++ b/sysdeps/x86/dl-prop.h
> > @@ -97,6 +97,7 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >
> >    const ElfW(Addr) start = (ElfW(Addr)) note;
> >
> > +  unsigned int needed_1 = 0;
> >    unsigned int feature_1_and = 0;
> >    unsigned int isa_1_needed = 0;
> >    unsigned int last_type = 0;
> > @@ -141,7 +142,8 @@ _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_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
>
> Ok.
>
> > @@ -151,12 +153,18 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >
> >                 /* 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),
> > +               _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_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;
>
> Ok.
>
> > @@ -187,9 +195,10 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >      }
> >
> >    /* We get here only if there is one or no GNU property note.  */
> > -  if (isa_1_needed != 0 || feature_1_and != 0)
> > +  if (needed_1 != 0 || isa_1_needed != 0 || feature_1_and != 0)
> >      {
> >        l->l_property = lc_property_valid;
> > +      l->l_1_needed = needed_1;
> >        l->l_x86_isa_1_needed = isa_1_needed;
> >        l->l_x86_feature_1_and = feature_1_and;
> >      }
>
> Ok.
>
> > diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h
> > index 4c46a25f83..0c7e25dc96 100644
> > --- a/sysdeps/x86/link_map.h
> > +++ b/sysdeps/x86/link_map.h
> > @@ -29,3 +29,5 @@ unsigned int l_x86_feature_1_and;
> >
> >  /* GNU_PROPERTY_X86_ISA_1_NEEDED of this object.  */
> >  unsigned int l_x86_isa_1_needed;
> > +
> > +#include <sysdeps/generic/link_map.h>
> >
>
> Ok.
  
Adhemerval Zanella Oct. 7, 2021, 4:53 p.m. UTC | #3
On 07/10/2021 12:11, H.J. Lu wrote:
> On Thu, Oct 7, 2021 at 7:53 AM Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>
>>
>>
>> On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote:
>>> 1. Add GNU_PROPERTY_1_NEEDED:
>>>
>>>  #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO
>>>
>>> to indicate the needed properties by the object file.
>>> 2. Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS:
>>>
>>>  #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
>>>
>>> to indicate that the object file requires canonical function pointers and
>>> cannot be used with copy relocation.
>>> 3. Scan GNU_PROPERTY_1_NEEDED property and store it in l_1_needed.
>>
>> It was added by binutils with commit 6320fd00dc374f74658, maybe add on
>> the commit the version expected to have this support.
>>
>> LGTM, with a small suggestion below.
>>
>> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>>
>>> ---
>>>  elf/elf.h                  | 17 +++++++++++++++++
>>>  sysdeps/generic/dl-prop.h  |  9 ++++++++-
>>>  sysdeps/generic/link_map.h |  3 ++-
>>>  sysdeps/x86/dl-prop.h      | 19 ++++++++++++++-----
>>>  sysdeps/x86/link_map.h     |  2 ++
>>>  5 files changed, 43 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/elf/elf.h b/elf/elf.h
>>> index 4738dfa28f..50f87baceb 100644
>>> --- a/elf/elf.h
>>> +++ b/elf/elf.h
>>> @@ -1312,6 +1312,23 @@ typedef struct
>>>  /* No copy relocation on protected data symbol.  */
>>>  #define GNU_PROPERTY_NO_COPY_ON_PROTECTED    2
>>>
>>> +/* A 4-byte unsigned integer property: A bit is set if it is set in all
>>> +   relocatable inputs.  */
>>> +#define GNU_PROPERTY_UINT32_AND_LO   0xb0000000
>>> +#define GNU_PROPERTY_UINT32_AND_HI   0xb0007fff
>>> +
>>> +/* A 4-byte unsigned integer property: A bit is set if it is set in any
>>> +   relocatable inputs.  */
>>> +#define GNU_PROPERTY_UINT32_OR_LO    0xb0008000
>>> +#define GNU_PROPERTY_UINT32_OR_HI    0xb000ffff
>>> +
>>> +/* The needed properties by the object file.  */
>>> +#define GNU_PROPERTY_1_NEEDED                GNU_PROPERTY_UINT32_OR_LO
>>> +
>>> +/* Set if the object file requires canonical function pointers and
>>> +   cannot be used with copy relocation.  */
>>> +#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
>>> +
>>>  /* Processor-specific semantics, lo */
>>>  #define GNU_PROPERTY_LOPROC                  0xc0000000
>>>  /* Processor-specific semantics, hi */
>>
>> Ok.
>>
>>> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
>>> index eaee8052b6..207aadb35d 100644
>>> --- a/sysdeps/generic/dl-prop.h
>>> +++ b/sysdeps/generic/dl-prop.h
>>> @@ -47,7 +47,14 @@ 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;
>>> +  /* Continue until GNU_PROPERTY_1_NEEDED is found.  */
>>> +  if (type == GNU_PROPERTY_1_NEEDED)
>>> +    {
>>> +      if (datasz == 4)
>>> +     l->l_1_needed = *(unsigned int *) data;
>>
>> Although the pointer seems to be aligned, I think it would be better to
>> use memcpy here:
>>
>>    unsigned int t;
>>    memcpy (&t, data, sizeof (t));
>>    l->l_1_needed = t;
>>
>> (the x86 header might use a more relax since the architecture always
>> support unaligned access)
> 
> The GNU property is guaranteed to be properly aligned:
> 
>  void
> _dl_process_pt_gnu_property (struct link_map *l, int fd, const ElfW(Phdr) *ph)
> {
>   const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
>   const ElfW(Addr) size = ph->p_memsz;
>   const ElfW(Addr) align = ph->p_align;
> 
>   /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
>      32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
>      with incorrect alignment.  */
>   if (align != (__ELF_NATIVE_CLASS / 8))
>     return;
> 
> data passed to _dl_process_gnu_property should be properly aligned.

Alright, the memcpy seems unnecessary then.
  
H.J. Lu Oct. 7, 2021, 5:04 p.m. UTC | #4
On Thu, Oct 7, 2021 at 9:53 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 07/10/2021 12:11, H.J. Lu wrote:
> > On Thu, Oct 7, 2021 at 7:53 AM Adhemerval Zanella
> > <adhemerval.zanella@linaro.org> wrote:
> >>
> >>
> >>
> >> On 03/08/2021 18:59, H.J. Lu via Libc-alpha wrote:
> >>> 1. Add GNU_PROPERTY_1_NEEDED:
> >>>
> >>>  #define GNU_PROPERTY_1_NEEDED      GNU_PROPERTY_UINT32_OR_LO
> >>>
> >>> to indicate the needed properties by the object file.
> >>> 2. Add GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS:
> >>>
> >>>  #define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> >>>
> >>> to indicate that the object file requires canonical function pointers and
> >>> cannot be used with copy relocation.
> >>> 3. Scan GNU_PROPERTY_1_NEEDED property and store it in l_1_needed.
> >>
> >> It was added by binutils with commit 6320fd00dc374f74658, maybe add on
> >> the commit the version expected to have this support.
> >>
> >> LGTM, with a small suggestion below.
> >>
> >> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
> >>
> >>> ---
> >>>  elf/elf.h                  | 17 +++++++++++++++++
> >>>  sysdeps/generic/dl-prop.h  |  9 ++++++++-
> >>>  sysdeps/generic/link_map.h |  3 ++-
> >>>  sysdeps/x86/dl-prop.h      | 19 ++++++++++++++-----
> >>>  sysdeps/x86/link_map.h     |  2 ++
> >>>  5 files changed, 43 insertions(+), 7 deletions(-)
> >>>
> >>> diff --git a/elf/elf.h b/elf/elf.h
> >>> index 4738dfa28f..50f87baceb 100644
> >>> --- a/elf/elf.h
> >>> +++ b/elf/elf.h
> >>> @@ -1312,6 +1312,23 @@ typedef struct
> >>>  /* No copy relocation on protected data symbol.  */
> >>>  #define GNU_PROPERTY_NO_COPY_ON_PROTECTED    2
> >>>
> >>> +/* A 4-byte unsigned integer property: A bit is set if it is set in all
> >>> +   relocatable inputs.  */
> >>> +#define GNU_PROPERTY_UINT32_AND_LO   0xb0000000
> >>> +#define GNU_PROPERTY_UINT32_AND_HI   0xb0007fff
> >>> +
> >>> +/* A 4-byte unsigned integer property: A bit is set if it is set in any
> >>> +   relocatable inputs.  */
> >>> +#define GNU_PROPERTY_UINT32_OR_LO    0xb0008000
> >>> +#define GNU_PROPERTY_UINT32_OR_HI    0xb000ffff
> >>> +
> >>> +/* The needed properties by the object file.  */
> >>> +#define GNU_PROPERTY_1_NEEDED                GNU_PROPERTY_UINT32_OR_LO
> >>> +
> >>> +/* Set if the object file requires canonical function pointers and
> >>> +   cannot be used with copy relocation.  */
> >>> +#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
> >>> +
> >>>  /* Processor-specific semantics, lo */
> >>>  #define GNU_PROPERTY_LOPROC                  0xc0000000
> >>>  /* Processor-specific semantics, hi */
> >>
> >> Ok.
> >>
> >>> diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
> >>> index eaee8052b6..207aadb35d 100644
> >>> --- a/sysdeps/generic/dl-prop.h
> >>> +++ b/sysdeps/generic/dl-prop.h
> >>> @@ -47,7 +47,14 @@ 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;
> >>> +  /* Continue until GNU_PROPERTY_1_NEEDED is found.  */
> >>> +  if (type == GNU_PROPERTY_1_NEEDED)
> >>> +    {
> >>> +      if (datasz == 4)
> >>> +     l->l_1_needed = *(unsigned int *) data;
> >>
> >> Although the pointer seems to be aligned, I think it would be better to
> >> use memcpy here:
> >>
> >>    unsigned int t;
> >>    memcpy (&t, data, sizeof (t));
> >>    l->l_1_needed = t;
> >>
> >> (the x86 header might use a more relax since the architecture always
> >> support unaligned access)
> >
> > The GNU property is guaranteed to be properly aligned:
> >
> >  void
> > _dl_process_pt_gnu_property (struct link_map *l, int fd, const ElfW(Phdr) *ph)
> > {
> >   const ElfW(Nhdr) *note = (const void *) (ph->p_vaddr + l->l_addr);
> >   const ElfW(Addr) size = ph->p_memsz;
> >   const ElfW(Addr) align = ph->p_align;
> >
> >   /* The NT_GNU_PROPERTY_TYPE_0 note must be aligned to 4 bytes in
> >      32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
> >      with incorrect alignment.  */
> >   if (align != (__ELF_NATIVE_CLASS / 8))
> >     return;
> >
> > data passed to _dl_process_gnu_property should be properly aligned.
>
> Alright, the memcpy seems unnecessary then.

I will check in my patch ASIS.

Thanks.
  

Patch

diff --git a/elf/elf.h b/elf/elf.h
index 4738dfa28f..50f87baceb 100644
--- a/elf/elf.h
+++ b/elf/elf.h
@@ -1312,6 +1312,23 @@  typedef struct
 /* No copy relocation on protected data symbol.  */
 #define GNU_PROPERTY_NO_COPY_ON_PROTECTED	2
 
+/* A 4-byte unsigned integer property: A bit is set if it is set in all
+   relocatable inputs.  */
+#define GNU_PROPERTY_UINT32_AND_LO	0xb0000000
+#define GNU_PROPERTY_UINT32_AND_HI	0xb0007fff
+
+/* A 4-byte unsigned integer property: A bit is set if it is set in any
+   relocatable inputs.  */
+#define GNU_PROPERTY_UINT32_OR_LO	0xb0008000
+#define GNU_PROPERTY_UINT32_OR_HI	0xb000ffff
+
+/* The needed properties by the object file.  */
+#define GNU_PROPERTY_1_NEEDED		GNU_PROPERTY_UINT32_OR_LO
+
+/* Set if the object file requires canonical function pointers and
+   cannot be used with copy relocation.  */
+#define GNU_PROPERTY_1_NEEDED_INDIRECT_EXTERN_ACCESS (1U << 0)
+
 /* Processor-specific semantics, lo */
 #define GNU_PROPERTY_LOPROC			0xc0000000
 /* Processor-specific semantics, hi */
diff --git a/sysdeps/generic/dl-prop.h b/sysdeps/generic/dl-prop.h
index eaee8052b6..207aadb35d 100644
--- a/sysdeps/generic/dl-prop.h
+++ b/sysdeps/generic/dl-prop.h
@@ -47,7 +47,14 @@  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;
+  /* Continue until GNU_PROPERTY_1_NEEDED is found.  */
+  if (type == GNU_PROPERTY_1_NEEDED)
+    {
+      if (datasz == 4)
+	l->l_1_needed = *(unsigned int *) data;
+      return 0;
+    }
+  return 1;
 }
 
 #endif /* _DL_PROP_H */
diff --git a/sysdeps/generic/link_map.h b/sysdeps/generic/link_map.h
index a056184690..9f482b8c20 100644
--- a/sysdeps/generic/link_map.h
+++ b/sysdeps/generic/link_map.h
@@ -1 +1,2 @@ 
-/* No architecture specific definitions.  */
+/* GNU_PROPERTY_1_NEEDED of this object.  */
+unsigned int l_1_needed;
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 56bd020b3c..385548fad3 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -97,6 +97,7 @@  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
 
   const ElfW(Addr) start = (ElfW(Addr)) note;
 
+  unsigned int needed_1 = 0;
   unsigned int feature_1_and = 0;
   unsigned int isa_1_needed = 0;
   unsigned int last_type = 0;
@@ -141,7 +142,8 @@  _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_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
@@ -151,12 +153,18 @@  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
 
 		  /* 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),
+		  _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_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;
@@ -187,9 +195,10 @@  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
     }
 
   /* We get here only if there is one or no GNU property note.  */
-  if (isa_1_needed != 0 || feature_1_and != 0)
+  if (needed_1 != 0 || isa_1_needed != 0 || feature_1_and != 0)
     {
       l->l_property = lc_property_valid;
+      l->l_1_needed = needed_1;
       l->l_x86_isa_1_needed = isa_1_needed;
       l->l_x86_feature_1_and = feature_1_and;
     }
diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h
index 4c46a25f83..0c7e25dc96 100644
--- a/sysdeps/x86/link_map.h
+++ b/sysdeps/x86/link_map.h
@@ -29,3 +29,5 @@  unsigned int l_x86_feature_1_and;
 
 /* GNU_PROPERTY_X86_ISA_1_NEEDED of this object.  */
 unsigned int l_x86_isa_1_needed;
+
+#include <sysdeps/generic/link_map.h>