[PATCH] D143020: [lld][WebAssembly] Apply relocations to TLS data
Derek Schuff via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list