[PATCH] D87537: [lld][WebAssembly] Error on import/export of mutable global without `mutable-globals` feature

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:246-252
+  llvm::Reloc::Model RelocationModel;
+  unsigned PICLevel;
+  bool IsPIE;
+  std::tie(RelocationModel, PICLevel, IsPIE) = ParsePICArgs(*this, DriverArgs);
+  if (RelocationModel == llvm::Reloc::PIC_) {
+    CC1Args.push_back("+mutable-globals");
+  }
----------------
tlively wrote:
> This should also check that the user didn't pass `options::OPT_mno_mutable_globals` and a test should be added in clang/test/Driver/wasm-toolchain.c.
Sorry, I meant that if the user passes -fPIC as well as `-mno-mutable-globals`, we should give them a `diag::err_drv_argument_not_allowed_with` error, like we do above for -pthread features and below for -fwasm-exceptions features,


================
Comment at: lld/wasm/Writer.cpp:478
+                  toString(*sym) + "`. Use --no-check-features to suppress.");
+          else if (sym->isExported())
+            error(Twine("mutable global exported but 'mutable-globals' feature "
----------------
sbc100 wrote:
> tlively wrote:
> > could this `if (sym->isExported())` be turned into an `assert(sym->isExported())`?
> I don't think so.  What about regular internal globals that are neither imported nor exported?  Unless I'm missing something?
Ah, makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87537