This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Add x32 support to dynamic linker audit


On Tue, Mar 20, 2012 at 12:54 PM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> Hi,
>
> This patch adds x32 support to dynamic linker audit. It checks __LP64__ and
> __x86_64__ for x32 support. ?It casts value to ptrdiff_t for printf %t.
> OK to install?
>
> Thanks.
>
>
> H.J.
> 2012-03-20 ?H.J. Lu ?<hongjiu.lu@intel.com>
>
> ? ? ? ?* elf/tst-auditmod1.c: Support la_x32_gnu_pltenter and
> ? ? ? ?la_x32_gnu_pltexit.
> ? ? ? ?(pltexit): Cast int_retval to ptrdiff_t.

Why are you casting this to ptrdiff_t?

The value of int_retval is actually lrv_eax which is uint32_t.

Did you choose ptrdiff_t because it *becomes* the right size and
signedness for all targets?

I would have expected uintptr_t to be used.

> ? ? ? ?* elf/tst-auditmod3b.c: Likewise.
> ? ? ? ?* elf/tst-auditmod4b.c: Likewise.
> ? ? ? ?* elf/tst-auditmod5b.c: Likewise.
> ? ? ? ?* elf/tst-auditmod6b.c: Likewise.
> ? ? ? ?* elf/tst-auditmod6c.c: Likewise.
> ? ? ? ?* elf/tst-auditmod7b.c: Likewise.

You don't need to modify the other tests also?

e.g. tst-auditmod1.c

#elif defined __x86_64__
# define pltenter la_x86_64_gnu_pltenter
# define pltexit la_x86_64_gnu_pltexit
# define La_regs La_x86_64_regs
# define La_retval La_x86_64_retval
# define int_retval lrv_rax
...

> ? ? ? ?* sysdeps/generic/ldsodefs.h (audit_ifaces): Add x32_gnu_pltenter
> ? ? ? ?and x32_gnu_pltexit.
>
> ? ? ? ?* sysdeps/x86_64/bits/link.h: Check __x86_64__ instead of
> ? ? ? ?__ELF_NATIVE_CLASS.
> ? ? ? ?(la_x32_gnu_pltenter): New.
> ? ? ? ?(la_x32_gnu_pltexit): Likewise.
>
> diff --git a/elf/tst-auditmod1.c b/elf/tst-auditmod1.c
> index 69da278..f762027 100644
> --- a/elf/tst-auditmod1.c
> +++ b/elf/tst-auditmod1.c
> @@ -109,8 +109,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ?# define La_retval La_i86_retval
> ?# define int_retval lrv_eax
> ?#elif defined __x86_64__
> -# define pltenter la_x86_64_gnu_pltenter
> -# define pltexit la_x86_64_gnu_pltexit
> +# ifdef __LP64__
> +# ?define pltenter la_x86_64_gnu_pltenter
> +# ?define pltexit la_x86_64_gnu_pltexit
> +# else
> +# ?define pltenter la_x32_gnu_pltenter
> +# ?define pltexit la_x32_gnu_pltexit
> +# endif
> ?# define La_regs La_x86_64_regs
> ?# define La_retval La_x86_64_retval
> ?# define int_retval lrv_rax
> @@ -188,7 +193,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ? return 0;
> ?}
> diff --git a/elf/tst-auditmod3b.c b/elf/tst-auditmod3b.c
> index 388ed6e..d1bb9b0 100644
> --- a/elf/tst-auditmod3b.c
> +++ b/elf/tst-auditmod3b.c
> @@ -105,8 +105,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -140,7 +145,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ? __m128i xmm = _mm_set1_epi32 (-1);
> ? asm volatile ("movdqa %0, %%xmm0" : : "x" (xmm) : "xmm0" );
> diff --git a/elf/tst-auditmod4b.c b/elf/tst-auditmod4b.c
> index 761d97c..222f3b7 100644
> --- a/elf/tst-auditmod4b.c
> +++ b/elf/tst-auditmod4b.c
> @@ -94,8 +94,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -177,7 +182,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ?#ifdef __AVX__
> ? if (check_avx () && strcmp (symname, "audit_test") == 0)
> diff --git a/elf/tst-auditmod5b.c b/elf/tst-auditmod5b.c
> index 7e1e941..78fe838 100644
> --- a/elf/tst-auditmod5b.c
> +++ b/elf/tst-auditmod5b.c
> @@ -95,8 +95,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -150,7 +155,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ? __m128i xmm;
>
> diff --git a/elf/tst-auditmod6b.c b/elf/tst-auditmod6b.c
> index a7a60b9..016b07b 100644
> --- a/elf/tst-auditmod6b.c
> +++ b/elf/tst-auditmod6b.c
> @@ -94,8 +94,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -179,7 +184,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ?#ifdef __AVX__
> ? if (check_avx () && strcmp (symname, "audit_test") == 0)
> diff --git a/elf/tst-auditmod6c.c b/elf/tst-auditmod6c.c
> index e0b5ac2..015be08 100644
> --- a/elf/tst-auditmod6c.c
> +++ b/elf/tst-auditmod6c.c
> @@ -94,8 +94,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -185,7 +190,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ?#ifdef __AVX__
> ? if (check_avx () && strcmp (symname, "audit_test") == 0)
> diff --git a/elf/tst-auditmod7b.c b/elf/tst-auditmod7b.c
> index a27d385..48b6626 100644
> --- a/elf/tst-auditmod7b.c
> +++ b/elf/tst-auditmod7b.c
> @@ -94,8 +94,13 @@ la_symbind64 (Elf64_Sym *sym, unsigned int ndx, uintptr_t *refcook,
> ? return sym->st_value;
> ?}
>
> -#define pltenter la_x86_64_gnu_pltenter
> -#define pltexit la_x86_64_gnu_pltexit
> +#ifdef __LP64__
> +# define pltenter la_x86_64_gnu_pltenter
> +# define pltexit la_x86_64_gnu_pltexit
> +#else
> +# define pltenter la_x32_gnu_pltenter
> +# define pltexit la_x32_gnu_pltexit
> +#endif
> ?#define La_regs La_x86_64_regs
> ?#define La_retval La_x86_64_retval
> ?#define int_retval lrv_rax
> @@ -177,7 +182,8 @@ pltexit (ElfW(Sym) *sym, unsigned int ndx, uintptr_t *refcook,
> ? ? ? ? const char *symname)
> ?{
> ? printf ("pltexit: symname=%s, st_value=%#lx, ndx=%u, retval=%tu\n",
> - ? ? ? ? symname, (long int) sym->st_value, ndx, outregs->int_retval);
> + ? ? ? ? symname, (long int) sym->st_value, ndx,
> + ? ? ? ? (ptrdiff_t) outregs->int_retval);
>
> ?#ifdef __AVX__
> ? if (check_avx () && strcmp (symname, "audit_test") == 0)
> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h
> index f4531b4..e5ed2be 100644
> --- a/sysdeps/generic/ldsodefs.h
> +++ b/sysdeps/generic/ldsodefs.h
> @@ -225,6 +225,10 @@ struct audit_ifaces
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uintptr_t *, struct La_x86_64_regs *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int *, const char *name,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? long int *framesizep);
> + ? ?Elf32_Addr (*x32_gnu_pltenter) (Elf32_Sym *, unsigned int, uintptr_t *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uintptr_t *, struct La_x86_64_regs *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int *, const char *name,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? long int *framesizep);
> ? ? Elf32_Addr (*ppc32_gnu_pltenter) (Elf32_Sym *, unsigned int, uintptr_t *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uintptr_t *, struct La_ppc32_regs *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int *, const char *name,
> @@ -269,6 +273,11 @@ struct audit_ifaces
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct La_x86_64_regs *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct La_x86_64_retval *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *);
> + ? ?unsigned int (*x32_gnu_pltexit) (Elf32_Sym *, unsigned int, uintptr_t *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uintptr_t *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const struct La_x86_64_regs *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct La_x86_64_retval *,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *);
> ? ? unsigned int (*ppc32_gnu_pltexit) (Elf32_Sym *, unsigned int, uintptr_t *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uintptr_t *,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const struct La_ppc32_regs *,
> diff --git a/sysdeps/x86_64/bits/link.h b/sysdeps/x86_64/bits/link.h
> index c79eda8..4dc255a 100644
> --- a/sysdeps/x86_64/bits/link.h
> +++ b/sysdeps/x86_64/bits/link.h
> @@ -20,7 +20,7 @@
> ?#endif
>
>
> -#if __ELF_NATIVE_CLASS == 32
> +#ifndef __x86_64__
> ?/* Registers for entry into PLT on IA-32. ?*/
> ?typedef struct La_i86_regs
> ?{
> @@ -124,6 +124,22 @@ extern unsigned int la_x86_64_gnu_pltexit (Elf64_Sym *__sym,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? La_x86_64_retval *__outregs,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *__symname);
>
> +extern Elf32_Addr la_x32_gnu_pltenter (Elf32_Sym *__sym,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int __ndx,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uintptr_t *__refcook,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?uintptr_t *__defcook,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?La_x86_64_regs *__regs,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned int *__flags,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *__symname,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?long int *__framesizep);
> +extern unsigned int la_x32_gnu_pltexit (Elf32_Sym *__sym,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? unsigned int __ndx,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uintptr_t *__refcook,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? uintptr_t *__defcook,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const La_x86_64_regs *__inregs,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? La_x86_64_retval *__outregs,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *__symname);
> +

It seems odd to me that you wouldn't just redefine a set of
La_x32_regs and La_x32_retval for the sake of consistency. I guess the
argument can be made that it *is* the same as the x86-64 version, but
that doesn't make it any easier to document or tell users that in this
case you use function name X and arguments named Y.

My preference would be to define an La_x32_regs and La_x32_retval for
use everywhere (even if they are just aliases to the x86-64 versions).

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]