[PATCH] D111388: [WebAssembly] Make EH work with dynamic linking

[Heejin Ahn via Phabricator via llvm-commits]

aheejin added inline comments.


================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:1127-1129
+      if (readUint8(Ctx) != 0) // Reserved 'attribute' field
+        return make_error<GenericBinaryError>("invalid attribute",
+                                              object_error::parse_failed);
----------------
This was deleted by mistake in D111086. It was fine with that patch because until now we don't have any case of tag imports.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:281-284
-      // The target flag indicates whether this is a symbol for a
-      // variable or a function.
-      assert(MO.getTargetFlags() == 0 &&
-             "WebAssembly uses only symbol flags on ExternalSymbols");
----------------
Now we are using LSDA symbol with target flags, for this I removed this comment and `assert`.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp:287-293
     case MachineOperand::MO_MCSymbol:
-      // This is currently used only for LSDA symbols (GCC_except_table),
-      // because global addresses or other external symbols are handled above.
       assert(MO.getTargetFlags() == 0 &&
              "WebAssembly does not use target flags on MCSymbol");
       MCOp = lowerSymbolOperand(MO, MO.getMCSymbol());
       break;
----------------
I tried to remove this `MO_MCSymbol` `case` altogether, but it turned out this `case`  is used by `__funcref_call_table` and `__indirect_call_table`. So I only deleted the (already incorrect) comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111388