unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] dl-runtime: reloc_{offset, index} now functions arch overide'able
@ 2020-05-28 19:43 Vineet Gupta via Libc-alpha
  2020-05-29 12:58 ` [PATCH] dl-runtime: reloc_{offset,index} " Adhemerval Zanella via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Vineet Gupta via Libc-alpha @ 2020-05-28 19:43 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet Gupta, linux-snps-arc

The existing macros are fragile and expect local variables with a
certain name. Fix this by defining them as functions with defaul
timplementation in a new header dl-runtime.h which arches can overrid
eif need be.

This came up during ARC port review.

This patch potentially only affects hppa/x86 ports,
build tested for both those configs and a few more.

Suggested-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 elf/dl-runtime.c                              | 24 +++++++-------
 elf/dl-runtime.h                              | 30 ++++++++++++++++++
 sysdeps/hppa/dl-runtime.c                     |  4 ---
 sysdeps/hppa/dl-runtime.h                     | 31 +++++++++++++++++++
 sysdeps/x86_64/{dl-runtime.c => dl-runtime.h} | 13 ++++++--
 5 files changed, 84 insertions(+), 18 deletions(-)
 create mode 100644 elf/dl-runtime.h
 create mode 100644 sysdeps/hppa/dl-runtime.h
 rename sysdeps/x86_64/{dl-runtime.c => dl-runtime.h} (60%)

diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c
index cf5f1d3e82e1..62f90579370b 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -27,6 +27,7 @@
 #include "dynamic-link.h"
 #include <tls.h>
 #include <dl-irel.h>
+#include <dl-runtime.h>
 
 
 #if (!ELF_MACHINE_NO_RELA && !defined ELF_MACHINE_PLT_REL) \
@@ -42,13 +43,6 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
-#ifndef reloc_offset
-# define reloc_offset reloc_arg
-# define reloc_index  reloc_arg / sizeof (PLTREL)
-#endif
-
-
-
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -68,8 +62,11 @@ _dl_fixup (
     = (const void *) D_PTR (l, l_info[DT_SYMTAB]);
   const char *strtab = (const void *) D_PTR (l, l_info[DT_STRTAB]);
 
+  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
   const PLTREL *const reloc
-    = (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
+    = (const void *) (D_PTR (l, l_info[DT_JMPREL])
+		      + reloc_offset (pltgot, reloc_arg));
   const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (reloc->r_info)];
   const ElfW(Sym) *refsym = sym;
   void *const rel_addr = (void *)(l->l_addr + reloc->r_offset);
@@ -182,7 +179,8 @@ _dl_profile_fixup (
 
   /* This is the address in the array where we store the result of previous
      relocations.  */
-  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
+  struct reloc_result *reloc_result
+    = &l->l_reloc_result[reloc_index (reloc_arg, sizeof (PLTREL))];
 
  /* CONCURRENCY NOTES:
 
@@ -219,8 +217,11 @@ _dl_profile_fixup (
 	= (const void *) D_PTR (l, l_info[DT_SYMTAB]);
       const char *strtab = (const char *) D_PTR (l, l_info[DT_STRTAB]);
 
+      const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
       const PLTREL *const reloc
-	= (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
+	= (const void *) (D_PTR (l, l_info[DT_JMPREL])
+			  + reloc_offset (pltgot, reloc_arg));
       const ElfW(Sym) *refsym = &symtab[ELFW(R_SYM) (reloc->r_info)];
       const ElfW(Sym) *defsym = refsym;
       lookup_t result;
@@ -489,7 +490,8 @@ _dl_call_pltexit (struct link_map *l, ElfW(Word) reloc_arg,
      relocations.  */
   // XXX Maybe the bound information must be stored on the stack since
   // XXX with bind_not a new value could have been stored in the meantime.
-  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
+  struct reloc_result *reloc_result =
+    &l->l_reloc_result[reloc_index (reloc_arg, sizeof (PLTREL))];
   ElfW(Sym) *defsym = ((ElfW(Sym) *) D_PTR (reloc_result->bound,
 					    l_info[DT_SYMTAB])
 		       + reloc_result->boundndx);
diff --git a/elf/dl-runtime.h b/elf/dl-runtime.h
new file mode 100644
index 000000000000..ed5db3ba51b7
--- /dev/null
+++ b/elf/dl-runtime.h
@@ -0,0 +1,30 @@
+/* Helpers for On-demand PLT fixup for shared objects, Generic version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn;
+}
+
+static inline uintptr_t
+reloc_index (uintptr_t pltn, size_t size)
+{
+  return pltn / size;
+}
diff --git a/sysdeps/hppa/dl-runtime.c b/sysdeps/hppa/dl-runtime.c
index 885a3f1837cb..2d061b150f06 100644
--- a/sysdeps/hppa/dl-runtime.c
+++ b/sysdeps/hppa/dl-runtime.c
@@ -17,10 +17,6 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
-/* Clear PA_GP_RELOC bit in relocation offset.  */
-#define reloc_offset (reloc_arg & ~PA_GP_RELOC)
-#define reloc_index  (reloc_arg & ~PA_GP_RELOC) / sizeof (PLTREL)
-
 #include <elf/dl-runtime.c>
 
 /* The caller has encountered a partially relocated function descriptor.
diff --git a/sysdeps/hppa/dl-runtime.h b/sysdeps/hppa/dl-runtime.h
new file mode 100644
index 000000000000..cfde0ec991e3
--- /dev/null
+++ b/sysdeps/hppa/dl-runtime.h
@@ -0,0 +1,31 @@
+/* Helpers for On-demand PLT fixup for shared objects, HPAA version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* Clear PA_GP_RELOC bit in relocation offset.  */
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn & ~PA_GP_RELOC;
+}
+
+static inline uintptr_t
+reloc_index (uintptr_t pltn, size_t size)
+{
+  return (pltn & ~PA_GP_RELOC )/ size;
+}
diff --git a/sysdeps/x86_64/dl-runtime.c b/sysdeps/x86_64/dl-runtime.h
similarity index 60%
rename from sysdeps/x86_64/dl-runtime.c
rename to sysdeps/x86_64/dl-runtime.h
index b625d1e88257..494ff47b7091 100644
--- a/sysdeps/x86_64/dl-runtime.c
+++ b/sysdeps/x86_64/dl-runtime.h
@@ -3,7 +3,14 @@
    also use the index.  Therefore it is wasteful to compute the offset
    in the trampoline just to reverse the operation immediately
    afterwards.  */
-#define reloc_offset reloc_arg * sizeof (PLTREL)
-#define reloc_index  reloc_arg
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn * sizeof (ElfW(Rela));
+}
 
-#include <elf/dl-runtime.c>
+static inline uintptr_t
+reloc_index (uintptr_t pltn, size_t size)
+{
+  return pltn;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] dl-runtime: reloc_{offset,index} now functions arch overide'able
  2020-05-28 19:43 [PATCH] dl-runtime: reloc_{offset, index} now functions arch overide'able Vineet Gupta via Libc-alpha
@ 2020-05-29 12:58 ` Adhemerval Zanella via Libc-alpha
  2020-05-29 17:39   ` Vineet Gupta via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-29 12:58 UTC (permalink / raw)
  To: Vineet Gupta, libc-alpha; +Cc: linux-snps-arc



On 28/05/2020 16:43, Vineet Gupta wrote:
> The existing macros are fragile and expect local variables with a
> certain name. Fix this by defining them as functions with defaul
> timplementation in a new header dl-runtime.h which arches can overrid
> eif need be.
> 
> This came up during ARC port review.
> 
> This patch potentially only affects hppa/x86 ports,
> build tested for both those configs and a few more.
> 
> Suggested-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

LGTM, thanks.

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

> ---
>  elf/dl-runtime.c                              | 24 +++++++-------
>  elf/dl-runtime.h                              | 30 ++++++++++++++++++
>  sysdeps/hppa/dl-runtime.c                     |  4 ---
>  sysdeps/hppa/dl-runtime.h                     | 31 +++++++++++++++++++
>  sysdeps/x86_64/{dl-runtime.c => dl-runtime.h} | 13 ++++++--
>  5 files changed, 84 insertions(+), 18 deletions(-)
>  create mode 100644 elf/dl-runtime.h
>  create mode 100644 sysdeps/hppa/dl-runtime.h
>  rename sysdeps/x86_64/{dl-runtime.c => dl-runtime.h} (60%)
> 
> diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c
> index cf5f1d3e82e1..62f90579370b 100644
> --- a/elf/dl-runtime.c
> +++ b/elf/dl-runtime.c
> @@ -27,6 +27,7 @@
>  #include "dynamic-link.h"
>  #include <tls.h>
>  #include <dl-irel.h>
> +#include <dl-runtime.h>
>  
>  
>  #if (!ELF_MACHINE_NO_RELA && !defined ELF_MACHINE_PLT_REL) \
> @@ -42,13 +43,6 @@
>  # define ARCH_FIXUP_ATTRIBUTE
>  #endif
>  
> -#ifndef reloc_offset
> -# define reloc_offset reloc_arg
> -# define reloc_index  reloc_arg / sizeof (PLTREL)
> -#endif
> -
> -
> -
>  /* This function is called through a special trampoline from the PLT the
>     first time each PLT entry is called.  We must perform the relocation
>     specified in the PLT of the given shared object, and return the resolved
> @@ -68,8 +62,11 @@ _dl_fixup (
>      = (const void *) D_PTR (l, l_info[DT_SYMTAB]);
>    const char *strtab = (const void *) D_PTR (l, l_info[DT_STRTAB]);
>  
> +  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
> +
>    const PLTREL *const reloc
> -    = (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
> +    = (const void *) (D_PTR (l, l_info[DT_JMPREL])
> +		      + reloc_offset (pltgot, reloc_arg));
>    const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (reloc->r_info)];
>    const ElfW(Sym) *refsym = sym;
>    void *const rel_addr = (void *)(l->l_addr + reloc->r_offset);

Ok (it should be dead code eliminated by compiler if reloc_offset does not
use the pltgot field, as for default implementation).

> @@ -182,7 +179,8 @@ _dl_profile_fixup (
>  
>    /* This is the address in the array where we store the result of previous
>       relocations.  */
> -  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
> +  struct reloc_result *reloc_result
> +    = &l->l_reloc_result[reloc_index (reloc_arg, sizeof (PLTREL))];
>  
>   /* CONCURRENCY NOTES:
>  
> @@ -219,8 +217,11 @@ _dl_profile_fixup (
>  	= (const void *) D_PTR (l, l_info[DT_SYMTAB]);
>        const char *strtab = (const char *) D_PTR (l, l_info[DT_STRTAB]);
>  
> +      const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
> +
>        const PLTREL *const reloc
> -	= (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
> +	= (const void *) (D_PTR (l, l_info[DT_JMPREL])
> +			  + reloc_offset (pltgot, reloc_arg));
>        const ElfW(Sym) *refsym = &symtab[ELFW(R_SYM) (reloc->r_info)];
>        const ElfW(Sym) *defsym = refsym;
>        lookup_t result;
> @@ -489,7 +490,8 @@ _dl_call_pltexit (struct link_map *l, ElfW(Word) reloc_arg,
>       relocations.  */
>    // XXX Maybe the bound information must be stored on the stack since
>    // XXX with bind_not a new value could have been stored in the meantime.
> -  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
> +  struct reloc_result *reloc_result =
> +    &l->l_reloc_result[reloc_index (reloc_arg, sizeof (PLTREL))];
>    ElfW(Sym) *defsym = ((ElfW(Sym) *) D_PTR (reloc_result->bound,
>  					    l_info[DT_SYMTAB])
>  		       + reloc_result->boundndx);

Ok.

> diff --git a/elf/dl-runtime.h b/elf/dl-runtime.h
> new file mode 100644
> index 000000000000..ed5db3ba51b7
> --- /dev/null
> +++ b/elf/dl-runtime.h
> @@ -0,0 +1,30 @@
> +/* Helpers for On-demand PLT fixup for shared objects, Generic version.

Maybe period here?

> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, write to the Free
> +   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> +   02111-1307 USA.  */
> +
> +static inline uintptr_t
> +reloc_offset (uintptr_t plt0, uintptr_t pltn)
> +{
> +  return pltn;
> +}
> +
> +static inline uintptr_t
> +reloc_index (uintptr_t pltn, size_t size)
> +{
> +  return pltn / size;
> +}

Ok.

> diff --git a/sysdeps/hppa/dl-runtime.c b/sysdeps/hppa/dl-runtime.c
> index 885a3f1837cb..2d061b150f06 100644
> --- a/sysdeps/hppa/dl-runtime.c
> +++ b/sysdeps/hppa/dl-runtime.c
> @@ -17,10 +17,6 @@
>     Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
>     02111-1307 USA.  */
>  
> -/* Clear PA_GP_RELOC bit in relocation offset.  */
> -#define reloc_offset (reloc_arg & ~PA_GP_RELOC)
> -#define reloc_index  (reloc_arg & ~PA_GP_RELOC) / sizeof (PLTREL)
> -
>  #include <elf/dl-runtime.c>
>  
>  /* The caller has encountered a partially relocated function descriptor.

Ok.

> diff --git a/sysdeps/hppa/dl-runtime.h b/sysdeps/hppa/dl-runtime.h
> new file mode 100644
> index 000000000000..cfde0ec991e3
> --- /dev/null
> +++ b/sysdeps/hppa/dl-runtime.h
> @@ -0,0 +1,31 @@
> +/* Helpers for On-demand PLT fixup for shared objects, HPAA version.

Maybe period instead of comma here?

> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, write to the Free
> +   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
> +   02111-1307 USA.  */
> +
> +/* Clear PA_GP_RELOC bit in relocation offset.  */
> +static inline uintptr_t
> +reloc_offset (uintptr_t plt0, uintptr_t pltn)
> +{
> +  return pltn & ~PA_GP_RELOC;
> +}
> +
> +static inline uintptr_t
> +reloc_index (uintptr_t pltn, size_t size)
> +{
> +  return (pltn & ~PA_GP_RELOC )/ size;
> +}

Ok.

> diff --git a/sysdeps/x86_64/dl-runtime.c b/sysdeps/x86_64/dl-runtime.h
> similarity index 60%
> rename from sysdeps/x86_64/dl-runtime.c
> rename to sysdeps/x86_64/dl-runtime.h
> index b625d1e88257..494ff47b7091 100644
> --- a/sysdeps/x86_64/dl-runtime.c
> +++ b/sysdeps/x86_64/dl-runtime.h
> @@ -3,7 +3,14 @@
>     also use the index.  Therefore it is wasteful to compute the offset
>     in the trampoline just to reverse the operation immediately
>     afterwards.  */
> -#define reloc_offset reloc_arg * sizeof (PLTREL)
> -#define reloc_index  reloc_arg
> +static inline uintptr_t
> +reloc_offset (uintptr_t plt0, uintptr_t pltn)
> +{
> +  return pltn * sizeof (ElfW(Rela));
> +}
>  
> -#include <elf/dl-runtime.c>
> +static inline uintptr_t
> +reloc_index (uintptr_t pltn, size_t size)
> +{
> +  return pltn;
> +}
> 

Ok.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dl-runtime: reloc_{offset,index} now functions arch overide'able
  2020-05-29 12:58 ` [PATCH] dl-runtime: reloc_{offset,index} " Adhemerval Zanella via Libc-alpha
@ 2020-05-29 17:39   ` Vineet Gupta via Libc-alpha
  2020-05-29 20:54     ` Adhemerval Zanella via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Vineet Gupta via Libc-alpha @ 2020-05-29 17:39 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha@sourceware.org
  Cc: linux-snps-arc@lists.infradead.org

On 5/29/20 5:58 AM, Adhemerval Zanella via Libc-alpha wrote:
> 
> 
> On 28/05/2020 16:43, Vineet Gupta wrote:
>> The existing macros are fragile and expect local variables with a
>> certain name. Fix this by defining them as functions with defaul
>> timplementation in a new header dl-runtime.h which arches can overrid
>> eif need be.
>>
>> This came up during ARC port review.
>>
>> This patch potentially only affects hppa/x86 ports,
>> build tested for both those configs and a few more.
>>
>> Suggested-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> LGTM, thanks.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

Sorry I didn't think through this before, but ARC port needs pltgot arg (runtime
address of plt0) in reloc_index not reloc_offset. I'll swap them and repost.

>>  /* This function is called through a special trampoline from the PLT the
>>     first time each PLT entry is called.  We must perform the relocation
>>     specified in the PLT of the given shared object, and return the resolved
>> @@ -68,8 +62,11 @@ _dl_fixup (
>>      = (const void *) D_PTR (l, l_info[DT_SYMTAB]);
>>    const char *strtab = (const void *) D_PTR (l, l_info[DT_STRTAB]);
>>  
>> +  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
>> +
>>    const PLTREL *const reloc
>> -    = (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
>> +    = (const void *) (D_PTR (l, l_info[DT_JMPREL])
>> +		      + reloc_offset (pltgot, reloc_arg));
>>    const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (reloc->r_info)];
>>    const ElfW(Sym) *refsym = sym;
>>    void *const rel_addr = (void *)(l->l_addr + reloc->r_offset);
> 
> Ok (it should be dead code eliminated by compiler if reloc_offset does not
> use the pltgot field, as for default implementation).

Yep.

> 
>> diff --git a/elf/dl-runtime.h b/elf/dl-runtime.h
>> new file mode 100644
>> index 000000000000..ed5db3ba51b7
>> --- /dev/null
>> +++ b/elf/dl-runtime.h
>> @@ -0,0 +1,30 @@
>> +/* Helpers for On-demand PLT fixup for shared objects, Generic version.
> 
> Maybe period here?

OK


>> diff --git a/sysdeps/hppa/dl-runtime.h b/sysdeps/hppa/dl-runtime.h
>> new file mode 100644
>> index 000000000000..cfde0ec991e3
>> --- /dev/null
>> +++ b/sysdeps/hppa/dl-runtime.h
>> @@ -0,0 +1,31 @@
>> +/* Helpers for On-demand PLT fixup for shared objects, HPAA version.
> 
> Maybe period instead of comma here?

OK.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] dl-runtime: reloc_{offset,index} now functions arch overide'able
  2020-05-29 17:39   ` Vineet Gupta via Libc-alpha
@ 2020-05-29 20:54     ` Adhemerval Zanella via Libc-alpha
  2020-05-29 21:21       ` [PATCH 1/2] dl-runtime: reloc_{offset, index} " Vineet Gupta via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-05-29 20:54 UTC (permalink / raw)
  To: Vineet Gupta, libc-alpha@sourceware.org
  Cc: linux-snps-arc@lists.infradead.org



On 29/05/2020 14:39, Vineet Gupta wrote:
> On 5/29/20 5:58 AM, Adhemerval Zanella via Libc-alpha wrote:
>>
>>
>> On 28/05/2020 16:43, Vineet Gupta wrote:
>>> The existing macros are fragile and expect local variables with a
>>> certain name. Fix this by defining them as functions with defaul
>>> timplementation in a new header dl-runtime.h which arches can overrid
>>> eif need be.
>>>
>>> This came up during ARC port review.
>>>
>>> This patch potentially only affects hppa/x86 ports,
>>> build tested for both those configs and a few more.
>>>
>>> Suggested-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>>
>> LGTM, thanks.
>>
>> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> 
> Sorry I didn't think through this before, but ARC port needs pltgot arg (runtime
> address of plt0) in reloc_index not reloc_offset. I'll swap them and repost.

Hum I though I had it covered on my suggestion, but it seems I mixed them
up. Ok, I will review the new version.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] dl-runtime: reloc_{offset, index} now functions arch overide'able
  2020-05-29 20:54     ` Adhemerval Zanella via Libc-alpha
@ 2020-05-29 21:21       ` Vineet Gupta via Libc-alpha
  2020-05-29 21:21         ` [PATCH 2/2] ARC/dl-runtime helper macros Vineet Gupta via Libc-alpha
  0 siblings, 1 reply; 6+ messages in thread
From: Vineet Gupta via Libc-alpha @ 2020-05-29 21:21 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet Gupta, linux-snps-arc

The existing macros are fragile and expect local variables with a
certain name. Fix this by defining them as functions with default
implementation in a new header dl-runtime.h which arches can override
if need be.

This came up during ARC port review, hence the need for argument pltgot
in reloc_index() which is not needed by existing ports.

This patch potentially only affects hppa/x86 ports,
build tested for both those configs and a few more.

Suggested-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
---
 elf/dl-runtime.c            | 28 +++++++++++++++++-----------
 elf/dl-runtime.h            | 30 ++++++++++++++++++++++++++++++
 sysdeps/hppa/dl-runtime.c   |  4 ----
 sysdeps/hppa/dl-runtime.h   | 31 +++++++++++++++++++++++++++++++
 sysdeps/x86_64/dl-runtime.c |  9 ---------
 sysdeps/x86_64/dl-runtime.h | 35 +++++++++++++++++++++++++++++++++++
 6 files changed, 113 insertions(+), 24 deletions(-)
 create mode 100644 elf/dl-runtime.h
 create mode 100644 sysdeps/hppa/dl-runtime.h
 delete mode 100644 sysdeps/x86_64/dl-runtime.c
 create mode 100644 sysdeps/x86_64/dl-runtime.h

diff --git a/elf/dl-runtime.c b/elf/dl-runtime.c
index cf5f1d3e82e1..85a229f6f019 100644
--- a/elf/dl-runtime.c
+++ b/elf/dl-runtime.c
@@ -27,6 +27,7 @@
 #include "dynamic-link.h"
 #include <tls.h>
 #include <dl-irel.h>
+#include <dl-runtime.h>
 
 
 #if (!ELF_MACHINE_NO_RELA && !defined ELF_MACHINE_PLT_REL) \
@@ -42,13 +43,6 @@
 # define ARCH_FIXUP_ATTRIBUTE
 #endif
 
-#ifndef reloc_offset
-# define reloc_offset reloc_arg
-# define reloc_index  reloc_arg / sizeof (PLTREL)
-#endif
-
-
-
 /* This function is called through a special trampoline from the PLT the
    first time each PLT entry is called.  We must perform the relocation
    specified in the PLT of the given shared object, and return the resolved
@@ -68,8 +62,11 @@ _dl_fixup (
     = (const void *) D_PTR (l, l_info[DT_SYMTAB]);
   const char *strtab = (const void *) D_PTR (l, l_info[DT_STRTAB]);
 
+  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
   const PLTREL *const reloc
-    = (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
+    = (const void *) (D_PTR (l, l_info[DT_JMPREL])
+		      + reloc_offset (pltgot, reloc_arg));
   const ElfW(Sym) *sym = &symtab[ELFW(R_SYM) (reloc->r_info)];
   const ElfW(Sym) *refsym = sym;
   void *const rel_addr = (void *)(l->l_addr + reloc->r_offset);
@@ -180,9 +177,12 @@ _dl_profile_fixup (
 			l, reloc_arg);
     }
 
+  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
   /* This is the address in the array where we store the result of previous
      relocations.  */
-  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
+  struct reloc_result *reloc_result
+    = &l->l_reloc_result[reloc_index (pltgot, reloc_arg, sizeof (PLTREL))];
 
  /* CONCURRENCY NOTES:
 
@@ -219,8 +219,11 @@ _dl_profile_fixup (
 	= (const void *) D_PTR (l, l_info[DT_SYMTAB]);
       const char *strtab = (const char *) D_PTR (l, l_info[DT_STRTAB]);
 
+      const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
       const PLTREL *const reloc
-	= (const void *) (D_PTR (l, l_info[DT_JMPREL]) + reloc_offset);
+	= (const void *) (D_PTR (l, l_info[DT_JMPREL])
+			  + reloc_offset (pltgot, reloc_arg));
       const ElfW(Sym) *refsym = &symtab[ELFW(R_SYM) (reloc->r_info)];
       const ElfW(Sym) *defsym = refsym;
       lookup_t result;
@@ -485,11 +488,14 @@ _dl_call_pltexit (struct link_map *l, ElfW(Word) reloc_arg,
 		  const void *inregs, void *outregs)
 {
 #ifdef SHARED
+  const uintptr_t pltgot = (uintptr_t) D_PTR (l, l_info[DT_PLTGOT]);
+
   /* This is the address in the array where we store the result of previous
      relocations.  */
   // XXX Maybe the bound information must be stored on the stack since
   // XXX with bind_not a new value could have been stored in the meantime.
-  struct reloc_result *reloc_result = &l->l_reloc_result[reloc_index];
+  struct reloc_result *reloc_result =
+    &l->l_reloc_result[reloc_index (pltgot, reloc_arg, sizeof (PLTREL))];
   ElfW(Sym) *defsym = ((ElfW(Sym) *) D_PTR (reloc_result->bound,
 					    l_info[DT_SYMTAB])
 		       + reloc_result->boundndx);
diff --git a/elf/dl-runtime.h b/elf/dl-runtime.h
new file mode 100644
index 000000000000..dd87d799465e
--- /dev/null
+++ b/elf/dl-runtime.h
@@ -0,0 +1,30 @@
+/* Helpers for On-demand PLT fixup for shared objects. Generic version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn;
+}
+
+static inline uintptr_t
+reloc_index (uintptr_t plt0, uintptr_t pltn, size_t size)
+{
+  return pltn / size;
+}
diff --git a/sysdeps/hppa/dl-runtime.c b/sysdeps/hppa/dl-runtime.c
index 885a3f1837cb..2d061b150f06 100644
--- a/sysdeps/hppa/dl-runtime.c
+++ b/sysdeps/hppa/dl-runtime.c
@@ -17,10 +17,6 @@
    Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
    02111-1307 USA.  */
 
-/* Clear PA_GP_RELOC bit in relocation offset.  */
-#define reloc_offset (reloc_arg & ~PA_GP_RELOC)
-#define reloc_index  (reloc_arg & ~PA_GP_RELOC) / sizeof (PLTREL)
-
 #include <elf/dl-runtime.c>
 
 /* The caller has encountered a partially relocated function descriptor.
diff --git a/sysdeps/hppa/dl-runtime.h b/sysdeps/hppa/dl-runtime.h
new file mode 100644
index 000000000000..be3bc929c74c
--- /dev/null
+++ b/sysdeps/hppa/dl-runtime.h
@@ -0,0 +1,31 @@
+/* Helpers for On-demand PLT fixup for shared objects. HPAA version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* Clear PA_GP_RELOC bit in relocation offset.  */
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn & ~PA_GP_RELOC;
+}
+
+static inline uintptr_t
+reloc_index (uintptr_t plt0, uintptr_t pltn, size_t size)
+{
+  return (pltn & ~PA_GP_RELOC )/ size;
+}
diff --git a/sysdeps/x86_64/dl-runtime.c b/sysdeps/x86_64/dl-runtime.c
deleted file mode 100644
index b625d1e88257..000000000000
--- a/sysdeps/x86_64/dl-runtime.c
+++ /dev/null
@@ -1,9 +0,0 @@
-/* The ABI calls for the PLT stubs to pass the index of the relocation
-   and not its offset.  In _dl_profile_fixup and _dl_call_pltexit we
-   also use the index.  Therefore it is wasteful to compute the offset
-   in the trampoline just to reverse the operation immediately
-   afterwards.  */
-#define reloc_offset reloc_arg * sizeof (PLTREL)
-#define reloc_index  reloc_arg
-
-#include <elf/dl-runtime.c>
diff --git a/sysdeps/x86_64/dl-runtime.h b/sysdeps/x86_64/dl-runtime.h
new file mode 100644
index 000000000000..b82fdce8367d
--- /dev/null
+++ b/sysdeps/x86_64/dl-runtime.h
@@ -0,0 +1,35 @@
+/* Helpers for On-demand PLT fixup for shared objects. x86_64 version.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+/* The ABI calls for the PLT stubs to pass the index of the relocation
+   and not its offset.  In _dl_profile_fixup and _dl_call_pltexit we
+   also use the index.  Therefore it is wasteful to compute the offset
+   in the trampoline just to reverse the operation immediately
+   afterwards.  */
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  return pltn * sizeof (ElfW(Rela));
+}
+
+static inline uintptr_t
+reloc_index (uintptr_t plt0, uintptr_t pltn, size_t size)
+{
+  return pltn;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ARC/dl-runtime helper macros
  2020-05-29 21:21       ` [PATCH 1/2] dl-runtime: reloc_{offset, index} " Vineet Gupta via Libc-alpha
@ 2020-05-29 21:21         ` Vineet Gupta via Libc-alpha
  0 siblings, 0 replies; 6+ messages in thread
From: Vineet Gupta via Libc-alpha @ 2020-05-29 21:21 UTC (permalink / raw)
  To: libc-alpha; +Cc: Vineet Gupta, linux-snps-arc

This is purely for review purposes to attest the interface defined
in prior patch
---
 sysdeps/arc/dl-runtime.h | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 sysdeps/arc/dl-runtime.h

diff --git a/sysdeps/arc/dl-runtime.h b/sysdeps/arc/dl-runtime.h
new file mode 100644
index 000000000000..529d49f5d0a1
--- /dev/null
+++ b/sysdeps/arc/dl-runtime.h
@@ -0,0 +1,42 @@
+/* Helpers for On-demand PLT fixup for shared objects. ARC version.
+   Copyright (C) 2017-2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library.  If not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* PLT jump into resolver passes PC of PLTn, while _dl_fixup expects the
+   address of corresponding .rela.plt entry.
+
+    - @plt0: runtime pc of first plt entry (DT_PLTGOT)
+    - @pltn: runtime pc of plt entry being resolved
+    - @size: size of .plt.rela entry (unused).  */
+static inline uintptr_t
+reloc_index (uintptr_t plt0, uintptr_t pltn, size_t size)
+{
+  unsigned long int idx = (unsigned long)pltn - (unsigned long)plt0;
+
+  /* PLT trampoline is 16 bytes. */
+  idx /= 16;
+
+  /* Exclude PLT0 and PLT1.  */
+  return idx - 2;
+}
+
+static inline uintptr_t
+reloc_offset (uintptr_t plt0, uintptr_t pltn)
+{
+  size_t sz = sizeof (ElfW(Rela));
+  return reloc_index(plt0, pltn, sz) * sz;
+}
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-29 21:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-28 19:43 [PATCH] dl-runtime: reloc_{offset, index} now functions arch overide'able Vineet Gupta via Libc-alpha
2020-05-29 12:58 ` [PATCH] dl-runtime: reloc_{offset,index} " Adhemerval Zanella via Libc-alpha
2020-05-29 17:39   ` Vineet Gupta via Libc-alpha
2020-05-29 20:54     ` Adhemerval Zanella via Libc-alpha
2020-05-29 21:21       ` [PATCH 1/2] dl-runtime: reloc_{offset, index} " Vineet Gupta via Libc-alpha
2020-05-29 21:21         ` [PATCH 2/2] ARC/dl-runtime helper macros Vineet Gupta via Libc-alpha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).