* PATCH: Replace @PLT with @GOTPCREL(%rip) in call
@ 2016-03-03 13:21 H.J. Lu
2016-03-04 0:03 ` Roland McGrath
0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2016-03-03 13:21 UTC (permalink / raw)
To: GNU C Library
Since both PREINIT_FUNCTION and __libc_start_main are defined externally,
they are called via PLT. Use "call *func@GOTPCREL(%rip)" removes the
extra branch to PLT entry.
[BZ #19745]
* sysdeps/x86_64/crti.S (_init): Replace @PLT with
@GOTPCREL(%rip) in call.
* sysdeps/x86_64/start.S (_start): Likewise.
---
sysdeps/x86_64/crti.S | 2 +-
sysdeps/x86_64/start.S | 2 +-
sysdeps/x86_64/sysdep.h | 14 +++++++++++---
3 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/sysdeps/x86_64/crti.S b/sysdeps/x86_64/crti.S
index a345259..3dff141 100644
--- a/sysdeps/x86_64/crti.S
+++ b/sysdeps/x86_64/crti.S
@@ -66,7 +66,7 @@ _init:
movq PREINIT_FUNCTION@GOTPCREL(%rip), %rax
testq %rax, %rax
je .Lno_weak_fn
- call PREINIT_FUNCTION@PLT
+ call *PREINIT_FUNCTION@GOTPCREL(%rip)
.Lno_weak_fn:
#else
call PREINIT_FUNCTION
diff --git a/sysdeps/x86_64/start.S b/sysdeps/x86_64/start.S
index 1374974..22b18ae 100644
--- a/sysdeps/x86_64/start.S
+++ b/sysdeps/x86_64/start.S
@@ -105,7 +105,7 @@ ENTRY (_start)
/* Call the user's main function, and exit with its value.
But let the libc call main. */
- call __libc_start_main@PLT
+ call *__libc_start_main@GOTPCREL(%rip)
#else
/* Pass address of our own entry points to .fini and .init. */
mov $__libc_csu_fini, %R8_LP
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: Replace @PLT with @GOTPCREL(%rip) in call
2016-03-03 13:21 PATCH: Replace @PLT with @GOTPCREL(%rip) in call H.J. Lu
@ 2016-03-04 0:03 ` Roland McGrath
2016-03-04 23:40 ` H.J. Lu
0 siblings, 1 reply; 4+ messages in thread
From: Roland McGrath @ 2016-03-04 0:03 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
> --- a/sysdeps/x86_64/crti.S
> +++ b/sysdeps/x86_64/crti.S
> @@ -66,7 +66,7 @@ _init:
> movq PREINIT_FUNCTION@GOTPCREL(%rip), %rax
> testq %rax, %rax
> je .Lno_weak_fn
> - call PREINIT_FUNCTION@PLT
> + call *PREINIT_FUNCTION@GOTPCREL(%rip)
Can't this just be "call *%rax"?
> --- a/sysdeps/x86_64/start.S
> +++ b/sysdeps/x86_64/start.S
> @@ -105,7 +105,7 @@ ENTRY (_start)
>
> /* Call the user's main function, and exit with its value.
> But let the libc call main. */
> - call __libc_start_main@PLT
> + call *__libc_start_main@GOTPCREL(%rip)
> #else
> /* Pass address of our own entry points to .fini and .init. */
> mov $__libc_csu_fini, %R8_LP
In the general case, foo@PLT is preferable to foo@GOTPCREL(%rip) because it
allows for lazy resolution. That's not worthwhile here because _start is
presumably always run and run very early. But it merits a comment saying
why it's not following the normal pattern for PIC calls.
Thanks,
Roland
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: Replace @PLT with @GOTPCREL(%rip) in call
2016-03-04 0:03 ` Roland McGrath
@ 2016-03-04 23:40 ` H.J. Lu
2016-03-04 23:46 ` Roland McGrath
0 siblings, 1 reply; 4+ messages in thread
From: H.J. Lu @ 2016-03-04 23:40 UTC (permalink / raw)
To: Roland McGrath; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]
On Thu, Mar 3, 2016 at 4:03 PM, Roland McGrath <roland@hack.frob.com> wrote:
>> --- a/sysdeps/x86_64/crti.S
>> +++ b/sysdeps/x86_64/crti.S
>> @@ -66,7 +66,7 @@ _init:
>> movq PREINIT_FUNCTION@GOTPCREL(%rip), %rax
>> testq %rax, %rax
>> je .Lno_weak_fn
>> - call PREINIT_FUNCTION@PLT
>> + call *PREINIT_FUNCTION@GOTPCREL(%rip)
>
> Can't this just be "call *%rax"?
Good idea.
>> --- a/sysdeps/x86_64/start.S
>> +++ b/sysdeps/x86_64/start.S
>> @@ -105,7 +105,7 @@ ENTRY (_start)
>>
>> /* Call the user's main function, and exit with its value.
>> But let the libc call main. */
>> - call __libc_start_main@PLT
>> + call *__libc_start_main@GOTPCREL(%rip)
>> #else
>> /* Pass address of our own entry points to .fini and .init. */
>> mov $__libc_csu_fini, %R8_LP
>
> In the general case, foo@PLT is preferable to foo@GOTPCREL(%rip) because it
> allows for lazy resolution. That's not worthwhile here because _start is
> presumably always run and run very early. But it merits a comment saying
> why it's not following the normal pattern for PIC calls.
>
Here are updated patches.
--
H.J.
[-- Attachment #2: 0001-Replace-PLT-with-GOTPCREL-rip-in-call.patch --]
[-- Type: text/x-patch, Size: 1714 bytes --]
From 976cfff7e4538503c70413d6746ccbcf87e395e1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 4 Mar 2016 15:28:59 -0800
Subject: [PATCH 1/2] Replace @PLT with @GOTPCREL(%rip) in call
Since __libc_start_main is called very early, lazy binding isn't relevant
here. Use indirect branch via GOT to avoid extra branch to PLT slot.
[BZ #19745]
* sysdeps/x86_64/start.S (_start): __libc_start_main@PLT
with *__libc_start_main@GOTPCREL(%rip) in call.
---
ChangeLog | 6 ++++++
sysdeps/x86_64/start.S | 6 ++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 143d4a6..af55529 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2016-03-04 H.J. Lu <hongjiu.lu@intel.com>
+
+ [BZ #19745]
+ * sysdeps/x86_64/start.S (_start): Replace __libc_start_main@PLT
+ with *__libc_start_main@GOTPCREL(%rip) in call.
+
2016-03-04 Roland McGrath <roland@hack.frob.com>
* Makefile (tests-special): Add $(objpfx)c++-types-check.out only
diff --git a/sysdeps/x86_64/start.S b/sysdeps/x86_64/start.S
index 1374974..2369b69 100644
--- a/sysdeps/x86_64/start.S
+++ b/sysdeps/x86_64/start.S
@@ -104,8 +104,10 @@ ENTRY (_start)
mov main@GOTPCREL(%rip), %RDI_LP
/* Call the user's main function, and exit with its value.
- But let the libc call main. */
- call __libc_start_main@PLT
+ But let the libc call main. Since __libc_start_main is
+ called very early, lazy binding isn't relevant here. Use
+ indirect branch via GOT to avoid extra branch to PLT slot. */
+ call *__libc_start_main@GOTPCREL(%rip)
#else
/* Pass address of our own entry points to .fini and .init. */
mov $__libc_csu_fini, %R8_LP
--
2.5.0
[-- Attachment #3: 0002-Replace-PREINIT_FUNCTION-PLT-with-rax-in-call.patch --]
[-- Type: text/x-patch, Size: 1324 bytes --]
From 9ff9ae1b792e3d986f68a127e0b3f8679e6bea17 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 4 Mar 2016 15:36:18 -0800
Subject: [PATCH 2/2] Replace PREINIT_FUNCTION@PLT with *%rax in call
Since we have loaded address of PREINIT_FUNCTION into %rax, we can
avoid extra branch to PLT slot.
[BZ #19745]
* sysdeps/x86_64/crti.S (_init): Replace PREINIT_FUNCTION@PLT
with *%rax in call.
---
ChangeLog | 6 ++++++
sysdeps/x86_64/crti.S | 2 +-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index af55529..f5b8d1e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,12 @@
2016-03-04 H.J. Lu <hongjiu.lu@intel.com>
[BZ #19745]
+ * sysdeps/x86_64/crti.S (_init): Replace PREINIT_FUNCTION@PLT
+ with *%rax in call.
+
+2016-03-04 H.J. Lu <hongjiu.lu@intel.com>
+
+ [BZ #19745]
* sysdeps/x86_64/start.S (_start): Replace __libc_start_main@PLT
with *__libc_start_main@GOTPCREL(%rip) in call.
diff --git a/sysdeps/x86_64/crti.S b/sysdeps/x86_64/crti.S
index a345259..e9d86ed 100644
--- a/sysdeps/x86_64/crti.S
+++ b/sysdeps/x86_64/crti.S
@@ -66,7 +66,7 @@ _init:
movq PREINIT_FUNCTION@GOTPCREL(%rip), %rax
testq %rax, %rax
je .Lno_weak_fn
- call PREINIT_FUNCTION@PLT
+ call *%rax
.Lno_weak_fn:
#else
call PREINIT_FUNCTION
--
2.5.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: PATCH: Replace @PLT with @GOTPCREL(%rip) in call
2016-03-04 23:40 ` H.J. Lu
@ 2016-03-04 23:46 ` Roland McGrath
0 siblings, 0 replies; 4+ messages in thread
From: Roland McGrath @ 2016-03-04 23:46 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
Both look fine to me.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-04 23:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-03 13:21 PATCH: Replace @PLT with @GOTPCREL(%rip) in call H.J. Lu
2016-03-04 0:03 ` Roland McGrath
2016-03-04 23:40 ` H.J. Lu
2016-03-04 23:46 ` Roland McGrath
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).