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

[sbc100]

Summary

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

The two new test failures represent tests that were failing
silents previously with incorrect relocation entries.

[sbc100]

cleanup

[sbc100]

• update test expectations

[sbc100]

Regarding this test case, it really should be failing earlier because of the use of blockaddress... but I'm not sure how to make that happen.

Any idea how else I can create relocation against a temporary?

(I'm probably going to land this now anyway BTW, but I'm curious how else I might create a test that tickles this).

[dschuff]

Yeah come to think of it, I thought we just failed isel for blockaddress?

[sbc100]

In D43365#1010371, @dschuff wrote:

Yeah come to think of it, I thought we just failed isel for blockaddress?

We do, but this is not an instruction, its purely a relocation.

llvm/trunk/lib/MC/WasmObjectWriter.cpp

void WasmObjectWriter::recordRelocation(MCAssembler &Asm,
   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");
 
     // Let A, B and C being the components of Target and R be the location of
     // the fixup. If the fixup is not pcrel, we want to compute (A - B + C).
     // If it is pcrel, we want to compute (A - B + C - R).
 
void WasmObjectWriter::recordRelocation(MCAssembler &Asm,
   assert(!IsPCRel);
   assert(SymA);
 
   unsigned Type = getRelocType(Target, Fixup);
 
   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");
 }
 
 // Write X as an (unsigned) LEB value at offset Offset in Stream, padded
 // to allow patching.
 static void
 WritePatchableLEB(raw_pwrite_stream &Stream, uint32_t X, uint64_t Offset) {
   uint8_t Buffer[5];
WasmObjectWriter::getProvisionalValue(const WasmRelocationEntry &RelEntry) {
   case wasm::R_WEBASSEMBLY_MEMORY_ADDR_I32:
   case wasm::R_WEBASSEMBLY_MEMORY_ADDR_SLEB: {
     // Provisional value is address of the global
     const MCSymbolWasm *Sym = ResolveSymbol(*RelEntry.Symbol);
     // For undefined symbols, use zero
     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;
 
     // Ignore overflow. LLVM allows address arithmetic to silently wrap.
     return Address;
   }
   default:

llvm/trunk/lib/Target/WebAssembly/known_gcc_test_failures.txt

 # Tests which are known to fail from the GCC torture test suite.
 
 # Syntax: Each line has a single test to be marked as a 'known failure' (or
 # 'exclusion'. Known failures are expected to fail, and will cause an error if
 # they pass. (Known failures that do not run at all will not cause an
 # error). The format is
 # <name> <attributes> # comment
 #
 # The attributes in this case represent the different arguments used to
 # compiler: 'wasm-s' is for compiling to .s files, and 'wasm-o' for compiling
 # to wasm object files (.o).
 
 # Computed gotos are not supported (Cannot select BlockAddress/BRIND)
-20071220-1.c wasm-o,O0
[phone].c wasm-o
[phone].c wasm-o
 20040302-1.c
 20041214-1.c O0
 20071210-1.c
 20071220-1.c wasm-s,O0
 920501-4.c
 920501-5.c
 comp-goto-1.c
 980526-1.c

 builtin-line1.C
 builtin-location.C
 devirt-6.C  # bad main signature
 devirt-13.C  # bad main signature
 devirt-14.C  # bad main signature
 devirt-21.C  # bad main signature
 devirt-23.C  # bad main signature
 
-# Untriaged: Assertion failure in WasmObjectWriter::applyRelocations
-20071220-2.c wasm-o,O0
-
 # Untriaged C++ failures
 spec5.C
 addr1.C
 ef_test.C
 friend18.C
 member2.C
 new39.C
 new40.C

llvm/trunk/test/MC/WebAssembly/blockaddress.ll

+; 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"
+
+@foo = internal global i8* blockaddress(@bar, %addr), align 4
+
+define hidden i32 @bar() #0 {
+entry:
+  br label %addr
+
+addr:
+  ret i32 0
+}