[PATCH] D143020: [lld][WebAssembly] Apply relocations to TLS data

Tue Jan 31 15:50:15 PST 2023

dschuff added inline comments.

================
Comment at: lld/wasm/Symbols.cpp:312
   if (isTLS() && config->sharedMemory)
-    return getOutputSegmentOffset() + value;
+    return getOutputSegmentOffset();
   if (segment)
----------------
sbc100 wrote:
> This is perhaps a concerning change.  The original line here was added without any test changes in: https://reviews.llvm.org/D112831.  Its seems to lack test coverage although the new test I've add here adds coverage.
> 
> I also ran the entire emscripten test suite with this change to ensure this change doesn't break anything.
oh yeah I was going to ask, is that value getting moved to somewhere else. So this is used in exactly the case your new test covers, right? I guess it's no surprise that it was wrong, if the use case was also wrong in a different way, and nobody noticed before.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143020/new/

https://reviews.llvm.org/D143020