[lld][WebAssembly] Remove relocation target verification

[sbc100]

Summary

We have this extra step in wasm-ld that doesn't exist in other lld
backend which verifies the existing contents of the relocation targets.
This was originally intended as an extra form of double checking and an
aid to compiler developers. However it has always been somewhat
contravertial and there have been suggestions in the past the we simply
remove it.

My motivation for removing it now is that its causing me a headache
when trying to fix an issue with negative addends. In the case of
negative addends that final result can be wrapped/negative but this
checking code would require significant modifiation to be able to deal
with that case. For example with some test cases I'm looking at I'm
seeing error like this:

wasm-ld: warning: /usr/local/google/home/sbc/dev/wasm/llvm-build/tools/lld/test/wasm/Output/merge-string.s.tmp.o:(.rodata_relocs): unexpected existing value for R_WASM_MEMORY_ADDR_I32: existing=FFFFFFFA expected=FFFFFFFFFFFFFFFA

Rather than try to refactor calcExpectedValue to somehow return two
different types of results (32 and 64-bit) depending on the relocation
type, I think we can just remove this code.

[dschuff]

lld/test/wasm/reloc-addend.s

39

Does it make sense to test negative addends for other reloc types too?

[sbc100]

lld/test/wasm/reloc-addend.s

39

Yes.... although I might push that to a followup since this change is mostly about removing the checker.

lld/test/wasm/reloc-addend.s

 # bytes in.  This generates an addend of 64 which is the value at which signed
 # and unsigned LEB encodes will differ.
   .section  .data.bar,"",@
   .p2align  2
 bar:
   .int32  foo+64
   .size bar, 4
 
+# Check that negative addends also work here
+  .section  .data.negative_addend,"",@
+  .p2align  2
+negative_addend:
+  .int32  foo-16
+  .size negative_addend, 4
+
 # CHECK:        - Type:            DATA
 # CHECK-NEXT:     Relocations:
 # CHECK-NEXT:       - Type:            R_WASM_MEMORY_ADDR_I32
 # CHECK-NEXT:         Index:           0
 # CHECK-NEXT:         Offset:          0x6
 # CHECK-NEXT:         Addend:          64
+# CHECK-NEXT:       - Type:            R_WASM_MEMORY_ADDR_I32
+# CHECK-NEXT:         Index:           0
+# CHECK-NEXT:         Offset:          0xF
+# CHECK-NEXT:         Addend:          -16

lld/wasm/InputChunks.cpp

 namespace wasm {
 StringRef InputChunk::getComdatName() const {
   uint32_t index = getComdat();
   if (index == UINT32_MAX)
     return StringRef();
   return file->getWasmObj()->linkingData().Comdats[index];
 }
 
-void InputChunk::verifyRelocTargets() const {
-  for (const WasmRelocation &rel : relocations) {
-    uint64_t existingValue;
-    unsigned bytesRead = 0;
-    unsigned paddedLEBWidth = 5;
-    auto offset = rel.Offset - getInputSectionOffset();
-    const uint8_t *loc = data().data() + offset;
-    switch (rel.Type) {
-    case R_WASM_TYPE_INDEX_LEB:
-    case R_WASM_FUNCTION_INDEX_LEB:
-    case R_WASM_GLOBAL_INDEX_LEB:
-    case R_WASM_EVENT_INDEX_LEB:
-    case R_WASM_MEMORY_ADDR_LEB:
-    case R_WASM_TABLE_NUMBER_LEB:
-      existingValue = decodeULEB128(loc, &bytesRead);
-      break;
-    case R_WASM_MEMORY_ADDR_LEB64:
-      existingValue = decodeULEB128(loc, &bytesRead);
-      paddedLEBWidth = 10;
-      break;
-    case R_WASM_TABLE_INDEX_SLEB:
-    case R_WASM_TABLE_INDEX_REL_SLEB:
-    case R_WASM_MEMORY_ADDR_SLEB:
-    case R_WASM_MEMORY_ADDR_REL_SLEB:
-    case R_WASM_MEMORY_ADDR_TLS_SLEB:
-      existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
-      break;
-    case R_WASM_TABLE_INDEX_SLEB64:
-    case R_WASM_MEMORY_ADDR_SLEB64:
-    case R_WASM_MEMORY_ADDR_REL_SLEB64:
-      existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
-      paddedLEBWidth = 10;
-      break;
-    case R_WASM_TABLE_INDEX_I32:
-    case R_WASM_MEMORY_ADDR_I32:
-    case R_WASM_FUNCTION_OFFSET_I32:
-    case R_WASM_SECTION_OFFSET_I32:
-    case R_WASM_GLOBAL_INDEX_I32:
-    case R_WASM_MEMORY_ADDR_LOCREL_I32:
-      existingValue = read32le(loc);
-      break;
-    case R_WASM_TABLE_INDEX_I64:
-    case R_WASM_MEMORY_ADDR_I64:
-    case R_WASM_FUNCTION_OFFSET_I64:
-      existingValue = read64le(loc);
-      break;
-    default:
-      llvm_unreachable("unknown relocation type");
-    }
-
-    if (bytesRead && bytesRead != paddedLEBWidth)
-      warn("expected LEB at relocation site be 5/10-byte padded");
-
-    if (rel.Type != R_WASM_GLOBAL_INDEX_LEB &&
-        rel.Type != R_WASM_GLOBAL_INDEX_I32) {
-      auto expectedValue = file->calcExpectedValue(rel);
-      if (expectedValue != existingValue)
-        warn(toString(this) + ": unexpected existing value for " +
-             relocTypeToString(rel.Type) + ": existing=" +
-             Twine(existingValue) + " expected=" + Twine(expectedValue));
-    }
-  }
-}
-
 // Copy this input chunk to an mmap'ed output file and apply relocations.
 void InputChunk::writeTo(uint8_t *buf) const {
   // Copy contents
   memcpy(buf + outSecOff, data().data(), data().size());
 
   // Apply relocations
   relocate(buf + outSecOff);
 }
 
 void InputChunk::relocate(uint8_t *buf) const {
   if (relocations.empty())
     return;
 
-#ifndef NDEBUG
-  verifyRelocTargets();
-#endif
-
   LLVM_DEBUG(dbgs() << "applying relocations: " << toString(this)
                     << " count=" << relocations.size() << "\n");
   int32_t inputSectionOffset = getInputSectionOffset();
   auto tombstone = getTombstone();
 
   for (const WasmRelocation &rel : relocations) {
     uint8_t *loc = buf + rel.Offset - inputSectionOffset;
     auto value = file->calcNewValue(rel, tombstone, this);

lld/wasm/InputFiles.h

public:
   const WasmObjectFile *getWasmObj() const { return wasmObj.get(); }
 
   void dumpInfo() const;
 
   uint32_t calcNewIndex(const WasmRelocation &reloc) const;
   uint64_t calcNewValue(const WasmRelocation &reloc, uint64_t tombstone,
                         const InputChunk *chunk) const;
   uint64_t calcNewAddend(const WasmRelocation &reloc) const;
-  uint64_t calcExpectedValue(const WasmRelocation &reloc) const;
   Symbol *getSymbol(const WasmRelocation &reloc) const {
     return symbols[reloc.Index];
   };
 
   const WasmSection *codeSection = nullptr;
   const WasmSection *dataSection = nullptr;
 
   // Maps input type indices to output type indices

lld/wasm/InputFiles.cpp

case R_WASM_MEMORY_ADDR_LOCREL_I32:
     return reloc.Addend;
   case R_WASM_SECTION_OFFSET_I32:
     return getSectionSymbol(reloc.Index)->section->getOffset(reloc.Addend);
   default:
     llvm_unreachable("unexpected relocation type");
   }
 }
 
-// Calculate the value we expect to find at the relocation location.
-// This is used as a sanity check before applying a relocation to a given
-// location.  It is useful for catching bugs in the compiler and linker.
-uint64_t ObjFile::calcExpectedValue(const WasmRelocation &reloc) const {
-  switch (reloc.Type) {
-  case R_WASM_TABLE_INDEX_I32:
-  case R_WASM_TABLE_INDEX_I64:
-  case R_WASM_TABLE_INDEX_SLEB:
-  case R_WASM_TABLE_INDEX_SLEB64: {
-    const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
-    return tableEntries[sym.Info.ElementIndex];
-  }
-  case R_WASM_TABLE_INDEX_REL_SLEB: {
-    const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
-    return tableEntriesRel[sym.Info.ElementIndex];
-  }
-  case R_WASM_MEMORY_ADDR_LEB:
-  case R_WASM_MEMORY_ADDR_LEB64:
-  case R_WASM_MEMORY_ADDR_SLEB:
-  case R_WASM_MEMORY_ADDR_SLEB64:
-  case R_WASM_MEMORY_ADDR_REL_SLEB:
-  case R_WASM_MEMORY_ADDR_REL_SLEB64:
-  case R_WASM_MEMORY_ADDR_I32:
-  case R_WASM_MEMORY_ADDR_I64:
-  case R_WASM_MEMORY_ADDR_TLS_SLEB:
-  case R_WASM_MEMORY_ADDR_LOCREL_I32: {
-    const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
-    if (sym.isUndefined())
-      return 0;
-    const WasmSegment &segment =
-        wasmObj->dataSegments()[sym.Info.DataRef.Segment];
-    if (segment.Data.Offset.Opcode == WASM_OPCODE_I32_CONST)
-      return segment.Data.Offset.Value.Int32 + sym.Info.DataRef.Offset +
-             reloc.Addend;
-    else if (segment.Data.Offset.Opcode == WASM_OPCODE_I64_CONST)
-      return segment.Data.Offset.Value.Int64 + sym.Info.DataRef.Offset +
-             reloc.Addend;
-    else
-      llvm_unreachable("unknown init expr opcode");
-  }
-  case R_WASM_FUNCTION_OFFSET_I32:
-  case R_WASM_FUNCTION_OFFSET_I64: {
-    const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
-    InputFunction *f =
-        functions[sym.Info.ElementIndex - wasmObj->getNumImportedFunctions()];
-    return f->getFunctionInputOffset() + f->getFunctionCodeOffset() +
-           reloc.Addend;
-  }
-  case R_WASM_SECTION_OFFSET_I32:
-    return reloc.Addend;
-  case R_WASM_TYPE_INDEX_LEB:
-    return reloc.Index;
-  case R_WASM_FUNCTION_INDEX_LEB:
-  case R_WASM_GLOBAL_INDEX_LEB:
-  case R_WASM_GLOBAL_INDEX_I32:
-  case R_WASM_EVENT_INDEX_LEB:
-  case R_WASM_TABLE_NUMBER_LEB: {
-    const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
-    return sym.Info.ElementIndex;
-  }
-  default:
-    llvm_unreachable("unknown relocation type");
-  }
-}
-
 // Translate from the relocation's index into the final linked output value.
 uint64_t ObjFile::calcNewValue(const WasmRelocation &reloc, uint64_t tombstone,
                                const InputChunk *chunk) const {
   const Symbol* sym = nullptr;
   if (reloc.Type != R_WASM_TYPE_INDEX_LEB) {
     sym = symbols[reloc.Index];
 
     // We can end up with relocations against non-live symbols.  For example