diff mbox series

[1/2] riscv: Use elf_machine_rela_relative to handle R_RISCV_RELATIVE

Message ID 20220623154705.14416-2-kito.cheng@sifive.com
State Committed
Commit 58fc66a91ca511cc12901cb599914f31948e02d5
Headers
Series riscv: Prevent potential unaligned memory access during dynamic relocation |

Checks

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

Commit Message

Kito Cheng June 23, 2022, 3:47 p.m. UTC 
  Minor clean-up, we need to change this part in following patch, clean this up
to prevent we duplicated the change twice.
---
 sysdeps/riscv/dl-machine.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Adhemerval Zanella Netto June 23, 2022, 4:39 p.m. UTC | #1 
> On 23 Jun 2022, at 12:47, Kito Cheng <kito.cheng@sifive.com> wrote:
> 
> Minor clean-up, we need to change this part in following patch, clean this up
> to prevent we duplicated the change twice.

LGTM, thanks.

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

> ---
> sysdeps/riscv/dl-machine.h | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> index a60a452952..4bb858adaa 100644
> --- a/sysdeps/riscv/dl-machine.h
> +++ b/sysdeps/riscv/dl-machine.h
> @@ -152,6 +152,14 @@ elf_machine_fixup_plt (struct link_map *map, lookup_t t,
> 
> #ifdef RESOLVE_MAP
> 
> +static inline void
> +__attribute__ ((always_inline))
> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> +			  void *const reloc_addr)
> +{
> +  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
> +}
> +
> /* Perform a relocation described by R_INFO at the location pointed to
>    by RELOC_ADDR.  SYM is the relocation symbol specified by R_INFO and
>    MAP is the object containing the reloc.  */
> @@ -182,7 +190,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>   switch (r_type)
>     {
>     case R_RISCV_RELATIVE:
> -      *addr_field = map->l_addr + reloc->r_addend;
> +      elf_machine_rela_relative (map->l_addr, reloc, addr_field);
>       break;
>     case R_RISCV_JUMP_SLOT:
>     case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
> @@ -258,14 +266,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>     }
> }
> 
> -static inline void
> -__attribute__ ((always_inline))
> -elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> -			  void *const reloc_addr)
> -{
> -  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
> -}
> -
> static inline void
> __attribute__ ((always_inline))
> elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> -- 
> 2.34.0
>
Palmer Dabbelt June 23, 2022, 9:50 p.m. UTC | #2 
On Thu, 23 Jun 2022 09:39:57 PDT (-0700), adhemerval.zanella@linaro.org wrote:
>
>
>> On 23 Jun 2022, at 12:47, Kito Cheng <kito.cheng@sifive.com> wrote:
>> 
>> Minor clean-up, we need to change this part in following patch, clean this up
>> to prevent we duplicated the change twice.
>
> LGTM, thanks.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
Acked-by: Palmer Dabbelt <palmer@rivosinc.com>

I forget if Kito has commit access.  I'd usually just commit it, but 
there's some comments on the second one.  They look independent to me, 
but I'll hold off just to be sure.

Thanks!

>> ---
>> sysdeps/riscv/dl-machine.h | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
>> index a60a452952..4bb858adaa 100644
>> --- a/sysdeps/riscv/dl-machine.h
>> +++ b/sysdeps/riscv/dl-machine.h
>> @@ -152,6 +152,14 @@ elf_machine_fixup_plt (struct link_map *map, lookup_t t,
>> 
>> #ifdef RESOLVE_MAP
>> 
>> +static inline void
>> +__attribute__ ((always_inline))
>> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
>> +			  void *const reloc_addr)
>> +{
>> +  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
>> +}
>> +
>> /* Perform a relocation described by R_INFO at the location pointed to
>>    by RELOC_ADDR.  SYM is the relocation symbol specified by R_INFO and
>>    MAP is the object containing the reloc.  */
>> @@ -182,7 +190,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>>   switch (r_type)
>>     {
>>     case R_RISCV_RELATIVE:
>> -      *addr_field = map->l_addr + reloc->r_addend;
>> +      elf_machine_rela_relative (map->l_addr, reloc, addr_field);
>>       break;
>>     case R_RISCV_JUMP_SLOT:
>>     case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
>> @@ -258,14 +266,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>>     }
>> }
>> 
>> -static inline void
>> -__attribute__ ((always_inline))
>> -elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
>> -			  void *const reloc_addr)
>> -{
>> -  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
>> -}
>> -
>> static inline void
>> __attribute__ ((always_inline))
>> elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>> -- 
>> 2.34.0
>>
Kito Cheng June 24, 2022, 2:41 a.m. UTC | #3 
Hi Palmer:

I didn't have commit access for glibc, so plz commit that once v2 is
everything OK, or you commit this first is also works for me :)

Thanks!

On Fri, Jun 24, 2022 at 5:50 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Thu, 23 Jun 2022 09:39:57 PDT (-0700), adhemerval.zanella@linaro.org wrote:
> >
> >
> >> On 23 Jun 2022, at 12:47, Kito Cheng <kito.cheng@sifive.com> wrote:
> >>
> >> Minor clean-up, we need to change this part in following patch, clean this up
> >> to prevent we duplicated the change twice.
> >
> > LGTM, thanks.
> >
> > Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>
> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>
> I forget if Kito has commit access.  I'd usually just commit it, but
> there's some comments on the second one.  They look independent to me,
> but I'll hold off just to be sure.
>
> Thanks!
>
> >> ---
> >> sysdeps/riscv/dl-machine.h | 18 +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
> >> index a60a452952..4bb858adaa 100644
> >> --- a/sysdeps/riscv/dl-machine.h
> >> +++ b/sysdeps/riscv/dl-machine.h
> >> @@ -152,6 +152,14 @@ elf_machine_fixup_plt (struct link_map *map, lookup_t t,
> >>
> >> #ifdef RESOLVE_MAP
> >>
> >> +static inline void
> >> +__attribute__ ((always_inline))
> >> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> >> +                      void *const reloc_addr)
> >> +{
> >> +  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
> >> +}
> >> +
> >> /* Perform a relocation described by R_INFO at the location pointed to
> >>    by RELOC_ADDR.  SYM is the relocation symbol specified by R_INFO and
> >>    MAP is the object containing the reloc.  */
> >> @@ -182,7 +190,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
> >>   switch (r_type)
> >>     {
> >>     case R_RISCV_RELATIVE:
> >> -      *addr_field = map->l_addr + reloc->r_addend;
> >> +      elf_machine_rela_relative (map->l_addr, reloc, addr_field);
> >>       break;
> >>     case R_RISCV_JUMP_SLOT:
> >>     case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
> >> @@ -258,14 +266,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
> >>     }
> >> }
> >>
> >> -static inline void
> >> -__attribute__ ((always_inline))
> >> -elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
> >> -                      void *const reloc_addr)
> >> -{
> >> -  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
> >> -}
> >> -
> >> static inline void
> >> __attribute__ ((always_inline))
> >> elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
> >> --
> >> 2.34.0
> >>
Palmer Dabbelt June 24, 2022, 4:11 a.m. UTC | #4 
On Thu, 23 Jun 2022 19:41:42 PDT (-0700), kito.cheng@sifive.com wrote:
> Hi Palmer:
>
> I didn't have commit access for glibc, so plz commit that once v2 is
> everything OK, or you commit this first is also works for me :)

I just committed the 1/2.  Thanks!

>
> Thanks!
>
> On Fri, Jun 24, 2022 at 5:50 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>>
>> On Thu, 23 Jun 2022 09:39:57 PDT (-0700), adhemerval.zanella@linaro.org wrote:
>> >
>> >
>> >> On 23 Jun 2022, at 12:47, Kito Cheng <kito.cheng@sifive.com> wrote:
>> >>
>> >> Minor clean-up, we need to change this part in following patch, clean this up
>> >> to prevent we duplicated the change twice.
>> >
>> > LGTM, thanks.
>> >
>> > Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>
>>
>> Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>
>> Acked-by: Palmer Dabbelt <palmer@rivosinc.com>
>>
>> I forget if Kito has commit access.  I'd usually just commit it, but
>> there's some comments on the second one.  They look independent to me,
>> but I'll hold off just to be sure.
>>
>> Thanks!
>>
>> >> ---
>> >> sysdeps/riscv/dl-machine.h | 18 +++++++++---------
>> >> 1 file changed, 9 insertions(+), 9 deletions(-)
>> >>
>> >> diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
>> >> index a60a452952..4bb858adaa 100644
>> >> --- a/sysdeps/riscv/dl-machine.h
>> >> +++ b/sysdeps/riscv/dl-machine.h
>> >> @@ -152,6 +152,14 @@ elf_machine_fixup_plt (struct link_map *map, lookup_t t,
>> >>
>> >> #ifdef RESOLVE_MAP
>> >>
>> >> +static inline void
>> >> +__attribute__ ((always_inline))
>> >> +elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
>> >> +                      void *const reloc_addr)
>> >> +{
>> >> +  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
>> >> +}
>> >> +
>> >> /* Perform a relocation described by R_INFO at the location pointed to
>> >>    by RELOC_ADDR.  SYM is the relocation symbol specified by R_INFO and
>> >>    MAP is the object containing the reloc.  */
>> >> @@ -182,7 +190,7 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>> >>   switch (r_type)
>> >>     {
>> >>     case R_RISCV_RELATIVE:
>> >> -      *addr_field = map->l_addr + reloc->r_addend;
>> >> +      elf_machine_rela_relative (map->l_addr, reloc, addr_field);
>> >>       break;
>> >>     case R_RISCV_JUMP_SLOT:
>> >>     case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
>> >> @@ -258,14 +266,6 @@ elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
>> >>     }
>> >> }
>> >>
>> >> -static inline void
>> >> -__attribute__ ((always_inline))
>> >> -elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
>> >> -                      void *const reloc_addr)
>> >> -{
>> >> -  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
>> >> -}
>> >> -
>> >> static inline void
>> >> __attribute__ ((always_inline))
>> >> elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],
>> >> --
>> >> 2.34.0
>> >>
diff mbox series

Patch

diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h
index a60a452952..4bb858adaa 100644
--- a/sysdeps/riscv/dl-machine.h
+++ b/sysdeps/riscv/dl-machine.h
@@ -152,6 +152,14 @@  elf_machine_fixup_plt (struct link_map *map, lookup_t t,
 
 #ifdef RESOLVE_MAP
 
+static inline void
+__attribute__ ((always_inline))
+elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
+			  void *const reloc_addr)
+{
+  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
+}
+
 /* Perform a relocation described by R_INFO at the location pointed to
    by RELOC_ADDR.  SYM is the relocation symbol specified by R_INFO and
    MAP is the object containing the reloc.  */
@@ -182,7 +190,7 @@  elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
   switch (r_type)
     {
     case R_RISCV_RELATIVE:
-      *addr_field = map->l_addr + reloc->r_addend;
+      elf_machine_rela_relative (map->l_addr, reloc, addr_field);
       break;
     case R_RISCV_JUMP_SLOT:
     case __WORDSIZE == 64 ? R_RISCV_64 : R_RISCV_32:
@@ -258,14 +266,6 @@  elf_machine_rela (struct link_map *map, struct r_scope_elem *scope[],
     }
 }
 
-static inline void
-__attribute__ ((always_inline))
-elf_machine_rela_relative (ElfW(Addr) l_addr, const ElfW(Rela) *reloc,
-			  void *const reloc_addr)
-{
-  *(ElfW(Addr) *) reloc_addr = l_addr + reloc->r_addend;
-}
-
 static inline void
 __attribute__ ((always_inline))
 elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[],