[RFC,1/2] Add IFUNC support for MIPS (v5)
Commit Message
Changes with respect to previous patch:
- Retain REL32 handling for STB_LOCAL symbols
- Added/modified comments
- Add bias to resolved function address if resolver is not yet relocated.
glibc suite cross-tested on mips32 little endian, summary:
324 FAIL
1537 PASS
9 XFAIL
All ifunc* tests are passing.
If this looks good, I invite comments on the prototype to be used for
the IFUNC resolver function (rfc 2 of 2)
ChangeLog:
* elf/elf.h
(R_MIPS_IRELATIVE): New relocation type.
(R_MIPS_NUM): Bump up to 129.
(DT_MIPS_GENERAL_GOTNO): New dynamic tags.
(DT_MIPS_NUM): Bump to 0x37.
* sysdeps/mips/dl-irel.h: New file.
(elf_ifunc_invoke): New function.
(elf_irel): Likewise.
* sysdeps/mips/dl-machine.h
Include new dl-irel.h
(ELF_MACHINE_BEFORE_RTLD_RELOC): Use DT_MIPS_GENERAL_GOTNO tag, if
present, to find the start of the normally relocated GOT.
(elf_machine_reloc): Add skip_ifunc to parameter.
Add case for R_MIPS_IRELATIVE. Modify REL32 to check for pre-emption
if symbol is IFUNC and then perform IFUNC indirection. Modify JUMP
relocation to perform IFUNC resolution if necessary.
(elf_machine_rel): Add skip_ifunc to call to elf_machine_reloc().
(elf_machine_rela):Add skip_ifunc to call to elf_machine_reloc().
(RESOLVE_GOTSYM): Add check for STT_GNU_IFUNC.
(elf_machine_got_rel): Add check for STT_GNU_IFUNC and IFUNC resolution
step. Use DT_MIPS_GENERAL_GOTNO tag, if present, to find the start
of the normally relocated GOT.
* sysdeps/mips/dl-trampoline.c
(__dl_runtime_resolve): Add check for STT_GNU_IFUNC.
* sysdeps/unix/sysv/linux/mips/ldsodefs.h
(VALID_ELF_ABIVERSION): Upped allowed version number to 4.
* sysdeps/unix/sysv/linux/mips/libc-abis
(IFUNC): New ABI compatibility level.
---
elf/elf.h | 7 +-
sysdeps/mips/dl-irel.h | 63 ++++++++++++++++
sysdeps/mips/dl-machine.h | 126 ++++++++++++++++++++++++-------
sysdeps/mips/dl-trampoline.c | 3 +
sysdeps/unix/sysv/linux/mips/ldsodefs.h | 2 +-
sysdeps/unix/sysv/linux/mips/libc-abis | 3 +
6 files changed, 173 insertions(+), 31 deletions(-)
create mode 100644 sysdeps/mips/dl-irel.h
Comments
On Thu, 3 Sep 2015, Faraz Shahbazker wrote:
> glibc suite cross-tested on mips32 little endian, summary:
> 324 FAIL
> 1537 PASS
> 9 XFAIL
> All ifunc* tests are passing.
You should have far fewer than 324 FAILs. You'll need to fix whatever the
problem is with your test environment, or whatever has regressed since
2.22 release, in order to get meaningful results (similar to those
described at <https://sourceware.org/glibc/wiki/Release/2.22>, so maybe on
the order of 10 FAILs, but not much more). These results don't give any
confidence in the patch.
Has the binutils patch yet been checked in? Until the binutils patch is
checked in and the ABI settled there doesn't seem much point in detailed
review of the glibc changes.
Joseph Myers <joseph@codesourcery.com> writes:
> On Thu, 3 Sep 2015, Faraz Shahbazker wrote:
>
> > glibc suite cross-tested on mips32 little endian, summary:
> > 324 FAIL
I have found native testing on debian to have this kind of failure rate
when libgcc_s.so and/or libstdc++.so.? can't be found. I have simply copied
them into the build tree in the past as I never figured out why things didn't
just work.
Matthew
Faraz Shahbazker <faraz.shahbazker@imgtec.com> writes:
> - /* This wouldn't work for a symbol imported from other
> - libraries for which there's no GOT entry, but MIPS
> - requires every symbol referenced in a dynamic
> - relocation to have a GOT entry in the primary GOT,
> - so we only get here for locally-defined symbols.
> - For section symbols, we should *NOT* be adding
> - sym->st_value (per the definition of the meaning of
> - S in reloc expressions in the ELF64 MIPS ABI),
> - since it should have already been added to
> - reloc_value by the linker, but older versions of
> - GNU ld didn't add it, and newer versions don't emit
> - useless relocations to section symbols any more, so
> - it is safe to keep on adding sym->st_value, even
> - though it's not ABI compliant. Some day we should
> - bite the bullet and stop doing this. */
> + if (ELFW(ST_BIND) (sym->st_info) == STB_LOCAL)
> + {
> + /* For section symbols, we should *NOT* be adding
> + sym->st_value (per the definition of the meaning of S
> + in reloc expressions in the ELF64 MIPS ABI), since it
> + should have already been added to reloc_value by the
> + linker, but older versions of GNU ld didn't add it, and
> + newer versions don't emit useless relocations to
> + section symbols any more, so it is safe to keep on
> + adding sym->st_value, even though it's not ABI
> + compliant. */
> #ifndef RTLD_BOOTSTRAP
> - if (map != &GL(dl_rtld_map))
> + if (map != &GL(dl_rtld_map))
> +#endif
> + reloc_value += sym->st_value + map->l_addr;
> + }
> +#ifndef RTLD_BOOTSTRAP
> + /* The original MIPS ABI required every global symbol used in
> + a relocation to be in the global GOT. We would then only
> + expect to get here for local symbols. This restriction is
> + removed for objects that use DT_MIPS_GENERAL_GOTNO, since
> + newer relocations and symbol types do not fit easily in the
> + original ABI scheme. Relocations against symbols below
> + DT_MIPS_GOTSYM bind in just the same way as relocations
> + against symbols in the global GOT; the only difference is
> + that we are not able to use the global GOT as a
> + directly-indexed lookup cache. Symbols below
> + DT_MIPS_GOTSYM might be in the general GOT region or might
> + not have a GOT entry at all. */
> + else if (__glibc_unlikely (map->l_info[DT_MIPS (GENERAL_GOTNO)]
> + == NULL))
> + {
> + const char *strtab;
> + strtab = (const void *) D_PTR (map, l_info[DT_STRTAB]);
> +
> + _dl_error_printf ("\
> +%s: Explicitly relocated symbol `%s' requires dynamic tag MIPS_GENERAL_GOTNO\n",
> + RTLD_PROGNAME, strtab + sym->st_name);
> + }
> + else
> + {
> + struct link_map *rmap = RESOLVE_MAP (&sym, version, r_type);
> + if ((ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
> + reloc_value = elf_ifunc_invoke (sym->st_value
> + + rmap->l_addr);
> + else
> + /* The behavior for section symbols described above
> + is now so firmly established that it is explicitly
> + adopted by objects with DT_MIPS_GLOBAL_GOTNO.
> + We therefore don't have a special case for
> + section symbols. */
> + reloc_value = sym->st_value + rmap->l_addr;
> + }
> #endif
> - reloc_value += sym->st_value + map->l_addr;
Don't shoot me :-) but I meant that the structure ought to be:
if (__glibc_unlikely (map->l_info[DT_MIPS (GENERAL_GOTNO)] == NULL))
{
if (ELFW(ST_BIND) (sym->st_info) == STB_LOCAL)
{
/* For section symbols, we should *NOT* be adding
...
}
else
{
const char *strtab;
...
}
}
else
{
struct link_map *rmap = RESOLVE_MAP (&sym, version, r_type);
...
}
I.e. in the DT_MIPS_GENERAL_GOTNO != NULL case we treat the relocation
exactly like a normal relocation on other targets. That just feels more
future proof to me, rather than than keeping the legacy special case
even for the new objects.
This also justifies the "The behavior for section symbols described above
is now so firmly established..." comment. With the structure in the patch
we'd never actually reach that point for section symbols so the comment
doesn't say much. With the revised structure we'd reach that point for
section symbols but not do anything special for them.
Aside from that shuffling around, this looks great to me.
Thanks,
Richard
> I have found native testing on debian to have this kind of failure rate
> when libgcc_s.so and/or libstdc++.so.? can't be found. I have simply copied
> them into the build tree in the past as I never figured out why things didn't
> just work.
Recent Debian-based systems use a new scheme for the library search paths
that has never been contributed upstream, so your libc build is not
configured compatibly with the filesystem layout of the system you are
using. A symlink to /lib/TUPLE/libgcc_s.so.1 in the build directory is the
easy way to work around this.
The long-term solution is that some folks from the Debian-based world
should work with us to get it so that there is some configure invocation
that sets up the search paths the way they want for their systems.
Joseph Myers <joseph@codesourcery.com> writes:
> On Thu, 3 Sep 2015, Faraz Shahbazker wrote:
>
>> glibc suite cross-tested on mips32 little endian, summary:
>> 324 FAIL
>> 1537 PASS
>> 9 XFAIL
>> All ifunc* tests are passing.
>
> You should have far fewer than 324 FAILs. You'll need to fix whatever the
> problem is with your test environment, or whatever has regressed since
> 2.22 release, in order to get meaningful results (similar to those
> described at <https://sourceware.org/glibc/wiki/Release/2.22>, so maybe on
> the order of 10 FAILs, but not much more). These results don't give any
> confidence in the patch.
>
> Has the binutils patch yet been checked in? Until the binutils patch is
> checked in and the ABI settled there doesn't seem much point in detailed
> review of the glibc changes.
This is my fault. I thought it would be much easier to review the
binutils patch after seeing how the dynamic linker would handle the
result. (And FWIW I did find it to be very useful.)
I can understand why we'd not want the glibc patch to go in ahead
of the binutils one, but I don't see why the binutils review has
to be completed before the glibc one starts. IMO settling the ABI
is much easier at the dynamic linker level rather than when reviewing
minutiae of bfd.
Thanks,
Richard
On 09/03/2015 03:09 PM, Joseph Myers wrote:
> On Thu, 3 Sep 2015, Faraz Shahbazker wrote:
>
>> glibc suite cross-tested on mips32 little endian, summary:
>> 324 FAIL
>> 1537 PASS
>> 9 XFAIL
>> All ifunc* tests are passing.
>
> You should have far fewer than 324 FAILs. You'll need to fix whatever the
> problem is with your test environment, or whatever has regressed since
> 2.22 release, in order to get meaningful results (similar to those
> described at <https://sourceware.org/glibc/wiki/Release/2.22>, so maybe on
> the order of 10 FAILs, but not much more). These results don't give any
> confidence in the patch.
I've managed to fix my test environment. I now get the same 6-7 failures listed in the release results. Will focus on the binutils patch for now, will post back a final version here when that is done. Hopefully, we'll get the ifunc prototype sorted out by then.
Thanks,
Faraz Shahbazker
@@ -1653,8 +1653,9 @@ typedef struct
#define R_MIPS_GLOB_DAT 51
#define R_MIPS_COPY 126
#define R_MIPS_JUMP_SLOT 127
+#define R_MIPS_IRELATIVE 128
/* Keep this the last entry. */
-#define R_MIPS_NUM 128
+#define R_MIPS_NUM 129
/* Legal values for p_type field of Elf32_Phdr. */
@@ -1731,7 +1732,9 @@ typedef struct
in a PIE as it stores a relative offset from the address of the tag
rather than an absolute address. */
#define DT_MIPS_RLD_MAP_REL 0x70000035
-#define DT_MIPS_NUM 0x36
+ /* Number of explicitly relocated GOT entries */
+#define DT_MIPS_GENERAL_GOTNO 0x70000036
+#define DT_MIPS_NUM 0x37
/* Legal values for DT_MIPS_FLAGS Elf32_Dyn entry. */
new file mode 100644
@@ -0,0 +1,63 @@
+/* Machine-dependent ELF indirect relocation inline functions.
+ MIPS version.
+ Copyright (C) 2015 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library. If not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _DL_IREL_H
+#define _DL_IREL_H
+
+#include <stdio.h>
+#include <unistd.h>
+#include <sgidefs.h>
+#include <link.h>
+#include <elf.h>
+#include <ldsodefs.h>
+
+#define ELF_MACHINE_IREL 1
+
+static inline ElfW(Addr)
+__attribute ((always_inline))
+elf_ifunc_invoke (ElfW(Addr) addr)
+{
+ /* Print some debugging info if wanted. */
+ if (__glibc_unlikely (GLRO(dl_debug_mask) & DL_DEBUG_SYMBOLS))
+ {
+ ElfW(Addr) t_addr =
+ ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
+ GLRO(dl_debug_printf) ("In elf_ifunc_invoke(0x%lx), return(0x%lx)\n",
+ (unsigned long int)addr,
+ (unsigned long int)t_addr);
+ }
+
+ return ((ElfW(Addr) (*) (unsigned long int)) (addr)) (GLRO(dl_hwcap));
+}
+
+/* Allow either R_MIPS_RELATIVE or the nop R_MIPS_NONE. */
+static inline void
+__attribute ((always_inline))
+elf_irel (const ElfW(Rel) *reloc)
+{
+ ElfW(Addr) *const reloc_addr = (void *) reloc->r_offset;
+ const unsigned long int r_type = ELFW(R_TYPE) (reloc->r_info);
+
+ if (__glibc_likely (r_type == R_MIPS_IRELATIVE))
+ *reloc_addr = elf_ifunc_invoke (*reloc_addr);
+ else if (r_type)
+ __libc_fatal ("unexpected reloc type in static binary");
+}
+
+#endif /* dl-irel.h */
@@ -33,6 +33,7 @@
#include <sysdep.h>
#include <sys/asm.h>
#include <dl-tls.h>
+#include <dl-irel.h>
/* The offset of gp from GOT might be system-dependent. It's set by
ld. The same value is also */
@@ -200,10 +201,13 @@ do { \
if (__builtin_expect (map->l_addr == 0, 1)) \
break; \
\
- /* got[0] is reserved. got[1] is also reserved for the dynamic object \
- generated by gnu ld. Skip these reserved entries from \
- relocation. */ \
- i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1; \
+ if (map->l_info[DT_MIPS (GENERAL_GOTNO)] != NULL) \
+ i = map->l_info[DT_MIPS (GENERAL_GOTNO)]->d_un.d_val; \
+ else \
+ /* got[0] is reserved. got[1] is also reserved for the dynamic \
+ object generated by gnu ld. Skip these reserved entries from \
+ relocation. */ \
+ i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1; \
n = map->l_info[DT_MIPS (LOCAL_GOTNO)]->d_un.d_val; \
\
/* Add the run-time displacement to all local got entries. */ \
@@ -493,7 +497,8 @@ auto inline void
__attribute__ ((always_inline))
elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
const ElfW(Sym) *sym, const struct r_found_version *version,
- void *reloc_addr, ElfW(Addr) r_addend, int inplace_p)
+ void *reloc_addr, ElfW(Addr) r_addend, int inplace_p,
+ int skip_ifunc)
{
const unsigned long int r_type = ELFW(R_TYPE) (r_info);
ElfW(Addr) *addr_field = (ElfW(Addr) *) reloc_addr;
@@ -579,25 +584,60 @@ elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
if ((ElfW(Word))symidx < gotsym)
{
- /* This wouldn't work for a symbol imported from other
- libraries for which there's no GOT entry, but MIPS
- requires every symbol referenced in a dynamic
- relocation to have a GOT entry in the primary GOT,
- so we only get here for locally-defined symbols.
- For section symbols, we should *NOT* be adding
- sym->st_value (per the definition of the meaning of
- S in reloc expressions in the ELF64 MIPS ABI),
- since it should have already been added to
- reloc_value by the linker, but older versions of
- GNU ld didn't add it, and newer versions don't emit
- useless relocations to section symbols any more, so
- it is safe to keep on adding sym->st_value, even
- though it's not ABI compliant. Some day we should
- bite the bullet and stop doing this. */
+ if (ELFW(ST_BIND) (sym->st_info) == STB_LOCAL)
+ {
+ /* For section symbols, we should *NOT* be adding
+ sym->st_value (per the definition of the meaning of S
+ in reloc expressions in the ELF64 MIPS ABI), since it
+ should have already been added to reloc_value by the
+ linker, but older versions of GNU ld didn't add it, and
+ newer versions don't emit useless relocations to
+ section symbols any more, so it is safe to keep on
+ adding sym->st_value, even though it's not ABI
+ compliant. */
#ifndef RTLD_BOOTSTRAP
- if (map != &GL(dl_rtld_map))
+ if (map != &GL(dl_rtld_map))
+#endif
+ reloc_value += sym->st_value + map->l_addr;
+ }
+#ifndef RTLD_BOOTSTRAP
+ /* The original MIPS ABI required every global symbol used in
+ a relocation to be in the global GOT. We would then only
+ expect to get here for local symbols. This restriction is
+ removed for objects that use DT_MIPS_GENERAL_GOTNO, since
+ newer relocations and symbol types do not fit easily in the
+ original ABI scheme. Relocations against symbols below
+ DT_MIPS_GOTSYM bind in just the same way as relocations
+ against symbols in the global GOT; the only difference is
+ that we are not able to use the global GOT as a
+ directly-indexed lookup cache. Symbols below
+ DT_MIPS_GOTSYM might be in the general GOT region or might
+ not have a GOT entry at all. */
+ else if (__glibc_unlikely (map->l_info[DT_MIPS (GENERAL_GOTNO)]
+ == NULL))
+ {
+ const char *strtab;
+ strtab = (const void *) D_PTR (map, l_info[DT_STRTAB]);
+
+ _dl_error_printf ("\
+%s: Explicitly relocated symbol `%s' requires dynamic tag MIPS_GENERAL_GOTNO\n",
+ RTLD_PROGNAME, strtab + sym->st_name);
+ }
+ else
+ {
+ struct link_map *rmap = RESOLVE_MAP (&sym, version, r_type);
+ if ((ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
+ reloc_value = elf_ifunc_invoke (sym->st_value
+ + rmap->l_addr);
+ else
+ /* The behavior for section symbols described above
+ is now so firmly established that it is explicitly
+ adopted by objects with DT_MIPS_GLOBAL_GOTNO.
+ We therefore don't have a special case for
+ section symbols. */
+ reloc_value = sym->st_value + rmap->l_addr;
+ }
#endif
- reloc_value += sym->st_value + map->l_addr;
}
else
{
@@ -663,6 +703,9 @@ elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
sym_map = RESOLVE_MAP (&sym, version, r_type);
value = sym_map == NULL ? 0 : sym_map->l_addr + sym->st_value;
+ if ((ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC) && !skip_ifunc)
+ value = elf_ifunc_invoke (value);
+
*addr_field = value;
break;
@@ -698,6 +741,17 @@ elf_machine_reloc (struct link_map *map, ElfW(Addr) r_info,
break;
}
+ case R_MIPS_IRELATIVE:
+#ifndef RTLD_BOOTSTRAP
+ /* The resolver routine is the symbol referenced by this relocation.
+ To get the address of the function to use at runtime, the resolver
+ routine is called and its return value is the address of the target
+ functon which is final relocation value. */
+ if (!skip_ifunc)
+ *addr_field = elf_ifunc_invoke (map->l_addr + *addr_field);
+#endif
+ break;
+
#if _MIPS_SIM == _ABI64
case R_MIPS_64:
/* For full compliance with the ELF64 ABI, one must precede the
@@ -727,7 +781,8 @@ elf_machine_rel (struct link_map *map, const ElfW(Rel) *reloc,
const ElfW(Sym) *sym, const struct r_found_version *version,
void *const reloc_addr, int skip_ifunc)
{
- elf_machine_reloc (map, reloc->r_info, sym, version, reloc_addr, 0, 1);
+ elf_machine_reloc (map, reloc->r_info, sym, version, reloc_addr, 0, 1,
+ skip_ifunc);
}
auto inline void
@@ -768,7 +823,7 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
void *const reloc_addr, int skip_ifunc)
{
elf_machine_reloc (map, reloc->r_info, sym, version, reloc_addr,
- reloc->r_addend, 0);
+ reloc->r_addend, 0, skip_ifunc);
}
auto inline void
@@ -795,8 +850,20 @@ elf_machine_got_rel (struct link_map *map, int lazy)
const struct r_found_version *version __attribute__ ((unused)) \
= vernum ? &map->l_versions[vernum[sym_index] & 0x7fff] : NULL; \
struct link_map *sym_map; \
+ ElfW(Addr) value = 0; \
sym_map = RESOLVE_MAP (&ref, version, reloc); \
- ref ? sym_map->l_addr + ref->st_value : 0; \
+ if (__glibc_likely (ref != NULL)) \
+ { \
+ value = sym_map->l_addr + ref->st_value; \
+ if (__glibc_unlikely (ELFW(ST_TYPE) (ref->st_info) \
+ == STT_GNU_IFUNC)) \
+ { \
+ value = elf_ifunc_invoke (value); \
+ if (!sym_map->l_relocated) \
+ value += sym_map->l_addr; \
+ } \
+ } \
+ value; \
})
if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL)
@@ -810,9 +877,12 @@ elf_machine_got_rel (struct link_map *map, int lazy)
/* The dynamic linker's local got entries have already been relocated. */
if (map != &GL(dl_rtld_map))
{
- /* got[0] is reserved. got[1] is also reserved for the dynamic object
- generated by gnu ld. Skip these reserved entries from relocation. */
- i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1;
+ if (map->l_info[DT_MIPS (GENERAL_GOTNO)] != NULL)
+ i = map->l_info[DT_MIPS (GENERAL_GOTNO)]->d_un.d_val;
+ else
+ /* got[0] is reserved. got[1] is also reserved for the dynamic object
+ generated by gnu ld. Skip these reserved entries from relocation. */
+ i = (got[1] & ELF_MIPS_GNU_GOT1_MASK)? 2 : 1;
/* Add the run-time displacement to all local got entries if
needed. */
@@ -193,6 +193,9 @@ __dl_runtime_resolve (ElfW(Word) sym_index,
/* Currently value contains the base load address of the object
that defines sym. Now add in the symbol offset. */
value = (sym ? sym_map->l_addr + sym->st_value : 0);
+ if (sym != NULL
+ && __glibc_unlikely (ELFW(ST_TYPE) (sym->st_info) == STT_GNU_IFUNC))
+ value = elf_ifunc_invoke (DL_FIXUP_VALUE_ADDR (value));
}
else
/* We already found the symbol. The module (and therefore its load
@@ -34,7 +34,7 @@ extern void _dl_static_init (struct link_map *map);
#undef VALID_ELF_ABIVERSION
#define VALID_ELF_ABIVERSION(osabi,ver) \
(ver == 0 \
- || (osabi == ELFOSABI_SYSV && ver < 4) \
+ || (osabi == ELFOSABI_SYSV && ver < 5) \
|| (osabi == ELFOSABI_GNU && ver < LIBC_ABI_MAX))
#endif /* ldsodefs.h */
@@ -14,3 +14,6 @@ UNIQUE
#
# MIPS O32 FP64
MIPS_O32_FP64 mips*-*-linux*
+#
+# MIPS IFUNC
+IFUNC mips*-*-linux*