diff mbox

Improve IFUNC check

Message ID 20200205060922.5pbgm5hfj2r6vrl2@google.com
State Superseded
Headers

Commit Message

Fangrui Song Feb. 5, 2020, 6:09 a.m. UTC 
  GNU ld's RISCV port does not support IFUNC. ld -no-pie produces no
relocation and the test passed incorrectly. Be more rigid by testing
IRELATIVE explicitly.
---
 configure    | 2 +-
 configure.ac | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Aurelien Jarno Feb. 6, 2020, 12:24 p.m. UTC | #1 
Hi,

On 2020-02-04 22:09, Fangrui Song wrote:
> GNU ld's RISCV port does not support IFUNC. ld -no-pie produces no
> relocation and the test passed incorrectly. Be more rigid by testing
> IRELATIVE explicitly.

Thanks for your patch and sorry for the time to answer, I was testing it
on many architectures. First of all I confirm it fixes the issue when
building glibc with PIE on mips* and riscv64. I also confirm it
introduces no regression on aarch64, armv5, armv7, hppa, m68k, powerpc,
ppc64, ppc64le, s390x and x86_64.

However it wrongly detects that IFUNC is not supported on sparc64 when
PIE is not in use. Here is the output of of readelf for the non-PIE
case:

| Relocation section '.rela.dyn' at offset 0x110 contains 1 entry:
|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
| 000000200280  0000000000f8 R_SPARC_JMP_IREL                     100128

And for the PIE case:

| Relocation section '.rela.dyn' at offset 0x290 contains 1 entry:
|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
| 0000001021a0  0000000000f9 R_SPARC_IRELATIVE                    2c0
|
| Relocation section '.rela.plt' at offset 0x2a8 contains 1 entry:
|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
| 000000102180  0000000000f8 R_SPARC_JMP_IREL                     2c0

Looks like you might want to slightly adjust the regex.

Also note that this is BZ#25506, to add to the commit message.
Fangrui Song Feb. 6, 2020, 6:53 p.m. UTC | #2 
On 2020-02-06, Aurelien Jarno wrote:
>Hi,
>
>On 2020-02-04 22:09, Fangrui Song wrote:
>> GNU ld's RISCV port does not support IFUNC. ld -no-pie produces no
>> relocation and the test passed incorrectly. Be more rigid by testing
>> IRELATIVE explicitly.
>
>Thanks for your patch and sorry for the time to answer, I was testing it
>on many architectures. First of all I confirm it fixes the issue when
>building glibc with PIE on mips* and riscv64. I also confirm it
>introduces no regression on aarch64, armv5, armv7, hppa, m68k, powerpc,
>ppc64, ppc64le, s390x and x86_64.
>
>However it wrongly detects that IFUNC is not supported on sparc64 when
>PIE is not in use. Here is the output of of readelf for the non-PIE
>case:
>
>| Relocation section '.rela.dyn' at offset 0x110 contains 1 entry:
>|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
>| 000000200280  0000000000f8 R_SPARC_JMP_IREL                     100128

sparc64 -no-pie does not produce an R_SPARC_IRELATIVE. This looks weird.
All other archs emit an R_*_IRELATIVE.

Can you dump the assembly (including .text and .[i]plt)?
Or can someone with sparc64 experience answer why sparc64 is different
here?

As my lld R_RISCV_IRELATIVE patch (https://reviews.llvm.org/D74022) shows

   # DIS64:      _start:
   # DIS64-NEXT:     1264: auipc a0, 0
   # DIS64-NEXT:           addi a0, a0, 12
   # DIS64:      Disassembly of section .iplt:
   # DIS64:      00001270 func:
   ## 64-bit: &.got.plt[func]-. = 0x3370-0x1270 = 4096*2+256
   # DIS64-NEXT:          auipc t3, 2
   # DIS64-NEXT:          ld t3, 256(t3)
   # DIS64-NEXT:          jalr t1, t3
   # DIS64-NEXT:          nop

The idea is to create an IPLT entry and bind absolute/PC-relative relocations to that address.
The IPLT entry needs a .got.plt slot, which is relocated by an R_*_IRELATIVE at runtime.
It is R_*_IRELATIVE because no symbol index is needed.


R_SPARC_JMP_IREL looks strange. I hope someone can explain.

>And for the PIE case:
>
>| Relocation section '.rela.dyn' at offset 0x290 contains 1 entry:
>|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
>| 0000001021a0  0000000000f9 R_SPARC_IRELATIVE                    2c0
>|
>| Relocation section '.rela.plt' at offset 0x2a8 contains 1 entry:
>|   Offset          Info           Type           Sym. Value    Sym. Name + Addend
>| 000000102180  0000000000f8 R_SPARC_JMP_IREL                     2c0
>
>Looks like you might want to slightly adjust the regex.
>
>Also note that this is BZ#25506, to add to the commit message.
>
>-- 
>Aurelien Jarno                          GPG: 4096R/1DDD8C9B
>aurelien@aurel32.net                 http://www.aurel32.net
Aurelien Jarno Feb. 8, 2020, 9:37 a.m. UTC | #3 
On 2020-02-06 10:53, Fangrui Song wrote:
> On 2020-02-06, Aurelien Jarno wrote:
> > Hi,
> > 
> > On 2020-02-04 22:09, Fangrui Song wrote:
> > > GNU ld's RISCV port does not support IFUNC. ld -no-pie produces no
> > > relocation and the test passed incorrectly. Be more rigid by testing
> > > IRELATIVE explicitly.
> > 
> > Thanks for your patch and sorry for the time to answer, I was testing it
> > on many architectures. First of all I confirm it fixes the issue when
> > building glibc with PIE on mips* and riscv64. I also confirm it
> > introduces no regression on aarch64, armv5, armv7, hppa, m68k, powerpc,
> > ppc64, ppc64le, s390x and x86_64.
> > 
> > However it wrongly detects that IFUNC is not supported on sparc64 when
> > PIE is not in use. Here is the output of of readelf for the non-PIE
> > case:
> > 
> > | Relocation section '.rela.dyn' at offset 0x110 contains 1 entry:
> > |   Offset          Info           Type           Sym. Value    Sym. Name + Addend
> > | 000000200280  0000000000f8 R_SPARC_JMP_IREL                     100128
> 
> sparc64 -no-pie does not produce an R_SPARC_IRELATIVE. This looks weird.
> All other archs emit an R_*_IRELATIVE.
> 
> Can you dump the assembly (including .text and .[i]plt)?
> Or can someone with sparc64 experience answer why sparc64 is different
> here?

Here is the dump of the .iplt section:
Disassembly of section .iplt:

0000000000200200 <.iplt>:
        ...
  200280:       03 00 00 80     sethi  %hi(0x20000), %g1
  200284:       30 6f ff e7     b,a   %xcc, 200220 <__start+0x1000f8>
  200288:       01 00 00 00     nop 
  20028c:       01 00 00 00     nop 
  200290:       01 00 00 00     nop 
  200294:       01 00 00 00     nop 
  200298:       01 00 00 00     nop 
  20029c:       01 00 00 00     nop

There is no .text nor .plt section.
diff mbox

Patch

diff --git a/configure b/configure
index b959d2d988..0107f0dec5 100755
--- a/configure
+++ b/configure
@@ -4035,7 +4035,7 @@  if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS \
 	    -o conftest conftest.S 1>&5 2>&5; then
   # Do a link to see if the backend supports IFUNC relocs.
   $READELF -r conftest 1>&5
-  LC_ALL=C $READELF -r conftest | grep 'no relocations' >/dev/null || {
+  LC_ALL=C $READELF -Wr conftest | grep -q IRELATIVE && {
     libc_cv_ld_gnu_indirect_function=yes
   }
 fi
diff --git a/configure.ac b/configure.ac
index 49b900c1ed..1fbaf65951 100644
--- a/configure.ac
+++ b/configure.ac
@@ -649,7 +649,7 @@  if ${CC-cc} $CFLAGS $CPPFLAGS $LDFLAGS \
 	    -o conftest conftest.S 1>&AS_MESSAGE_LOG_FD 2>&AS_MESSAGE_LOG_FD; then
   # Do a link to see if the backend supports IFUNC relocs.
   $READELF -r conftest 1>&AS_MESSAGE_LOG_FD
-  LC_ALL=C $READELF -r conftest | grep 'no relocations' >/dev/null || {
+  LC_ALL=C $READELF -Wr conftest | grep -q IRELATIVE && {
     libc_cv_ld_gnu_indirect_function=yes
   }
 fi