[PATCH] D44184: Write DWARF data into WASM object file

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: include/llvm/Object/RelocVisitor.h:323
+
+  uint64_t visitWasm(uint32_t Rel, RelocationRef R, uint64_t Value) {
+    if (ObjToVisit.getArch() == Triple::wasm32) {
----------------
Where is this RelocVistor change used?  Can this be a separate change perhaps?


================
Comment at: lib/MC/WasmObjectWriter.cpp:175
 struct WasmCustomSection {
+  enum WasmCustomSectionKind { Custom, Debug };
+
----------------
How about just a bool here?  isDebug?


================
Comment at: lib/MC/WasmObjectWriter.cpp:490
+    const auto &FixupSectionName = FixupSection.getSectionName().str();
+    CustomSectionsRelocations[FixupSectionName].push_back(Rec);
+    return;
----------------
Could we move this down the end of the function and just have an .isMetadata() before the final else?

Do we need the above error?


================
Comment at: lib/Object/WasmObjectFile.cpp:1083
+      return SymbolRef::ST_Debug;
+    // Support of non-debug custom sections is not implemented yet.
+    break;
----------------
Seems like ELF always return ST_Debug for STT_SECTION type symbols:  ELFObjectFile.h:536


================
Comment at: lib/Object/WasmObjectFile.cpp:1237
+symbol_iterator WasmObjectFile::getRelocationSymbol(DataRefImpl Ref) const {
+  const wasm::WasmRelocation &Rel = getWasmRelocation(Ref);
+  DataRefImpl Sym;
----------------
Is this needed as part of this change?

On thing to note is that Rel.Index is not always a symbol.  For the TYPE_INDEX relocation types its a type index.   We might want to revisit this in order to be consistent.


Repository:
  rL LLVM

https://reviews.llvm.org/D44184