[PATCH] D111345: [WebAssembly] Add import info to `dylink` section of shared libraries

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: lld/wasm/SyntheticSections.cpp:77
 
-  // Under certain circumstances we need to include extra information about the
-  // exports we are providing to the dynamic linker.  Currently this is only the
-  // case for TLS symbols where the exported value is relative to __tls_base
-  // rather than __memory_base.
+  // Under certain circumstances we need to include extra information about out
+  // exports and/or exports to the dynamic linker.
----------------
out exports and/or exports?


================
Comment at: lld/wasm/SyntheticSections.cpp:82
+  // For imports we need to notify the dynamic linker when an import is weak
+  // so that knows not to report and error for such symbols.
+  std::vector<const Symbol *> importInfo;
----------------



================
Comment at: lld/wasm/SyntheticSections.cpp:87
+    if (sym->isLive()) {
+      if (sym->isExported() && sym->isLive() && sym->isTLS() &&
+          isa<DefinedData>(sym)) {
----------------
don't need `sym->IsLive()` in the inner condition now


================
Comment at: lld/wasm/SyntheticSections.cpp:91
+      }
+      if (sym->isUndefWeak()) {
+        importInfo.push_back(sym);
----------------
is it only for weak undef symbols? What about just regular strong undef symbols?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111345