[PATCH] D145626: [WebAssembly] Support debug info for TLS + global in PIC mode

[Heejin Ahn via Phabricator via llvm-commits]

aheejin marked an inline comment as done.
aheejin added a comment.

In D145626#4181966 <https://reviews.llvm.org/D145626#4181966>, @dschuff wrote:

> In D145626#4179867 <https://reviews.llvm.org/D145626#4179867>, @aheejin wrote:
>
>> Only debug info won't work. So the devtools DWARF extension (https://chrome.google.com/webstore/detail/cc%20%20-devtools-support-dwa/pdcpmagijalfljmkmjngeonclgbbannb) will display `undefined` or some garbage value for TLS variables in dynamic linking. But that's the status quo for them anyway.
>
> Just so I'm clear, this is TLS + dynamic linking + split-dwarf, right?

Yes.

> Since the test does actually run that case, maybe it makes sense to put a comment in there about what the output should look like once that case is fixed?

That was tricky part when adding the test case. You can see that I put `{{[0-9]+}}` in place of all global indices. Internally they would contain a relocation symbol that's to be resolved in linker, but at this object file stage they don't mean much; I think they just mean the order of symbols added. So, when you don't use split dwarf, when you use dylink + TLS, because source code adds TLS first, `__tls_base` is shown as 0 and `__memory_base` is shown as 1. And if you use dylink witout TLS, `__memory_base` is shown as 0 instead. So these are not final indices. On the other hand, in case of split dwarf, the indices are fixed, because we hardcoded them, which can be incorrect in the case of dylink + TLS + split dwarf.

If we want to test final indices, we should move this test to lld. I actually considered that, but didn't end up doing it because the change itself doesn't involve anything in the linker. Do you think it's better? Or we leave this as is and only test indices in case of split dwarf? This is not really easy either because we don't know what the real values of those indices in split dwarf should be at this point in the object file.



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:496
+    // For now we hardcode the indices in the callsites. Global indices are not
+    // fixed, but in practice a few are fixed; for example, __stack_overflow is
+    // always index 0.
----------------
dschuff wrote:
> 
lol


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145626