Bug 25694 - R_RISCV_TPREL_HI20 relocations cause riscv64 to add TEXTREL bit on executables
Summary: R_RISCV_TPREL_HI20 relocations cause riscv64 to add TEXTREL bit on executables
Status: UNCONFIRMED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.34
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-03-18 22:28 UTC by Sergei Trofimovich
Modified: 2020-04-05 04:15 UTC (History)
4 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: riscv64-unknown-linux-gnu
Build: x86_64-pc-linux-gnu
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Sergei Trofimovich 2020-03-18 22:28:27 UTC
Initial bug is reported as a https://bugs.gentoo.org/713082 where gdb-9.1 was observed to be complied with TEXTRELs. Here is the minimal reproducer:

// $ cat a.cc
static int a () { return 1; }

static thread_local int (*tlf) () = &a;

int main(void) { return tlf(); }
[sf] /tmp/z:cat a.cc
static int a () { return 1; }

static thread_local int (*tlf) () = &a;

int main(void) { return tlf(); }

$ riscv64-unknown-linux-gnu-g++ a.cc -o a -Wl,-z,text -pie -fPIE
/usr/libexec/gcc/riscv64-unknown-linux-gnu/ld: read-only segment has dynamic relocations
collect2: error: ld returned 1 exit status

$ riscv64-unknown-linux-gnu-g++ -S a.cc -o a.S -Wl,-z,text -pie -fPIE
$ cat a.S
...
main:
...
	lui	a5,%tprel_hi(_ZL3tlf)
	add	a5,a5,tp,%tprel_add(_ZL3tlf)
	ld	a5,%tprel_lo(_ZL3tlf)(a5)
	jalr	a5
...


I assume it's a binutils deficiency as I see no relocations in read-only sections:

$ riscv64-unknown-linux-gnu-g++ a.cc -o a -pie -fPIE
/usr/libexec/gcc/riscv64-unknown-linux-gnu/ld: warning: creating a DT_TEXTREL in object
$ LANG=C objdump -D -R a | egrep 'R_RISC|section ' | egrep -A2 'section '
Disassembly of section .interp:
Disassembly of section .note.ABI-tag:
Disassembly of section .gnu.hash:
Disassembly of section .dynsym:
Disassembly of section .dynstr:
Disassembly of section .gnu.version:
Disassembly of section .gnu.version_r:
Disassembly of section .rela.dyn:
Disassembly of section .rela.plt:
Disassembly of section .plt:
Disassembly of section .text:
Disassembly of section .rodata:
Disassembly of section .eh_frame_hdr:
Disassembly of section .eh_frame:
Disassembly of section .tdata:
			1da0: R_RISCV_RELATIVE	*ABS*+0x72a
Disassembly of section .preinit_array:
			1da8: R_RISCV_RELATIVE	*ABS*+0x68e
Disassembly of section .init_array:
			1db0: R_RISCV_RELATIVE	*ABS*+0x728
Disassembly of section .fini_array:
			1db8: R_RISCV_RELATIVE	*ABS*+0x6ee
Disassembly of section .dynamic:
Disassembly of section .data:
			2000: R_RISCV_RELATIVE	*ABS*+0x2000
Disassembly of section .got:
			2018: R_RISCV_JUMP_SLOT	__libc_start_main@GLIBC_2.27
			2020: R_RISCV_JUMP_SLOT	__stack_chk_fail@GLIBC_2.27
--
Disassembly of section .bss:
Disassembly of section .comment:
			0: R_RISCV_NONE	*ABS*


riscv_elf_check_relocs() is probably too conservative at attributing thread_local relocations in PIEs as TEXTRELs.
Comment 1 Jim Wilson 2020-03-19 17:01:58 UTC
I believe this is the same as bug 22263, which added a testcase, so we already have a ld testsuite failure for it.  The problem also shows up in openembedded builds.
Comment 2 Sergei Trofimovich 2020-03-19 21:55:44 UTC
(In reply to Jim Wilson from comment #1)
> I believe this is the same as bug 22263, which added a testcase, so we
> already have a ld testsuite failure for it.  The problem also shows up in
> openembedded builds.

Yeah, looks like it's exactly it!

Don't know if it should be duped or fixed in this bug. That bug accumulated a lot of fixes which are nice to look at how to fix things but make it hard to track what targets were not fixed yet.
Comment 3 Fangrui Song 2020-04-05 04:15:19 UTC
Instead of --warn-shared-textrel, we can make bfd_link_info::error_textrel default to 1 and delete warn_shared_textrel.

The bi-state approach is used by LLD: either -z notext or -z text. There is no state where DT_TEXTREL/DF_TEXTREL is added on demand. The diagnostic should remind the user that -z notext may be needed.