[PATCH] RISC-V: Fix TLS and --gc-sections conflict.

[Jim Wilson]

This fixes a problem reported as a gcc bug.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86770
The libstdc++ testsuite fails when linking with --gc-sections, generating an
error that says we have a reference to a discarded section .tdata.dyn.

The problem here is that .tdata.dyn is a linker created section for copy
relocs, that doesn't get populated until after sections are garbage collected.
So we have to mark it to keep it from being deleted, and a good way to do that
is to add the SEC_LINKER_CREATED flag. I didn't see any bad effects from this.
It gets merged into .tdata, which still gets optimized away if empty at the
end.

This was tested with cross binutils and gcc builds and checks, with the gcc
testing hacked to force use of --gc-sections. The patch fixes about 59
libstdc++ failures.

Committed.

Jim

bfd/
* elfnn-riscv.c (riscv_elf_create_dynamic_sections): For .tdata.dyn,
add SEC_LINKER_CREATED flag.
---
bfd/elfnn-riscv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bfd/elfnn-riscv.c b/bfd/elfnn-riscv.c
index 934704a87e..4be3eceda8 100644
--- a/bfd/elfnn-riscv.c
+++ b/bfd/elfnn-riscv.c
@@ -353,7 +353,8 @@ riscv_elf_create_dynamic_sections (bfd *dynobj,
{
htab->sdyntdata =
bfd_make_section_anyway_with_flags (dynobj, ".tdata.dyn",
- SEC_ALLOC | SEC_THREAD_LOCAL);
+ (SEC_ALLOC | SEC_THREAD_LOCAL
+ | SEC_LINKER_CREATED));
}

if (!htab->elf.splt || !htab->elf.srelplt || !htab->elf.sdynbss

[Andreas Schwab]

Post by Jim Wilson
It gets merged into .tdata, which still gets optimized away if empty at the
end.

Does it? I don't see that happening. The linker now always generates a
.tdata section, moreover with the wrong type NOBITS if empty.

Andreas.

[Jim Wilson]

Post by Andreas Schwab

Post by Jim Wilson
It gets merged into .tdata, which still gets optimized away if empty at the
end.

Does it? I don't see that happening. The linker now always generates a
.tdata section, moreover with the wrong type NOBITS if empty.

Apparently not, as I can see it in a testcase now. Not sure where I
made the mistake. But does this cause any actual problems? I didn't
see any problems with my gcc testsuite run. It is target independent
code that is setting the .tdata section to NOBITS when empty. So that
part doesn't seem to be a problem with the RISC-V code. Is there some
reason why this is wrong? I do see that we have an empty PT_TLS
segment. Maybe there is a performance issue here with doing extra
unnecessary work at load time?

As far as I know, RISC-V is the only target with copy relocs against
thread data which is the ultimate cause of this problem. This means
tdata sections may not work exactly the same as other targets, and we
might need changes to target independent code in bfd to improve this.
But I don't think that my earlier patch is wrong, as it is fixing gcc
testsuite failures with --gc-sections. I probably need to spend some
time studying bfd and see if I can find a way to delete an empty tdata
section when there are no copy relocs. Maybe I can strip the
tdata.dyn section in elfnn-riscv.c when it is zero size. Or maybe I
can change where it is created, so that it is only created when
non-zero. I will take a look at this.

Still, it would be nice to know exactly why this is wrong.

Jim

[Andreas Schwab]

Post by Jim Wilson

Post by Andreas Schwab

Post by Jim Wilson
It gets merged into .tdata, which still gets optimized away if empty at the
end.

Does it? I don't see that happening. The linker now always generates a
.tdata section, moreover with the wrong type NOBITS if empty.

Apparently not, as I can see it in a testcase now. Not sure where I
made the mistake. But does this cause any actual problems? I didn't
see any problems with my gcc testsuite run.

elflint is complaining, and that breaks the elfutils testsuite.

Andreas.

[Jim Wilson]

Post by Andreas Schwab

Post by Jim Wilson
Apparently not, as I can see it in a testcase now. Not sure where I
made the mistake. But does this cause any actual problems? I didn't
see any problems with my gcc testsuite run.

elflint is complaining, and that breaks the elfutils testsuite.

I checked in a fix. It turned out to be pretty simple, and looks
oddly familiar. I think I might have accidentally checked in the
wrong version of the patch before.
https://sourceware.org/ml/binutils/2018-10/msg00100.html

I didn't check against elfutils, but for a simple hello world program
I get an empty .tdata section before the patch, and none after the
patch.

Jim