x86: Turn PLT32 to PC32 only for PC-relative relocations

Message ID CAMe9rOpi_sWMm+o423ys=4mn=pcVYuaV5z+R8e5RsOHEfJVjJQ@mail.gmail.com
State New
Headers
Series x86: Turn PLT32 to PC32 only for PC-relative relocations |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed

Commit Message

H.J. Lu Sept. 22, 2024, 11:05 a.m. UTC
  commit 292676c15a615b5a95bede9ee91004d3f7ee7dfd
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Feb 13 13:44:17 2020 -0800

    x86: Resolve PLT32 reloc aganst local symbol to section

resolved PLT32 relocation against local symbol to section and

commit 2585b7a5ce5830e60a089aa2316a329558902f0c
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Jul 19 06:51:19 2020 -0700

    x86: Change PLT32 reloc against section to PC32

turned PLT32 relocation against section into PC32 relocation.  But these
transformations are valid only for PC-relative relocations.  Add fx_pcrel
check for PC-relative relocations when performing these transformations.
  

Comments

Fangrui Song Sept. 22, 2024, 5:48 p.m. UTC | #1
On Sun, Sep 22, 2024 at 4:05 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> commit 292676c15a615b5a95bede9ee91004d3f7ee7dfd
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Thu Feb 13 13:44:17 2020 -0800
>
>     x86: Resolve PLT32 reloc aganst local symbol to section
>
> resolved PLT32 relocation against local symbol to section and
>
> commit 2585b7a5ce5830e60a089aa2316a329558902f0c
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Sun Jul 19 06:51:19 2020 -0700
>
>     x86: Change PLT32 reloc against section to PC32
>
> turned PLT32 relocation against section into PC32 relocation.  But these
> transformations are valid only for PC-relative relocations.  Add fx_pcrel
> check for PC-relative relocations when performing these transformations.
>
>
> --
> H.J.

The code looks good to me.

Perhaps mentioning that  `movq $foo@PLT, %rax` generates the wrong 32
instead of PLT32 makes it easier for readers to understand the
difference.

The new file ld/testsuite/ld-x86-64/pr32196.s could be renamed to
plt<something>.s for better discoverability.
  

Patch

From c580d18543cde2211007f61a9e94940275e390b4 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Sun, 22 Sep 2024 17:06:45 +0800
Subject: [PATCH] x86: Turn PLT32 to PC32 only for PC-relative relocations

commit 292676c15a615b5a95bede9ee91004d3f7ee7dfd
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Thu Feb 13 13:44:17 2020 -0800

    x86: Resolve PLT32 reloc aganst local symbol to section

resolved PLT32 relocation against local symbol to section and

commit 2585b7a5ce5830e60a089aa2316a329558902f0c
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Sun Jul 19 06:51:19 2020 -0700

    x86: Change PLT32 reloc against section to PC32

turned PLT32 relocation against section into PC32 relocation.  But these
transformations are valid only for PC-relative relocations.  Add fx_pcrel
check for PC-relative relocations when performing these transformations.

gas/

	PR gas/32196
	* config/tc-i386.c (tc_i386_fix_adjustable): Return fixP->fx_pcrel
	for PLT32 relocations.
	(i386_validate_fix): Turn PLT32 relocation into PC32 relocation
	only if fixp->fx_pcrel is set.
	* testsuite/gas/i386/reloc32.d: Updated.
	* testsuite/gas/i386/reloc64.d: Likewise.
	* testsuite/gas/i386/reloc32.s: Add PR gas/32196 test.
	* testsuite/gas/i386/reloc64.s: Likewise.

ld/

	PR gas/32196
	* testsuite/ld-x86-64/pr32196.s: New file.
	* testsuite/ld-x86-64/x86-64.exp: Run PR gas/32196 test.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 gas/config/tc-i386.c              | 11 ++++++++++-
 gas/testsuite/gas/i386/reloc32.d  |  1 +
 gas/testsuite/gas/i386/reloc32.s  |  7 +++++++
 gas/testsuite/gas/i386/reloc64.d  |  1 +
 gas/testsuite/gas/i386/reloc64.s  |  2 ++
 ld/testsuite/ld-x86-64/pr32196.s  | 26 ++++++++++++++++++++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  8 ++++++++
 7 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-x86-64/pr32196.s

diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
index 11565ac7cb0..864a38a9a20 100644
--- a/gas/config/tc-i386.c
+++ b/gas/config/tc-i386.c
@@ -3930,6 +3930,11 @@  tc_i386_fix_adjustable (fixS *fixP)
       || fixP->fx_r_type == BFD_RELOC_VTABLE_INHERIT
       || fixP->fx_r_type == BFD_RELOC_VTABLE_ENTRY)
     return 0;
+  /* Resolve PLT32 relocation against local symbol to section only for
+     PC-relative relocations.  */
+  if (fixP->fx_r_type == BFD_RELOC_386_PLT32
+      || fixP->fx_r_type == BFD_RELOC_X86_64_PLT32)
+    return fixP->fx_pcrel;
   return 1;
 }
 #endif
@@ -17792,8 +17797,12 @@  i386_validate_fix (fixS *fixp)
     {
       /* NB: Commit 292676c1 resolved PLT32 reloc aganst local symbol
 	 to section.  Since PLT32 relocation must be against symbols,
-	 turn such PLT32 relocation into PC32 relocation.  */
+	 turn such PLT32 relocation into PC32 relocation.  NB: We can
+	 turn PLT32 relocation into PC32 relocation only for PC-relative
+	 relocations since non-PC-relative relocations need PLT entries.
+       */
       if (fixp->fx_addsy
+	  && fixp->fx_pcrel
 	  && (fixp->fx_r_type == BFD_RELOC_386_PLT32
 	      || fixp->fx_r_type == BFD_RELOC_X86_64_PLT32)
 	  && symbol_section_p (fixp->fx_addsy))
diff --git a/gas/testsuite/gas/i386/reloc32.d b/gas/testsuite/gas/i386/reloc32.d
index 7d1b1ba2ae0..ebac545b200 100644
--- a/gas/testsuite/gas/i386/reloc32.d
+++ b/gas/testsuite/gas/i386/reloc32.d
@@ -43,6 +43,7 @@  Disassembly of section \.text:
 .*[ 	]+R_386_TLS_LE[ 	]+xtrn
 .*[ 	]+R_386_TLS_LE_32[ 	]+xtrn
 .*[ 	]+R_386_TLS_LE_32[ 	]+xtrn
+.*[ 	]+R_386_PLT32[ 	]+ptr
 Disassembly of section \.data:
 #...
 .*[ 	]+R_386_32[ 	]+xtrn
diff --git a/gas/testsuite/gas/i386/reloc32.s b/gas/testsuite/gas/i386/reloc32.s
index e766a3dcc25..5616cd57e3f 100644
--- a/gas/testsuite/gas/i386/reloc32.s
+++ b/gas/testsuite/gas/i386/reloc32.s
@@ -162,3 +162,10 @@  bad	.byte	xtrn@tpoff
 	.long	xtrn@got + 4
 	.long	xtrn@got - 4
 bad	.long	xtrn@plt - .
+
+	.text
+	movl	$ptr@PLT, %eax
+
+	.data
+ptr:
+	.dc.a 0
diff --git a/gas/testsuite/gas/i386/reloc64.d b/gas/testsuite/gas/i386/reloc64.d
index 665ede6f264..5fee029e56a 100644
--- a/gas/testsuite/gas/i386/reloc64.d
+++ b/gas/testsuite/gas/i386/reloc64.d
@@ -59,6 +59,7 @@  Disassembly of section \.text:
 .*[ 	]+R_X86_64_32[ 	]+xtrn
 .*[ 	]+R_X86_64_GOT64[ 	]+ptr
 .*[ 	]+R_X86_64_GOTOFF64[ 	]+Ldst
+.*[ 	]+R_X86_64_PLT32[ 	]+ptr
 Disassembly of section \.data:
 #...
 .*[ 	]+R_X86_64_64[ 	]+xtrn
diff --git a/gas/testsuite/gas/i386/reloc64.s b/gas/testsuite/gas/i386/reloc64.s
index 2293865df20..5c0f4136a09 100644
--- a/gas/testsuite/gas/i386/reloc64.s
+++ b/gas/testsuite/gas/i386/reloc64.s
@@ -229,6 +229,8 @@  bad	.long	xtrn@plt - .
 	.text
 	movabs	$ptr@GOT, %rax
 	movabs	$Ldst@GOTOFF, %rdx
+	movq	$ptr@PLT, %rax
+
 	.data
 ptr:
 	.quad 0
diff --git a/ld/testsuite/ld-x86-64/pr32196.s b/ld/testsuite/ld-x86-64/pr32196.s
new file mode 100644
index 00000000000..e2bd5125d95
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr32196.s
@@ -0,0 +1,26 @@ 
+	.text
+	.global _start
+_start:
+	movq $foo@PLT, %rax
+	leaq -11(%rip), %rbx
+	movq (%rbx, %rax), %rax
+
+	# Write out "PASS\n".
+	movl	$5, %edx
+	movl	$.LC0, %esi
+	movl	$1, %edi
+	movl	$1, %eax
+	syscall
+
+	# exit
+	movq $60, %rax
+	movq $0, %rdi
+	syscall
+
+foo:
+	ret
+
+	.section	.rodata.str1.1,"aMS",@progbits,1
+.LC0:
+	.string	"PASS\n"
+	.section	.note.GNU-stack,"",@progbits
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index cf78fffe29f..a1c7d978932 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -1929,6 +1929,14 @@  if { [isnative] && [check_compiler_available] } {
 	    "pr32189" \
 	    "pass.out" \
 	] \
+	[list \
+	    "Run pr32196" \
+	    "$NOPIE_LDFLAGS -nostdlib -nostartfiles" \
+	    "" \
+	    { pr32196.s } \
+	    "pr32196" \
+	    "pass.out" \
+	] \
     ]
 
     # Run-time tests which require working ifunc attribute support.
-- 
2.46.1