Re: [PATCH 1/2] Initial support for GNU_PROPERTY_1_NEEDED

["H.J. Lu via Libc-alpha" <libc-alpha@sourceware.org>]

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.

-- 
H.J.