[PATCH] D102044: [lld][WebAssembly] Disallow exporting of TLS symbols

[Derek Schuff via Phabricator via llvm-commits]

dschuff accepted this revision.
dschuff added inline comments.
This revision is now accepted and ready to land.


================
Comment at: lld/test/wasm/tls-export.s:9
+
+.section  .tdata.tls1,"",@
+.globl  tls1
----------------
What is it that actually designates the symbol as TLS? Is it just the section name?
edit: ok yeah i just read the rest of the code. That's surprising; I guess in the object file (and eventually in dylibs) there's a flag in the symbol table, right? or is it still just based on the section name? is ELF like this too, or do they have a section or symbol flag?


================
Comment at: lld/test/wasm/tls-export.s:16
+
+.section  .custom_section.target_features,"",@
+  .int8 3
----------------
Hm, you know, If we're going to be writing a lot of these asm tests with post-MVP features, maybe we actually do want to have an assembler directive for it at some point.


================
Comment at: lld/wasm/InputChunks.h:121
+  bool isTLS() {
+    return getName().startswith(".tdata") || getName().startswith(".tbss");
+  }
----------------
I guess there doesn't need to be a `.trodata` because if it's read-only it can always be shared across threads....


================
Comment at: lld/wasm/OutputSegment.h:35
 
+  bool isTLS() const { return name == ".tdata"; }
+
----------------
I guess this is asymmetric with input chunks because BSS doesn't end up in any kind of segment or descriptor in the output file (it's just implicitly-zeroed space)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102044