* [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
@ 2022-05-21 2:23 H.J. Lu
2022-05-22 22:10 ` Fangrui Song
2022-05-25 16:52 ` Florian Weimer
0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu @ 2022-05-21 2:23 UTC (permalink / raw)
To: libc-alpha
According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
and R_X86_64_JUMP_SLOT.
---
sysdeps/x86_64/dl-machine.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index c70af7ab1e..7f607f6dff 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -325,11 +325,13 @@ and creates an unsatisfiable circular dependency.\n",
# endif
/* Set to symbol size plus addend. */
value = sym->st_size;
+ *reloc_addr = value + reloc->r_addend;
+ break;
# endif
- /* Fall through. */
+
case R_X86_64_GLOB_DAT:
case R_X86_64_JUMP_SLOT:
- *reloc_addr = value + reloc->r_addend;
+ *reloc_addr = value;
break;
case R_X86_64_DTPMOD64:
--
2.36.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-21 2:23 [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT H.J. Lu
@ 2022-05-22 22:10 ` Fangrui Song
2022-05-25 16:52 ` Florian Weimer
1 sibling, 0 replies; 10+ messages in thread
From: Fangrui Song @ 2022-05-22 22:10 UTC (permalink / raw)
To: H.J. Lu; +Cc: libc-alpha
On 2022-05-20, H.J. Lu via Libc-alpha wrote:
>According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
>and R_X86_64_JUMP_SLOT.
Looks good. In musl, All GLOB_DAT and JUMP_SLOT ignore the addend.
Ignoring r_addend may or may not be profitable. Since GLOB and JUMP_SLOT
are handled the same way as the symbolic relocation R_X86_64_64, this
may not save code.
>---
> sysdeps/x86_64/dl-machine.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
>index c70af7ab1e..7f607f6dff 100644
>--- a/sysdeps/x86_64/dl-machine.h
>+++ b/sysdeps/x86_64/dl-machine.h
>@@ -325,11 +325,13 @@ and creates an unsatisfiable circular dependency.\n",
> # endif
> /* Set to symbol size plus addend. */
> value = sym->st_size;
>+ *reloc_addr = value + reloc->r_addend;
>+ break;
> # endif
>- /* Fall through. */
>+
> case R_X86_64_GLOB_DAT:
> case R_X86_64_JUMP_SLOT:
>- *reloc_addr = value + reloc->r_addend;
>+ *reloc_addr = value;
> break;
>
> case R_X86_64_DTPMOD64:
>--
>2.36.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-21 2:23 [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT H.J. Lu
2022-05-22 22:10 ` Fangrui Song
@ 2022-05-25 16:52 ` Florian Weimer
2022-05-25 17:49 ` H.J. Lu
1 sibling, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2022-05-25 16:52 UTC (permalink / raw)
To: H.J. Lu via Libc-alpha
* H. J. Lu via Libc-alpha:
> According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
> and R_X86_64_JUMP_SLOT.
> ---
> sysdeps/x86_64/dl-machine.h | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> index c70af7ab1e..7f607f6dff 100644
> --- a/sysdeps/x86_64/dl-machine.h
> +++ b/sysdeps/x86_64/dl-machine.h
> @@ -325,11 +325,13 @@ and creates an unsatisfiable circular dependency.\n",
> # endif
> /* Set to symbol size plus addend. */
> value = sym->st_size;
> + *reloc_addr = value + reloc->r_addend;
> + break;
> # endif
> - /* Fall through. */
> +
> case R_X86_64_GLOB_DAT:
> case R_X86_64_JUMP_SLOT:
> - *reloc_addr = value + reloc->r_addend;
> + *reloc_addr = value;
> break;
>
> case R_X86_64_DTPMOD64:
Context:
{
# ifndef RTLD_BOOTSTRAP
# ifdef __ILP32__
case R_X86_64_SIZE64:
/* Set to symbol size plus addend. */
*(Elf64_Addr *) (uintptr_t) reloc_addr
= (Elf64_Addr) sym->st_size + reloc->r_addend;
break;
case R_X86_64_SIZE32:
# else
case R_X86_64_SIZE64:
# endif
/* Set to symbol size plus addend. */
value = sym->st_size;
# endif
/* Fall through. */
case R_X86_64_GLOB_DAT:
case R_X86_64_JUMP_SLOT:
*reloc_addr = value + reloc->r_addend;
break;
The new version is:
{
# ifndef RTLD_BOOTSTRAP
# ifdef __ILP32__
case R_X86_64_SIZE64:
/* Set to symbol size plus addend. */
*(Elf64_Addr *) (uintptr_t) reloc_addr
= (Elf64_Addr) sym->st_size + reloc->r_addend;
break;
case R_X86_64_SIZE32:
# else
case R_X86_64_SIZE64:
# endif
/* Set to symbol size plus addend. */
value = sym->st_size;
*reloc_addr = value + reloc->r_addend;
break;
# endif
case R_X86_64_GLOB_DAT:
case R_X86_64_JUMP_SLOT:
*reloc_addr = value;
break;
So that “break” is actually in the right place.
Are there binaries out there with a non-zero addend? I'm a bit worried
that this change introduces hard-to-diagnose regressions.
Thanks,
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 16:52 ` Florian Weimer
@ 2022-05-25 17:49 ` H.J. Lu
2022-05-25 18:19 ` Florian Weimer
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2022-05-25 17:49 UTC (permalink / raw)
To: Florian Weimer; +Cc: H.J. Lu via Libc-alpha
On Wed, May 25, 2022 at 9:52 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu via Libc-alpha:
>
> > According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
> > and R_X86_64_JUMP_SLOT.
> > ---
> > sysdeps/x86_64/dl-machine.h | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
> > index c70af7ab1e..7f607f6dff 100644
> > --- a/sysdeps/x86_64/dl-machine.h
> > +++ b/sysdeps/x86_64/dl-machine.h
> > @@ -325,11 +325,13 @@ and creates an unsatisfiable circular dependency.\n",
> > # endif
> > /* Set to symbol size plus addend. */
> > value = sym->st_size;
> > + *reloc_addr = value + reloc->r_addend;
> > + break;
> > # endif
> > - /* Fall through. */
> > +
> > case R_X86_64_GLOB_DAT:
> > case R_X86_64_JUMP_SLOT:
> > - *reloc_addr = value + reloc->r_addend;
> > + *reloc_addr = value;
> > break;
> >
> > case R_X86_64_DTPMOD64:
>
> Context:
>
> {
> # ifndef RTLD_BOOTSTRAP
> # ifdef __ILP32__
> case R_X86_64_SIZE64:
> /* Set to symbol size plus addend. */
> *(Elf64_Addr *) (uintptr_t) reloc_addr
> = (Elf64_Addr) sym->st_size + reloc->r_addend;
> break;
>
> case R_X86_64_SIZE32:
> # else
> case R_X86_64_SIZE64:
> # endif
> /* Set to symbol size plus addend. */
> value = sym->st_size;
> # endif
> /* Fall through. */
> case R_X86_64_GLOB_DAT:
> case R_X86_64_JUMP_SLOT:
> *reloc_addr = value + reloc->r_addend;
> break;
>
>
> The new version is:
>
> {
> # ifndef RTLD_BOOTSTRAP
> # ifdef __ILP32__
> case R_X86_64_SIZE64:
> /* Set to symbol size plus addend. */
> *(Elf64_Addr *) (uintptr_t) reloc_addr
> = (Elf64_Addr) sym->st_size + reloc->r_addend;
> break;
>
> case R_X86_64_SIZE32:
> # else
> case R_X86_64_SIZE64:
> # endif
> /* Set to symbol size plus addend. */
> value = sym->st_size;
> *reloc_addr = value + reloc->r_addend;
> break;
> # endif
> case R_X86_64_GLOB_DAT:
> case R_X86_64_JUMP_SLOT:
> *reloc_addr = value;
> break;
>
> So that “break” is actually in the right place.
>
> Are there binaries out there with a non-zero addend? I'm a bit worried
Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
R_X86_64_JUMP_SLOT.
> that this change introduces hard-to-diagnose regressions.
>
> Thanks,
> Florian
>
--
H.J.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 17:49 ` H.J. Lu
@ 2022-05-25 18:19 ` Florian Weimer
2022-05-25 18:28 ` Adhemerval Zanella
0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2022-05-25 18:19 UTC (permalink / raw)
To: H.J. Lu; +Cc: H.J. Lu via Libc-alpha
* H. J. Lu:
> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
> R_X86_64_JUMP_SLOT.
Okay, so it's very unlikely that we see non-zero addends.
Maybe you can add this to the commit message?
Given that, I think the change should be safe.
Thanks,
Florian
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 18:19 ` Florian Weimer
@ 2022-05-25 18:28 ` Adhemerval Zanella
2022-05-25 19:29 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2022-05-25 18:28 UTC (permalink / raw)
To: libc-alpha
On 25/05/2022 15:19, Florian Weimer via Libc-alpha wrote:
> * H. J. Lu:
>
>> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
>> R_X86_64_JUMP_SLOT.
>
> Okay, so it's very unlikely that we see non-zero addends.
> Maybe you can add this to the commit message?
>
> Given that, I think the change should be safe.
>
> Thanks,
> Florian
>
Maybe assert that r_addend is always zero?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 18:28 ` Adhemerval Zanella
@ 2022-05-25 19:29 ` H.J. Lu
2022-05-25 20:24 ` Fangrui Song
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2022-05-25 19:29 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: GNU C Library
On Wed, May 25, 2022 at 11:28 AM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
>
> On 25/05/2022 15:19, Florian Weimer via Libc-alpha wrote:
> > * H. J. Lu:
> >
> >> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
> >> R_X86_64_JUMP_SLOT.
> >
> > Okay, so it's very unlikely that we see non-zero addends.
> > Maybe you can add this to the commit message?
> >
> > Given that, I think the change should be safe.
> >
> > Thanks,
> > Florian
> >
>
> Maybe assert that r_addend is always zero?
I don't think it is necessary since r_addend has never been non-zero.
--
H.J.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 19:29 ` H.J. Lu
@ 2022-05-25 20:24 ` Fangrui Song
2022-05-26 20:59 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Fangrui Song @ 2022-05-25 20:24 UTC (permalink / raw)
To: H.J. Lu; +Cc: Adhemerval Zanella, GNU C Library
On 2022-05-25, H.J. Lu via Libc-alpha wrote:
>On Wed, May 25, 2022 at 11:28 AM Adhemerval Zanella via Libc-alpha
><libc-alpha@sourceware.org> wrote:
>>
>>
>>
>> On 25/05/2022 15:19, Florian Weimer via Libc-alpha wrote:
>> > * H. J. Lu:
>> >
>> >> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
>> >> R_X86_64_JUMP_SLOT.
>> >
>> > Okay, so it's very unlikely that we see non-zero addends.
>> > Maybe you can add this to the commit message?
>> >
>> > Given that, I think the change should be safe.
>> >
>> > Thanks,
>> > Florian
>> >
>>
>> Maybe assert that r_addend is always zero?
>
>I don't think it is necessary since r_addend has never been non-zero.
I agree that an assert is unnecessary. Linkers just guarantee zero
addends.
It seems that by omitting reloc->r_addend, the GCC generated assembler
is slightly shorter. I've tried placing GLOB_DAT/JUMP_SLOT near 64
and don't get simplified assembly.
Reviewed-by: Fangrui Song <maskray@google.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-25 20:24 ` Fangrui Song
@ 2022-05-26 20:59 ` H.J. Lu
2022-07-14 0:06 ` Sunil Pandey
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2022-05-26 20:59 UTC (permalink / raw)
To: Fangrui Song; +Cc: Adhemerval Zanella, GNU C Library
[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]
On Wed, May 25, 2022 at 1:24 PM Fangrui Song <maskray@google.com> wrote:
>
> On 2022-05-25, H.J. Lu via Libc-alpha wrote:
> >On Wed, May 25, 2022 at 11:28 AM Adhemerval Zanella via Libc-alpha
> ><libc-alpha@sourceware.org> wrote:
> >>
> >>
> >>
> >> On 25/05/2022 15:19, Florian Weimer via Libc-alpha wrote:
> >> > * H. J. Lu:
> >> >
> >> >> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
> >> >> R_X86_64_JUMP_SLOT.
> >> >
> >> > Okay, so it's very unlikely that we see non-zero addends.
> >> > Maybe you can add this to the commit message?
> >> >
> >> > Given that, I think the change should be safe.
> >> >
> >> > Thanks,
> >> > Florian
> >> >
> >>
> >> Maybe assert that r_addend is always zero?
> >
> >I don't think it is necessary since r_addend has never been non-zero.
>
> I agree that an assert is unnecessary. Linkers just guarantee zero
> addends.
>
> It seems that by omitting reloc->r_addend, the GCC generated assembler
> is slightly shorter. I've tried placing GLOB_DAT/JUMP_SLOT near 64
> and don't get simplified assembly.
>
> Reviewed-by: Fangrui Song <maskray@google.com>
Here is the patch I am checking in.
Thanks.
--
H.J.
[-- Attachment #2: v2-0001-x86-64-Ignore-r_addend-for-R_X86_64_GLOB_DAT-R_X8.patch --]
[-- Type: text/x-patch, Size: 1147 bytes --]
From 4def09c796bcac6abbdf73a1d21b78d28160eeb1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 20 May 2022 19:21:48 -0700
Subject: [PATCH v2] x86-64: Ignore r_addend for
R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
According to x86-64 psABI, r_addend should be ignored for R_X86_64_GLOB_DAT
and R_X86_64_JUMP_SLOT. Since linkers always set their r_addends to 0, we
can ignore their r_addends.
Reviewed-by: Fangrui Song <maskray@google.com>
---
sysdeps/x86_64/dl-machine.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/sysdeps/x86_64/dl-machine.h b/sysdeps/x86_64/dl-machine.h
index c70af7ab1e..7f607f6dff 100644
--- a/sysdeps/x86_64/dl-machine.h
+++ b/sysdeps/x86_64/dl-machine.h
@@ -325,11 +325,13 @@ and creates an unsatisfiable circular dependency.\n",
# endif
/* Set to symbol size plus addend. */
value = sym->st_size;
+ *reloc_addr = value + reloc->r_addend;
+ break;
# endif
- /* Fall through. */
+
case R_X86_64_GLOB_DAT:
case R_X86_64_JUMP_SLOT:
- *reloc_addr = value + reloc->r_addend;
+ *reloc_addr = value;
break;
case R_X86_64_DTPMOD64:
--
2.36.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT
2022-05-26 20:59 ` H.J. Lu
@ 2022-07-14 0:06 ` Sunil Pandey
0 siblings, 0 replies; 10+ messages in thread
From: Sunil Pandey @ 2022-07-14 0:06 UTC (permalink / raw)
To: H.J. Lu; +Cc: Fangrui Song, GNU C Library
On Thu, May 26, 2022 at 2:00 PM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Wed, May 25, 2022 at 1:24 PM Fangrui Song <maskray@google.com> wrote:
> >
> > On 2022-05-25, H.J. Lu via Libc-alpha wrote:
> > >On Wed, May 25, 2022 at 11:28 AM Adhemerval Zanella via Libc-alpha
> > ><libc-alpha@sourceware.org> wrote:
> > >>
> > >>
> > >>
> > >> On 25/05/2022 15:19, Florian Weimer via Libc-alpha wrote:
> > >> > * H. J. Lu:
> > >> >
> > >> >> Linker always sets r_addend to 0 for R_X86_64_GLOB_DAT and
> > >> >> R_X86_64_JUMP_SLOT.
> > >> >
> > >> > Okay, so it's very unlikely that we see non-zero addends.
> > >> > Maybe you can add this to the commit message?
> > >> >
> > >> > Given that, I think the change should be safe.
> > >> >
> > >> > Thanks,
> > >> > Florian
> > >> >
> > >>
> > >> Maybe assert that r_addend is always zero?
> > >
> > >I don't think it is necessary since r_addend has never been non-zero.
> >
> > I agree that an assert is unnecessary. Linkers just guarantee zero
> > addends.
> >
> > It seems that by omitting reloc->r_addend, the GCC generated assembler
> > is slightly shorter. I've tried placing GLOB_DAT/JUMP_SLOT near 64
> > and don't get simplified assembly.
> >
> > Reviewed-by: Fangrui Song <maskray@google.com>
>
> Here is the patch I am checking in.
>
> Thanks.
>
> --
> H.J.
I would like to backport this patch to release branches.
Any comments or objections?
--Sunil
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-07-14 0:06 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-21 2:23 [PATCH] x86-64: Ignore r_addend for R_X86_64_GLOB_DAT/R_X86_64_JUMP_SLOT H.J. Lu
2022-05-22 22:10 ` Fangrui Song
2022-05-25 16:52 ` Florian Weimer
2022-05-25 17:49 ` H.J. Lu
2022-05-25 18:19 ` Florian Weimer
2022-05-25 18:28 ` Adhemerval Zanella
2022-05-25 19:29 ` H.J. Lu
2022-05-25 20:24 ` Fangrui Song
2022-05-26 20:59 ` H.J. Lu
2022-07-14 0:06 ` Sunil Pandey
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).