This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add new relocation types for TLS data symbols
ClosedPublic

Authored by sbc100 on Nov 11 2020, 9:18 AM.

Details

Summary

These relocations represent offsets from the __tls_base symbol.

Previously we were just using normal MEMORY_ADDR relocations and relying
on the linker to select a segment-offset rather and absolute value in
Symbol::getVirtualAddress(). Using an explicit relocation type allows
allow us to clearly distingish absolute from relative relocations based
on the relocation information alone.

One place this is usefull is being able to reject absolute relocation in
the PIC case, but still accept TLS relocations.

Diff Detail

Event Timeline

sbc100 created this revision.Nov 11 2020, 9:18 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 11 2020, 9:18 AM
sbc100 requested review of this revision.Nov 11 2020, 9:18 AM

Why are there separate versions of the new relocation for both i32 and sleb128? Do we use both of them?

lld/test/wasm/tls.s
10

Was the previous direct addressing relocation implicit in i32.const tls1 because it used a symbol name in place of a number literal?

lld/wasm/InputFiles.cpp
241

What about .tbss? Are there other possible thread-local section names? Same question down in scanRelocations and elsewhere.

llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
246 ↗(On Diff #304546)

What is this function used for?

Ah yes, I forgot about the I32 relocation type. I looks like we don't actually need that since you can't have thread local address in global data... great!

# cat test.c
_Thread_local int bar;
int* a = &bar;
$ gcc test.c
test.c:2:10: error: initializer element is not constant
    2 | int* a = &bar;
      |          ^
lld/test/wasm/tls.s
10

Yes. This is basically the bug I'm trying to fix here.

i32.const tls1 is a "normal" relocation against the symbol tls1. However, as you can see from the comments in the lld changes here we had special handling of "normal" relocations in the linker when the target symbol happened to be in a TLS area.

This new mode is more explicit and also allows us to catch bugs (e.g. @tls relocation against non-tls symbols and vice versa).

lld/wasm/InputFiles.cpp
241

We currently combine all tls data into a single .tdata section here in the linker. So there is always exactly one output TLS segment.

llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
246 ↗(On Diff #304546)

Good question :) Maybe I should revert this part.. I noticed that other platforms implement this but we were missing it. I'm guessing its for debugging of MIR or some thing like that?

sbc100 retitled this revision from [WebAssembly] Add new relocation types for TLS data symbols to [WebAssembly] Add new relocation type for TLS data symbols.Nov 12 2020, 11:19 PM
sbc100 edited the summary of this revision. (Show Details)
sbc100 updated this revision to Diff 305039.Nov 12 2020, 11:19 PM
sbc100 retitled this revision from [WebAssembly] Add new relocation type for TLS data symbols to [WebAssembly] Add new relocation types for TLS data symbols.
sbc100 edited the summary of this revision. (Show Details)
  • remove I32 version
tlively accepted this revision.Nov 12 2020, 11:46 PM
tlively added inline comments.
llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.cpp
246 ↗(On Diff #304546)

Yeah, I think that would be best until figure out what this is for and how we can test it.

This revision is now accepted and ready to land.Nov 12 2020, 11:46 PM
This revision was landed with ongoing or failed builds.Nov 13 2020, 8:00 AM
This revision was automatically updated to reflect the committed changes.