This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [PATCH] x86; Allow IFUNC pointer defined in PDE
On Fri, May 11, 2018 at 2:21 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Fri, May 11, 2018 at 10:50 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
>> If STT_GNU_IFUNC symbol is defined in position-dependent executable, we
>> should change it to the normal function and set its address to its PLT
>> entry which should be resolved by R_*_IRELATIVE at run-time. All
>> external references should be resolved to its PLT in executable.
>>
>> The type field is added to elf_x86_link_hash_entry to keep track of the
>> originl symbol file. elf_x86_allocate_dynrelocs is updated to change
>> IFUNC symbol defined in PDE to FUNC and its address will be changed
>> to its PLT entry by _bfd_x86_elf_link_fixup_ifunc_symbol.
>>
>> Any comments?
>>
>>
>> H.J.
>> ---
>> bfd/
>>
>> PR ld/23169
>> * elf-ifunc.c (_bfd_elf_allocate_ifunc_dyn_relocs): Don't issue
>> an error on IFUNC pointer defined in PDE.
>> * elf32-i386.c (elf_i386_check_relocs): Copy h->type to
>> eh->type.
>> (elf_i386_relocate_section): Check eh->type instead of h->type.
>> (elf_i386_finish_dynamic_symbol): Pass eh to PLT_LOCAL_IFUNC_P.
>> Call _bfd_x86_elf_link_fixup_ifunc_symbol. Check eh->type
>> instead of h->type.
>> * elf64-x86-64.c (elf_x86_64_check_relocs): Copy h->type to
>> eh->type.
>> (elf_x86_64_relocate_section): Check eh->type instead of h->type.
>> (elf_x86_64_finish_dynamic_symbol): Pass eh to PLT_LOCAL_IFUNC_P.
>> Call _bfd_x86_elf_link_fixup_ifunc_symbol. Check eh->type
>> instead of h->type.
>> * elfxx-x86.c (elf_x86_allocate_dynrelocs): Change IFUNC symbol
>> defined in PDE to FUNC.
>> (_bfd_x86_elf_link_fixup_ifunc_symbol): New function.
>> * elfxx-x86.h (PLT_LOCAL_IFUNC_P): Take EH instead of H.
>> (elf_x86_link_hash_entry): Add type.
>> (_bfd_x86_elf_link_fixup_ifunc_symbol): New.
>>
>> ld/
>>
>> PR ld/23169
>> * testsuite/ld-ifunc/ifunc-9-i386.d: New file.
>> * testsuite/ld-ifunc/ifunc-9-x86-64.d: Likewise.
>> * testsuite/ld-ifunc/pr23169a.c: Likewise.
>> * testsuite/ld-ifunc/pr23169a.rd: Likewise.
>> * testsuite/ld-ifunc/pr23169b.c: Likewise.
>> * testsuite/ld-ifunc/pr23169b.c: Likewise.
>> * testsuite/ld-ifunc/pr23169c.rd: Likewise.
>> * testsuite/ld-ifunc/pr23169c.rd: Likewise.
>> * testsuite/ld-ifunc/ifunc-9-x86.d: Removed.
>> * testsuite/ld-ifunc/ifunc.exp: Run PR ld/23169 tests.
>
> I am going to check in this much simpler patch next week.
>
This is what I checked in.
--
H.J.
From 83d6242ad6f746fa07b64a644290e63ff6b6ca06 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 11 May 2018 10:16:43 -0700
Subject: [PATCH] x86; Allow IFUNC pointer defined in PDE
If IFUNC symbol is defined in position-dependent executable, we should
change it to the normal function and set its address to its PLT entry
which should be resolved by R_*_IRELATIVE at run-time. All external
references should be resolved to its PLT in executable.
bfd/
PR ld/23169
* elf-ifunc.c (_bfd_elf_allocate_ifunc_dyn_relocs): Don't issue
an error on IFUNC pointer defined in PDE.
* elf32-i386.c (elf_i386_finish_dynamic_symbol): Call
_bfd_x86_elf_link_fixup_ifunc_symbol.
* elf64-x86-64.c (elf_x86_64_finish_dynamic_symbol): Likewise.
* elfxx-x86.c (_bfd_x86_elf_link_fixup_ifunc_symbol): New
function.
* elfxx-x86.h (_bfd_x86_elf_link_fixup_ifunc_symbol): New.
ld/
PR ld/23169
* testsuite/ld-ifunc/ifunc-9-i386.d: New file.
* testsuite/ld-ifunc/ifunc-9-x86-64.d: Likewise.
* testsuite/ld-ifunc/pr23169a.c: Likewise.
* testsuite/ld-ifunc/pr23169a.rd: Likewise.
* testsuite/ld-ifunc/pr23169b.c: Likewise.
* testsuite/ld-ifunc/pr23169b.c: Likewise.
* testsuite/ld-ifunc/pr23169c.rd: Likewise.
* testsuite/ld-ifunc/pr23169c.rd: Likewise.
* testsuite/ld-ifunc/ifunc-9-x86.d: Removed.
* testsuite/ld-ifunc/ifunc.exp: Run PR ld/23169 tests.
---
bfd/elf-ifunc.c | 9 +-
bfd/elf32-i386.c | 2 +
bfd/elf64-x86-64.c | 2 +
bfd/elfxx-x86.c | 46 +++++++++
bfd/elfxx-x86.h | 4 +
ld/testsuite/ld-ifunc/ifunc-9-i386.d | 9 ++
ld/testsuite/ld-ifunc/ifunc-9-x86-64.d | 9 ++
ld/testsuite/ld-ifunc/ifunc-9-x86.d | 3 -
ld/testsuite/ld-ifunc/ifunc.exp | 124 +++++++++++++++++++++++++
ld/testsuite/ld-ifunc/pr23169a.c | 9 ++
ld/testsuite/ld-ifunc/pr23169a.rd | 6 ++
ld/testsuite/ld-ifunc/pr23169b.c | 23 +++++
ld/testsuite/ld-ifunc/pr23169b.rd | 3 +
ld/testsuite/ld-ifunc/pr23169c.c | 13 +++
ld/testsuite/ld-ifunc/pr23169c.rd | 6 ++
15 files changed, 264 insertions(+), 4 deletions(-)
create mode 100644 ld/testsuite/ld-ifunc/ifunc-9-i386.d
create mode 100644 ld/testsuite/ld-ifunc/ifunc-9-x86-64.d
delete mode 100644 ld/testsuite/ld-ifunc/ifunc-9-x86.d
create mode 100644 ld/testsuite/ld-ifunc/pr23169a.c
create mode 100644 ld/testsuite/ld-ifunc/pr23169a.rd
create mode 100644 ld/testsuite/ld-ifunc/pr23169b.c
create mode 100644 ld/testsuite/ld-ifunc/pr23169b.rd
create mode 100644 ld/testsuite/ld-ifunc/pr23169c.c
create mode 100644 ld/testsuite/ld-ifunc/pr23169c.rd
diff --git a/bfd/elf-ifunc.c b/bfd/elf-ifunc.c
index 5716ab36f1..b8a3632095 100644
--- a/bfd/elf-ifunc.c
+++ b/bfd/elf-ifunc.c
@@ -131,8 +131,15 @@ _bfd_elf_allocate_ifunc_dyn_relocs (struct bfd_link_info *info,
the resolved function may be used. But in non-PIC executable,
the address of its .plt slot may be used. Pointer equality may
not work correctly. PIE or non-PLT reference should be used if
- pointer equality is required here. */
+ pointer equality is required here.
+
+ If STT_GNU_IFUNC symbol is defined in position-dependent executable,
+ backend should change it to the normal function and set its address
+ to its PLT entry which should be resolved by R_*_IRELATIVE at
+ run-time. All external references should be resolved to its PLT in
+ executable. */
if (!need_dynreloc
+ && !(bfd_link_pde (info) && h->def_regular)
&& (h->dynindx != -1
|| info->export_dynamic)
&& h->pointer_equality_needed)
diff --git a/bfd/elf32-i386.c b/bfd/elf32-i386.c
index c1c4ed0237..580a591816 100644
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -3805,6 +3805,8 @@ elf_i386_finish_dynamic_symbol (bfd *output_bfd,
sym->st_value = 0;
}
+ _bfd_x86_elf_link_fixup_ifunc_symbol (info, htab, h, sym);
+
/* Don't generate dynamic GOT relocation against undefined weak
symbol in executable. */
if (h->got.offset != (bfd_vma) -1
diff --git a/bfd/elf64-x86-64.c b/bfd/elf64-x86-64.c
index a0ebd2cf69..e5a4dbcd51 100644
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -4147,6 +4147,8 @@ elf_x86_64_finish_dynamic_symbol (bfd *output_bfd,
sym->st_value = 0;
}
+ _bfd_x86_elf_link_fixup_ifunc_symbol (info, htab, h, sym);
+
/* Don't generate dynamic GOT relocation against undefined weak
symbol in executable. */
if (h->got.offset != (bfd_vma) -1
diff --git a/bfd/elfxx-x86.c b/bfd/elfxx-x86.c
index 6c1f4c32fc..936b4c67d2 100644
--- a/bfd/elfxx-x86.c
+++ b/bfd/elfxx-x86.c
@@ -1730,6 +1730,52 @@ _bfd_x86_elf_fixup_symbol (struct bfd_link_info *info,
return TRUE;
}
+/* Change the STT_GNU_IFUNC symbol defined in position-dependent
+ executable into the normal function symbol and set its address
+ to its PLT entry, which should be resolved by R_*_IRELATIVE at
+ run-time. */
+
+void
+_bfd_x86_elf_link_fixup_ifunc_symbol (struct bfd_link_info *info,
+ struct elf_x86_link_hash_table *htab,
+ struct elf_link_hash_entry *h,
+ Elf_Internal_Sym *sym)
+{
+ if (bfd_link_pde (info)
+ && h->def_regular
+ && h->dynindx != -1
+ && h->plt.offset != (bfd_vma) -1
+ && h->type == STT_GNU_IFUNC
+ && h->pointer_equality_needed)
+ {
+ asection *plt_s;
+ bfd_vma plt_offset;
+ bfd *output_bfd = info->output_bfd;
+
+ if (htab->plt_second)
+ {
+ struct elf_x86_link_hash_entry *eh
+ = (struct elf_x86_link_hash_entry *) h;
+
+ plt_s = htab->plt_second;
+ plt_offset = eh->plt_second.offset;
+ }
+ else
+ {
+ plt_s = htab->elf.splt;
+ plt_offset = h->plt.offset;
+ }
+
+ sym->st_size = 0;
+ sym->st_info = ELF_ST_INFO (ELF_ST_BIND (sym->st_info), STT_FUNC);
+ sym->st_shndx
+ = _bfd_elf_section_from_bfd_section (output_bfd,
+ plt_s->output_section);
+ sym->st_value = (plt_s->output_section->vma
+ + plt_s->output_offset + plt_offset);
+ }
+}
+
/* Return TRUE if symbol should be hashed in the `.gnu.hash' section. */
bfd_boolean
diff --git a/bfd/elfxx-x86.h b/bfd/elfxx-x86.h
index efa835e104..8f67c23a9f 100644
--- a/bfd/elfxx-x86.h
+++ b/bfd/elfxx-x86.h
@@ -674,6 +674,10 @@ extern bfd_boolean _bfd_x86_elf_merge_gnu_properties
extern bfd * _bfd_x86_elf_link_setup_gnu_properties
(struct bfd_link_info *, struct elf_x86_init_table *);
+extern void _bfd_x86_elf_link_fixup_ifunc_symbol
+ (struct bfd_link_info *, struct elf_x86_link_hash_table *,
+ struct elf_link_hash_entry *, Elf_Internal_Sym *sym);
+
#define bfd_elf64_mkobject \
_bfd_x86_elf_mkobject
#define bfd_elf32_mkobject \
diff --git a/ld/testsuite/ld-ifunc/ifunc-9-i386.d b/ld/testsuite/ld-ifunc/ifunc-9-i386.d
new file mode 100644
index 0000000000..754ee2deca
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-9-i386.d
@@ -0,0 +1,9 @@
+#source: ifunc-9-x86.s
+#as: --32
+#ld: -m elf_i386 --export-dynamic
+#readelf: -r --wide
+#target: x86_64-*-* i?86-*-*
+
+Relocation section '.rel.plt' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_386_IRELATIVE[ ]*
diff --git a/ld/testsuite/ld-ifunc/ifunc-9-x86-64.d b/ld/testsuite/ld-ifunc/ifunc-9-x86-64.d
new file mode 100644
index 0000000000..85c3a69150
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/ifunc-9-x86-64.d
@@ -0,0 +1,9 @@
+#source: ifunc-9-x86.s
+#as: --64
+#ld: -melf_x86_64 --export-dynamic
+#readelf: -r --wide
+#target: x86_64-*-*
+
+Relocation section '.rela.plt' at .*
+[ ]+Offset[ ]+Info[ ]+Type[ ]+.*
+[0-9a-f]+[ ]+[0-9a-f]+[ ]+R_X86_64_IRELATIVE[ ]+[0-9a-f]*
diff --git a/ld/testsuite/ld-ifunc/ifunc-9-x86.d b/ld/testsuite/ld-ifunc/ifunc-9-x86.d
deleted file mode 100644
index 7390dd1fed..0000000000
--- a/ld/testsuite/ld-ifunc/ifunc-9-x86.d
+++ /dev/null
@@ -1,3 +0,0 @@
-#ld: --export-dynamic
-#error: .*dynamic STT_GNU_IFUNC symbol `foo' with pointer equality in `.*.o' can not be used when making an executable; recompile with -fPIE and relink with -pie
-#target: x86_64-*-* i?86-*-*
diff --git a/ld/testsuite/ld-ifunc/ifunc.exp b/ld/testsuite/ld-ifunc/ifunc.exp
index d9cc381e44..9f4aa73e7a 100644
--- a/ld/testsuite/ld-ifunc/ifunc.exp
+++ b/ld/testsuite/ld-ifunc/ifunc.exp
@@ -581,6 +581,76 @@ run_cc_link_tests [list \
{} \
"libpr18841cn.so" \
] \
+ [list \
+ "Build libpr23169a.so" \
+ "-shared" \
+ "-fPIC -O2 -g" \
+ { pr23169a.c } \
+ {} \
+ "libpr23169a.so" \
+ ] \
+ [list \
+ "Build libpr23169b.so" \
+ "-shared -Wl,-z,now" \
+ "-fPIC -O2 -g" \
+ { pr23169a.c } \
+ {} \
+ "libpr23169b.so" \
+ ] \
+ [list \
+ "Build pr23169a" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "$NOPIE_CFLAGS -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169a.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169a" \
+ ] \
+ [list \
+ "Build pr23169b" \
+ "-pie -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "-fPIE -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169c.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169b" \
+ ] \
+ [list \
+ "Build pr23169c" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "-fPIE -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169c.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169c" \
+ ] \
+ [list \
+ "Build pr23169d" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "$NOPIE_CFLAGS -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169a.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169d" \
+ ] \
+ [list \
+ "Build pr23169e" \
+ "-pie -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "-fPIE -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169c.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169e" \
+ ] \
+ [list \
+ "Build pr23169f" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "-fPIE -O2 -g" \
+ { pr23169b.c pr23169c.c } \
+ {{readelf {--dyn-syms} pr23169c.rd} \
+ {readelf {-r -W} pr23169b.rd}} \
+ "pr23169f" \
+ ] \
]
run_ld_link_exec_tests [list \
@@ -632,4 +702,58 @@ run_ld_link_exec_tests [list \
"pr18841cn" \
"pr18841.out" \
] \
+ [list \
+ "Run pr23169a" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169a" \
+ "pass.out" \
+ "$NOPIE_CFLAGS -O2 -g" \
+ ] \
+ [list \
+ "Run pr23169b" \
+ "-pie -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169b" \
+ "pass.out" \
+ "-fPIE -O2 -g" \
+ ] \
+ [list \
+ "Run pr23169c" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed tmpdir/libpr23169a.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169c" \
+ "pass.out" \
+ "-fPIE -O2 -g" \
+ ] \
+ [list \
+ "Run pr23169d" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169d" \
+ "pass.out" \
+ "$NOPIE_CFLAGS -O2 -g" \
+ ] \
+ [list \
+ "Run pr23169e" \
+ "-pie -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169e" \
+ "pass.out" \
+ "-fPIE -O2 -g" \
+ ] \
+ [list \
+ "Run pr23169f" \
+ "$NOPIE_LDFLAGS -Wl,--no-as-needed,-z,now tmpdir/libpr23169b.so" \
+ "" \
+ { pr23169b.c pr23169c.c } \
+ "pr23169f" \
+ "pass.out" \
+ "-fPIE -O2 -g" \
+ ] \
]
diff --git a/ld/testsuite/ld-ifunc/pr23169a.c b/ld/testsuite/ld-ifunc/pr23169a.c
new file mode 100644
index 0000000000..02bf220890
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169a.c
@@ -0,0 +1,9 @@
+int (*func_p) (void);
+extern int func (void);
+
+void
+foo (void)
+{
+ if (func_p != &func || func_p () != 0xbadbeef)
+ __builtin_abort ();
+}
diff --git a/ld/testsuite/ld-ifunc/pr23169a.rd b/ld/testsuite/ld-ifunc/pr23169a.rd
new file mode 100644
index 0000000000..ca04d8fc38
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169a.rd
@@ -0,0 +1,6 @@
+
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+#...
+ +[0-9]+: +[a-f0-9]+ +0 +FUNC +GLOBAL +DEFAULT +[0-9]+ +func
+#...
diff --git a/ld/testsuite/ld-ifunc/pr23169b.c b/ld/testsuite/ld-ifunc/pr23169b.c
new file mode 100644
index 0000000000..99790d4547
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169b.c
@@ -0,0 +1,23 @@
+#include <stdio.h>
+
+extern int (*func_p) (void);
+extern int func (void);
+extern void foo (void);
+
+
+void
+bar (void)
+{
+ if (func_p != &func || func_p () != 0xbadbeef)
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ func_p = &func;
+ foo ();
+ bar ();
+ printf ("PASS\n");
+ return 0;
+}
diff --git a/ld/testsuite/ld-ifunc/pr23169b.rd b/ld/testsuite/ld-ifunc/pr23169b.rd
new file mode 100644
index 0000000000..d57a79308b
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169b.rd
@@ -0,0 +1,3 @@
+#failif
+[a-f0-9]+ +[0-9a-f]+ +R_.*_* +[a-f0-9]+ +func(| \+ 0)
+...
diff --git a/ld/testsuite/ld-ifunc/pr23169c.c b/ld/testsuite/ld-ifunc/pr23169c.c
new file mode 100644
index 0000000000..b530108213
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169c.c
@@ -0,0 +1,13 @@
+static int
+ifunc (void)
+{
+ return 0xbadbeef;
+}
+
+void func(void) __attribute__((ifunc("resolve_func")));
+
+static void *
+resolve_func (void)
+{
+ return ifunc;
+}
diff --git a/ld/testsuite/ld-ifunc/pr23169c.rd b/ld/testsuite/ld-ifunc/pr23169c.rd
new file mode 100644
index 0000000000..1c4ba8ad4a
--- /dev/null
+++ b/ld/testsuite/ld-ifunc/pr23169c.rd
@@ -0,0 +1,6 @@
+
+Symbol table '\.dynsym' contains [0-9]+ entries:
+ +Num: +Value +Size Type +Bind +Vis +Ndx Name
+#...
+ +[0-9]+: +[a-f0-9]+ +[0-9]+ +IFUNC +GLOBAL +DEFAULT +[0-9]+ +func
+#...
--
2.17.0