[WebAssembly] Only perform sanity checking of relocation targets in debug builds

[sbc100]

Summary

Also reduce the error to a warning so that debug and release
versions can still link the same inputs.

[ruiu]

LGTM with this chagne.

wasm/InputChunks.cpp

60

I think it is better to define this function whether it is debug or not make the number of moving things as small as possible. You can just not call it if not debug.

[sbc100]

feedback

[sbc100]

• rebase

[sbc100]

Also converted to a warning since it doesn't actually stop the linker doing its job if the input has something different at the existing location.

[sbc100]

• Add an additional check for padded LEB

wasm/InputChunks.h

 
 protected:
   InputChunk(ObjFile *F, Kind K)
       : File(F), Live(!Config->GcSections), SectionKind(K) {}
   virtual ~InputChunk() = default;
   virtual ArrayRef<uint8_t> data() const = 0;
   virtual uint32_t getInputSectionOffset() const = 0;
 
+  // Verifies the existing data at relocation targets matches our expectations.
+  // This is performed only debug builds as an extra sanity check.
+  void verifyRelocTargets() const;
+
   std::vector<WasmRelocation> Relocations;
   Kind SectionKind;
 };
 
 // Represents a WebAssembly data segment which can be included as part of
 // an output data segments.  Note that in WebAssembly, unlike ELF and other
 // formats, used the term "data segment" to refer to the continous regions of
 // memory that make on the data section. See:

wasm/InputChunks.cpp

if (Section.Relocations.empty())
     return;
   size_t Start = getInputSectionOffset();
   size_t Size = getSize();
   for (const WasmRelocation &R : Section.Relocations)
     if (R.Offset >= Start && R.Offset < Start + Size)
       Relocations.push_back(R);
 }
 
+void InputChunk::verifyRelocTargets() const {
+  for (const WasmRelocation &Rel : Relocations) {
+    uint32_t ExistingValue;
+    unsigned BytesRead = 0;
+    uint32_t Offset = Rel.Offset - getInputSectionOffset();
+    const uint8_t *Loc = data().data() + Offset;
+    switch (Rel.Type) {
+    case R_WEBASSEMBLY_TYPE_INDEX_LEB:
+    case R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
+    case R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
+    case R_WEBASSEMBLY_MEMORY_ADDR_LEB:
+      ExistingValue = decodeULEB128(Loc, &BytesRead);
+      break;
+    case R_WEBASSEMBLY_TABLE_INDEX_SLEB:
+    case R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
+      ExistingValue = static_cast<uint32_t>(decodeSLEB128(Loc, &BytesRead));
+      break;
+    case R_WEBASSEMBLY_TABLE_INDEX_I32:
+    case R_WEBASSEMBLY_MEMORY_ADDR_I32:
+    case R_WEBASSEMBLY_FUNCTION_OFFSET_I32:
+    case R_WEBASSEMBLY_SECTION_OFFSET_I32:
+      ExistingValue = static_cast<uint32_t>(read32le(Loc));
+      break;
+    default:
+      llvm_unreachable("unknown relocation type");
+    }
+
+    if (BytesRead && BytesRead != 5)
+      warn("expected LEB at relocation site be 5-byte padded");
+    uint32_t ExpectedValue = File->calcExpectedValue(Rel);
+    if (ExpectedValue != ExistingValue)
+      warn("unexpected existing value for " + ReloctTypeToString(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 + OutputOffset, data().data(), data().size());
 
   // Apply relocations
   if (Relocations.empty())
     return;
 
+#ifndef NDEBUG
+  verifyRelocTargets();
+#endif
+
   DEBUG(dbgs() << "applying relocations: " << getName()
                << " count=" << Relocations.size() << "\n");
   int32_t Off = OutputOffset - getInputSectionOffset();
 
   for (const WasmRelocation &Rel : Relocations) {
     uint8_t *Loc = Buf + Rel.Offset + Off;
     uint32_t Value = File->calcNewValue(Rel);
-    uint32_t ExistingValue;
     DEBUG(dbgs() << "apply reloc: type=" << ReloctTypeToString(Rel.Type)
                  << " addend=" << Rel.Addend << " index=" << Rel.Index
                  << " value=" << Value << " offset=" << Rel.Offset << "\n");
 
     switch (Rel.Type) {
     case R_WEBASSEMBLY_TYPE_INDEX_LEB:
     case R_WEBASSEMBLY_FUNCTION_INDEX_LEB:
     case R_WEBASSEMBLY_GLOBAL_INDEX_LEB:
     case R_WEBASSEMBLY_MEMORY_ADDR_LEB:
-      ExistingValue = decodeULEB128(Loc);
       encodeULEB128(Value, Loc, 5);
       break;
     case R_WEBASSEMBLY_TABLE_INDEX_SLEB:
     case R_WEBASSEMBLY_MEMORY_ADDR_SLEB:
-      ExistingValue = static_cast<uint32_t>(decodeSLEB128(Loc));
       encodeSLEB128(static_cast<int32_t>(Value), Loc, 5);
       break;
     case R_WEBASSEMBLY_TABLE_INDEX_I32:
     case R_WEBASSEMBLY_MEMORY_ADDR_I32:
     case R_WEBASSEMBLY_FUNCTION_OFFSET_I32:
     case R_WEBASSEMBLY_SECTION_OFFSET_I32:
-      ExistingValue = static_cast<uint32_t>(read32le(Loc));
       write32le(Loc, Value);
       break;
     default:
       llvm_unreachable("unknown relocation type");
     }
-
-    uint32_t ExpectedValue = File->calcExpectedValue(Rel);
-    if (ExpectedValue != ExistingValue)
-      error("unexpected existing value for " + ReloctTypeToString(Rel.Type) +
-            ": existing=" + Twine(ExistingValue) +
-            " expected=" + Twine(ExpectedValue));
   }
 }
 
 // Copy relocation entries to a given output stream.
 // This function is used only when a user passes "-r". For a regular link,
 // we consume relocations instead of copying them to an output file.
 void InputChunk::writeRelocations(raw_ostream &OS) const {
   if (Relocations.empty())