[1/2] bfd/elfnn-aarch64.c: Fix miscalculation of GOTPLT offset for ifunc syms.

Message ID 529461A9.4060909@linaro.org
State Accepted
Headers show

Commit Message

Will Newton Nov. 26, 2013, 8:54 a.m.
The .got.plt header size was not being correctly taken into account
when calculating the offset for relocations against ifunc symbols.

bfd/ChangeLog:

2013-11-25  Will Newton  <will.newton@linaro.org>

	* elfnn-aarch64.c (elfNN_aarch64_final_link_relocate): Ensure
	PLT_INDEX is calculated using correct header size.

ld/testsuite/ChangeLog:

2013-11-25  Will Newton  <will.newton@linaro.org>

	* ld-aarch64/aarch64-elf.exp: Add ifunc-21 test.
	* ld-aarch64/ifunc-21.d: New file.
	* ld-aarch64/ifunc-21.s: Likewise.
---
 bfd/elfnn-aarch64.c                     |  3 ++-
 ld/testsuite/ld-aarch64/aarch64-elf.exp |  1 +
 ld/testsuite/ld-aarch64/ifunc-21.d      | 31 +++++++++++++++++++++++++++++++
 ld/testsuite/ld-aarch64/ifunc-21.s      | 13 +++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-aarch64/ifunc-21.d
 create mode 100644 ld/testsuite/ld-aarch64/ifunc-21.s

OK for trunk and binutils_2_24-branch? I am aware these changes are quite late but they only touch STT_GNU_IFUNC
code paths.

Comments

Marcus Shawcroft Nov. 26, 2013, 10:29 a.m. | #1
On 26 November 2013 08:54, Will Newton <will.newton@linaro.org> wrote:
>
> The .got.plt header size was not being correctly taken into account
> when calculating the offset for relocations against ifunc symbols.
>
> bfd/ChangeLog:
>
> 2013-11-25  Will Newton  <will.newton@linaro.org>
>
>         * elfnn-aarch64.c (elfNN_aarch64_final_link_relocate): Ensure
>         PLT_INDEX is calculated using correct header size.
>
> ld/testsuite/ChangeLog:
>
> 2013-11-25  Will Newton  <will.newton@linaro.org>
>
>         * ld-aarch64/aarch64-elf.exp: Add ifunc-21 test.
>         * ld-aarch64/ifunc-21.d: New file.
>         * ld-aarch64/ifunc-21.s: Likewise.

This looks OK to me.
/Marcus
Yufeng Zhang Dec. 10, 2013, 4:26 p.m. | #2
Hi Will,

On 11/26/13 08:54, Will Newton wrote:
[snip]
> diff --git a/ld/testsuite/ld-aarch64/ifunc-21.d b/ld/testsuite/ld-aarch64/ifunc-21.d
> new file mode 100644
> index 0000000..fa139b2
> --- /dev/null
> +++ b/ld/testsuite/ld-aarch64/ifunc-21.d
> @@ -0,0 +1,31 @@
> +#source: ifunc-21.s
> +#ld: -shared -z nocombreloc
> +#objdump: -d -s -j .got.plt -j .text
> +#target: aarch64*-*-*
> +
> +# Ensure the .got.plt slot used is correct
> +
> +.*:     file format elf64-(little|big)aarch64
> +
> +Contents of section .text:
> + 02a0 .*
> +Contents of section .got.plt:
> + 103a8 0+ 0+ 0+ 0+  .*
> + 103b8 0+ 0+ [0-9a-f]+ 0+  .*
> +
> +Disassembly of section .text:
> +
> +0+2a0<ifunc>:
> + 2a0:	d65f03c0 	ret
> +
> +0+2a4<bar>:
> + 2a4:	90000080 	adrp	x0, 10000<.*>
> + 2a8:	f941e000 	ldr	x0, \[x0,#960\]
> + 2ac:	d65f03c0 	ret

This test case (and ifunc-22) fails on aarch64*-*-elf targets because of 
different section starting addresses; I attached one example dump at the 
end of the email.  I tried to fix the test case by replacing the 
hard-coded number with regexp, but I think by doing that it defeats the 
original idea of checking the right .got.plt slot is used.  Can you 
please have a quick look at and see if the test cases can be improved to 
be less sensitive to section starting addresses?

Thanks,
Yufeng

P.s. The dump I had on aarch64-none-elf:

Contents of section .text:
  02f0 c0035fd6 80000090 000842f9 c0035fd6  .._.......B..._.
Contents of section .got.plt:
  103f8 00000000 00000000 00000000 00000000  ................
  10408 00000000 00000000 c0020000 00000000  ................

Disassembly of section .text:

00000000000002f0 <ifunc>:
  2f0:   d65f03c0        ret

00000000000002f4 <bar>:
  2f4:   90000080        adrp    x0, 10000 <bar+0xfd0c>
  2f8:   f9420800        ldr     x0, [x0,#1040]
  2fc:   d65f03c0        ret

Patch

diff --git a/bfd/elfnn-aarch64.c b/bfd/elfnn-aarch64.c
index 48fa3d2..7cce6f4 100644
--- a/bfd/elfnn-aarch64.c
+++ b/bfd/elfnn-aarch64.c
@@ -3589,7 +3589,8 @@  elfNN_aarch64_final_link_relocate (reloc_howto_type *howto,

 	      if (globals->root.splt != NULL)
 		{
-		  plt_index = h->plt.offset / globals->plt_entry_size - 1;
+		  plt_index = ((h->plt.offset - globals->plt_header_size) /
+			       globals->plt_entry_size);
 		  off = (plt_index + 3) * GOT_ENTRY_SIZE;
 		  base_got = globals->root.sgotplt;
 		}
diff --git a/ld/testsuite/ld-aarch64/aarch64-elf.exp b/ld/testsuite/ld-aarch64/aarch64-elf.exp
index 5c150dd..a6b3ea2 100644
--- a/ld/testsuite/ld-aarch64/aarch64-elf.exp
+++ b/ld/testsuite/ld-aarch64/aarch64-elf.exp
@@ -155,3 +155,4 @@  run_dump_test "ifunc-18b"
 run_dump_test "ifunc-19a"
 run_dump_test "ifunc-19b"
 run_dump_test "ifunc-20"
+run_dump_test "ifunc-21"
diff --git a/ld/testsuite/ld-aarch64/ifunc-21.d b/ld/testsuite/ld-aarch64/ifunc-21.d
new file mode 100644
index 0000000..fa139b2
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/ifunc-21.d
@@ -0,0 +1,31 @@ 
+#source: ifunc-21.s
+#ld: -shared -z nocombreloc
+#objdump: -d -s -j .got.plt -j .text
+#target: aarch64*-*-*
+
+# Ensure the .got.plt slot used is correct
+
+.*:     file format elf64-(little|big)aarch64
+
+Contents of section .text:
+ 02a0 .*
+Contents of section .got.plt:
+ 103a8 0+ 0+ 0+ 0+  .*
+ 103b8 0+ 0+ [0-9a-f]+ 0+  .*
+
+Disassembly of section .text:
+
+0+2a0 <ifunc>:
+ 2a0:	d65f03c0 	ret
+
+0+2a4 <bar>:
+ 2a4:	90000080 	adrp	x0, 10000 <.*>
+ 2a8:	f941e000 	ldr	x0, \[x0,#960\]
+ 2ac:	d65f03c0 	ret
+
+Disassembly of section .got.plt:
+
+.*:
+.*
+.*
+.*
diff --git a/ld/testsuite/ld-aarch64/ifunc-21.s b/ld/testsuite/ld-aarch64/ifunc-21.s
new file mode 100644
index 0000000..a1563dc
--- /dev/null
+++ b/ld/testsuite/ld-aarch64/ifunc-21.s
@@ -0,0 +1,13 @@ 
+	.text
+	.type ifunc, @gnu_indirect_function
+	.hidden ifunc
+ifunc:
+	ret
+	.size	ifunc, .-ifunc
+	.type bar, @function
+	.globl bar
+bar:
+        adrp    x0, :got:ifunc
+        ldr     x0, [x0, #:got_lo12:ifunc]
+	ret
+	.size	bar, .-bar