x86: Treat relocation against IFUNC symbol as FUNC

Message ID 20180307133645.GA15290@gmail.com
State New
Headers show
Series
  • x86: Treat relocation against IFUNC symbol as FUNC
Related show

Commit Message

H.J. Lu March 7, 2018, 1:36 p.m.
When resolving a relocation against IFUNC symbol in a SHT_NOTE section
without SHF_ALLOC, we treat it as relocation against FUNC symbol since
it needs the address of IFUNC symbol, not the address returned by IFUNC
function.

Any comments?

H.J.
---
bfd/

	PR ld/22929
	* elf32-i386.c (elf_i386_relocate_section): Treat relocation
	against IFUNC symbol in SHT_NOTE section without SHF_ALLOC as
	relocation against FUNC symbol.
	* elf64-x86-64.c (elf_x86_64_relocate_section): Likewise.

ld/

	PR ld/22929
	* testsuite/ld-i386/i386.exp: Run PR ld/22929 test.
	* testsuite/ld-x86-64/x86-64.exp: Likewise.
	* testsuite/ld-i386/pr22929.d: New file.
	* testsuite/ld-i386/pr22929.s: Likewise.
	* testsuite/ld-x86-64/pr22929.d: Likewise.
	* testsuite/ld-x86-64/pr22929.s: Likewise.
---
 bfd/elf32-i386.c                  |  5 +++++
 bfd/elf64-x86-64.c                |  5 +++++
 ld/testsuite/ld-i386/i386.exp     |  1 +
 ld/testsuite/ld-i386/pr22929.d    | 11 +++++++++++
 ld/testsuite/ld-i386/pr22929.s    | 21 +++++++++++++++++++++
 ld/testsuite/ld-x86-64/pr22929.d  | 11 +++++++++++
 ld/testsuite/ld-x86-64/pr22929.s  | 21 +++++++++++++++++++++
 ld/testsuite/ld-x86-64/x86-64.exp |  1 +
 8 files changed, 76 insertions(+)
 create mode 100644 ld/testsuite/ld-i386/pr22929.d
 create mode 100644 ld/testsuite/ld-i386/pr22929.s
 create mode 100644 ld/testsuite/ld-x86-64/pr22929.d
 create mode 100644 ld/testsuite/ld-x86-64/pr22929.s

-- 
2.14.3

Comments

Nick Clifton March 7, 2018, 2:05 p.m. | #1
Hi H.J.

> When resolving a relocation against IFUNC symbol in a SHT_NOTE section

> without SHF_ALLOC, we treat it as relocation against FUNC symbol since

> it needs the address of IFUNC symbol, not the address returned by IFUNC

> function.

> 

> Any comments?


Works for me.  If you do not hear anything from anyone else by tomorrow,
please consider this patch approved.

Cheers
  Nick
Nick Clifton March 7, 2018, 2:34 p.m. | #2
Hi H.J.

>> When resolving a relocation against IFUNC symbol in a SHT_NOTE section

>> without SHF_ALLOC, we treat it as relocation against FUNC symbol since

>> it needs the address of IFUNC symbol, not the address returned by IFUNC

>> function.


Actually - thinking about it further ... why should it matter that it is a
note section that is being relocated ?  Surely the same logic should apply
to any ifunc-targeted relocation in a non-alloc section.  For example, I
wonder if your patch would actually work for debug sections as well ? 
(Rather than the current option of just silently ignoring the reloc).

Cheers
  Nick
H.J. Lu March 7, 2018, 2:47 p.m. | #3
On Wed, Mar 7, 2018 at 6:34 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi H.J.

>

>>> When resolving a relocation against IFUNC symbol in a SHT_NOTE section

>>> without SHF_ALLOC, we treat it as relocation against FUNC symbol since

>>> it needs the address of IFUNC symbol, not the address returned by IFUNC

>>> function.

>

> Actually - thinking about it further ... why should it matter that it is a

> note section that is being relocated ?  Surely the same logic should apply

> to any ifunc-targeted relocation in a non-alloc section.  For example, I

> wonder if your patch would actually work for debug sections as well ?

> (Rather than the current option of just silently ignoring the reloc).


GDB has special handling for IFUNC symbols which works well for the
current approach with debug sections.  Will such a change break GDB?

-- 
H.J.
Nick Clifton March 7, 2018, 3:04 p.m. | #4
Hi H.J.

>> Actually - thinking about it further ... why should it matter that it is a

>> note section that is being relocated ?  Surely the same logic should apply

>> to any ifunc-targeted relocation in a non-alloc section.  For example, I

>> wonder if your patch would actually work for debug sections as well ?

>> (Rather than the current option of just silently ignoring the reloc).

> 

> GDB has special handling for IFUNC symbols which works well for the

> current approach with debug sections.  Will such a change break GDB?


Probably.  I did not know that GDB had such handling.

OK, so apart from debug sections, is there any reason not to treat ifunc
targeted relocs in non-alloc sections as if they were targeted at the func
instead ?  IE not just for note sections, but for all non-alloc, non-debug
sections ?  

Cheers
  Nick
H.J. Lu March 7, 2018, 3:50 p.m. | #5
On Wed, Mar 7, 2018 at 7:04 AM, Nick Clifton <nickc@redhat.com> wrote:
> Hi H.J.

>

>>> Actually - thinking about it further ... why should it matter that it is a

>>> note section that is being relocated ?  Surely the same logic should apply

>>> to any ifunc-targeted relocation in a non-alloc section.  For example, I

>>> wonder if your patch would actually work for debug sections as well ?

>>> (Rather than the current option of just silently ignoring the reloc).

>>

>> GDB has special handling for IFUNC symbols which works well for the

>> current approach with debug sections.  Will such a change break GDB?

>

> Probably.  I did not know that GDB had such handling.

>

> OK, so apart from debug sections, is there any reason not to treat ifunc

> targeted relocs in non-alloc sections as if they were targeted at the func

> instead ?  IE not just for note sections, but for all non-alloc, non-debug

> sections ?


This is the first time we saw abort.  There are 2 ways to handle it, skip it
or treat it as FUNC.  We don't know which way is appropriate for the next
instance.  I prefer to keep my patch as is.

-- 
H.J.

Patch

diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index 61a14097b0..557084647c 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -2221,6 +2221,10 @@  elf_i386_relocate_section (bfd *output_bfd,
 
 	  if ((input_section->flags & SEC_ALLOC) == 0)
 	    {
+	      /* If this is a SHT_NOTE section without SHF_ALLOC, treat
+	         STT_GNU_IFUNC symbol as STT_FUNC.  */
+	      if (elf_section_type (input_section) == SHT_NOTE)
+		goto skip_ifunc;
 	      /* Dynamic relocs are not propagated for SEC_DEBUGGING
 		 sections because such sections are not SEC_ALLOC and
 		 thus ld.so will not process them.  */
@@ -2440,6 +2444,7 @@  do_ifunc_pointer:
 	    }
 	}
 
+skip_ifunc:
       resolved_to_zero = (eh != NULL
 			  && UNDEFINED_WEAK_RESOLVED_TO_ZERO (info, eh));
 
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index 7016964ace..a964316e44 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -2503,6 +2503,10 @@  elf_x86_64_relocate_section (bfd *output_bfd,
 
 	  if ((input_section->flags & SEC_ALLOC) == 0)
 	    {
+	      /* If this is a SHT_NOTE section without SHF_ALLOC, treat
+	         STT_GNU_IFUNC symbol as STT_FUNC.  */
+	      if (elf_section_type (input_section) == SHT_NOTE)
+		goto skip_ifunc;
 	      /* Dynamic relocs are not propagated for SEC_DEBUGGING
 		 sections because such sections are not SEC_ALLOC and
 		 thus ld.so will not process them.  */
@@ -2726,6 +2730,7 @@  do_ifunc_pointer:
 	    }
 	}
 
+skip_ifunc:
       resolved_to_zero = (eh != NULL
 			  && UNDEFINED_WEAK_RESOLVED_TO_ZERO (info, eh));
 
diff --git a/ld/testsuite/ld-i386/i386.exp b/ld/testsuite/ld-i386/i386.exp
index 240850a02c..243c0a57a1 100644
--- a/ld/testsuite/ld-i386/i386.exp
+++ b/ld/testsuite/ld-i386/i386.exp
@@ -459,6 +459,7 @@  run_dump_test "pr22115-1c"
 run_dump_test "pr22115-1d"
 run_dump_test "pr22135"
 run_dump_test "pr22782"
+run_dump_test "pr22929"
 
 if { !([istarget "i?86-*-linux*"]
        || [istarget "i?86-*-gnu*"]
diff --git a/ld/testsuite/ld-i386/pr22929.d b/ld/testsuite/ld-i386/pr22929.d
new file mode 100644
index 0000000000..099880e94d
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr22929.d
@@ -0,0 +1,11 @@ 
+#as: --32
+#ld: -melf_i386 -shared
+#readelf: -r --wide -s
+
+There are no relocations in this file.
+#...
+Symbol table '.symtab' contains [0-9]+ entries:
+   Num:    Value  Size Type    Bind   Vis      Ndx Name
+#...
+[ \t]+[a-f0-9]+: [a-f0-9]+ +1 IFUNC +GLOBAL DEFAULT +[a-f0-9]+ +foo
+#...
diff --git a/ld/testsuite/ld-i386/pr22929.s b/ld/testsuite/ld-i386/pr22929.s
new file mode 100644
index 0000000000..3fa3dc949c
--- /dev/null
+++ b/ld/testsuite/ld-i386/pr22929.s
@@ -0,0 +1,21 @@ 
+	.text
+	.globl	foo
+	.type foo, %gnu_indirect_function
+foo:
+	ret
+	.size	foo, .-foo
+	.pushsection .gnu.build.attributes, "", %note
+	.dc.l 6		# size of name
+	.dc.l 16	# descsz = sizeof (addresses)
+	.dc.l 0x101	# type = FUNC
+	.dc.b 0x47, 0x41, 0x2a, 0x2, 0, 0 	# name (numeric: -fstack-protector status)
+	.dc.b 0, 0 	# Padding
+	.dc.a foo
+	.dc.a foo_end	# description (symbol name)
+	.popsection
+
+foo_end:
+	.section	.rodata.cst4,"aM",@progbits,4
+	.align 4
+.LC0:
+	.long	1065353216
diff --git a/ld/testsuite/ld-x86-64/pr22929.d b/ld/testsuite/ld-x86-64/pr22929.d
new file mode 100644
index 0000000000..537b3969e8
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22929.d
@@ -0,0 +1,11 @@ 
+#as: --64
+#ld: -melf_x86_64 -shared
+#readelf: -r --wide -s
+
+There are no relocations in this file.
+#...
+Symbol table '.symtab' contains [0-9]+ entries:
+   Num:    Value          Size Type    Bind   Vis      Ndx Name
+#...
+[ \t]+[a-f0-9]+: [a-f0-9]+ +1 IFUNC +GLOBAL DEFAULT +[a-f0-9]+ +foo
+#...
diff --git a/ld/testsuite/ld-x86-64/pr22929.s b/ld/testsuite/ld-x86-64/pr22929.s
new file mode 100644
index 0000000000..3fa3dc949c
--- /dev/null
+++ b/ld/testsuite/ld-x86-64/pr22929.s
@@ -0,0 +1,21 @@ 
+	.text
+	.globl	foo
+	.type foo, %gnu_indirect_function
+foo:
+	ret
+	.size	foo, .-foo
+	.pushsection .gnu.build.attributes, "", %note
+	.dc.l 6		# size of name
+	.dc.l 16	# descsz = sizeof (addresses)
+	.dc.l 0x101	# type = FUNC
+	.dc.b 0x47, 0x41, 0x2a, 0x2, 0, 0 	# name (numeric: -fstack-protector status)
+	.dc.b 0, 0 	# Padding
+	.dc.a foo
+	.dc.a foo_end	# description (symbol name)
+	.popsection
+
+foo_end:
+	.section	.rodata.cst4,"aM",@progbits,4
+	.align 4
+.LC0:
+	.long	1065353216
diff --git a/ld/testsuite/ld-x86-64/x86-64.exp b/ld/testsuite/ld-x86-64/x86-64.exp
index 02aa38a75a..5a06947057 100644
--- a/ld/testsuite/ld-x86-64/x86-64.exp
+++ b/ld/testsuite/ld-x86-64/x86-64.exp
@@ -605,6 +605,7 @@  run_dump_test "pr20253-5a"
 run_dump_test "pr20253-5b"
 run_dump_test "tlsdesc2"
 run_dump_test "pr22048"
+run_dump_test "pr22929"
 
 proc undefined_weak {cflags ldflags} {
     set testname "Undefined weak symbol"