[PATCH] D43365: [WebAssembly] MC: Make explicit our current lack of support for relocations against unnamed temporary symbols.

[Sam Clegg via Phabricator via llvm-commits]

sbc100 created this revision.
Herald added subscribers: llvm-commits, sunfish, aheejin, jgravelle-google, dschuff, jfb.

Add an explict check before looking up symbol in SymbolIndices.
This was previously silently succeeding and returning zero for such
unnamed temporaries.


Repository:
  rL LLVM

https://reviews.llvm.org/D43365

Files:
  lib/MC/WasmObjectWriter.cpp
  test/MC/WebAssembly/blockaddress.ll


Index: test/MC/WebAssembly/blockaddress.ll
===================================================================
--- /dev/null
+++ test/MC/WebAssembly/blockaddress.ll
@@ -0,0 +1,15 @@
+; TODO(sbc): Make this test pass by adding support for unnamed tempoaries
+; in wasm relocations.
+; RUN: not llc -filetype=obj %s
+
+target triple = "wasm32-unknown-unknown-wasm"
+
+ at foo = internal global i8* blockaddress(@bar, %addr), align 4
+
+define hidden i32 @bar() #0 {
+entry:
+  br label %addr
+
+addr:
+  ret i32 0
+}
Index: lib/MC/WasmObjectWriter.cpp
===================================================================
--- lib/MC/WasmObjectWriter.cpp
+++ lib/MC/WasmObjectWriter.cpp
@@ -347,14 +347,19 @@
   bool IsPCRel = Backend.getFixupKindInfo(Fixup.getKind()).Flags &
                  MCFixupKindInfo::FKF_IsPCRel;
   const auto &FixupSection = cast<MCSectionWasm>(*Fragment->getParent());
+
   uint64_t C = Target.getConstant();
   uint64_t FixupOffset = Layout.getFragmentOffset(Fragment) + Fixup.getOffset();
   MCContext &Ctx = Asm.getContext();
 
   // The .init_array isn't translated as data, so don't do relocations in it.
   if (FixupSection.getSectionName().startswith(".init_array"))
     return;
 
+  // TODO(sbc): Add support for debug sections.
+  if (FixupSection.getKind().isMetadata())
+    return;
+
   if (const MCSymbolRefExpr *RefB = Target.getSymB()) {
     assert(RefB->getKind() == MCSymbolRefExpr::VK_None &&
            "Should not have constructed this");
@@ -423,12 +428,20 @@
   WasmRelocationEntry Rec(FixupOffset, SymA, C, Type, &FixupSection);
   DEBUG(dbgs() << "WasmReloc: " << Rec << "\n");
 
+  // Relocation other than R_WEBASSEMBLY_TYPE_INDEX_LEB are currently required
+  // to be against a named symbol.
+  // TODO(sbc): Add support for relocations against unnamed temporaries such
+  // as those generated by llvm's `blockaddress`.
+  // See: test/MC/WebAssembly/blockaddress.ll
+  if (SymA->getName().empty() && Type != wasm::R_WEBASSEMBLY_TYPE_INDEX_LEB)
+    report_fatal_error("relocations against un-named temporaries are not yet "
+                       "supported by wasm");
+
   if (FixupSection.isWasmData())
     DataRelocations.push_back(Rec);
   else if (FixupSection.getKind().isText())
     CodeRelocations.push_back(Rec);
-  else if (!FixupSection.getKind().isMetadata())
-    // TODO(sbc): Add support for debug sections.
+  else
     llvm_unreachable("unexpected section type");
 }
 
@@ -496,6 +509,9 @@
     if (!Sym->isDefined())
       return 0;
 
+    if (!SymbolIndices.count(Sym))
+      report_fatal_error("symbol not found in function/global index space: " +
+                         Sym->getName());
     uint32_t GlobalIndex = SymbolIndices[Sym];
     const WasmGlobal& Global = Globals[GlobalIndex - NumGlobalImports];
     uint64_t Address = Global.InitialValue + RelEntry.Addend;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43365.134535.patch
Type: text/x-patch
Size: 2855 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180216/8d54f1a6/attachment.bin>