[lld] b49a798 - [lld][WebAssembly] Remove relocation target verification
Sam Clegg via llvm-commits
llvm-commits at lists.llvm.org
Tue May 11 12:05:57 PDT 2021
Author: Sam Clegg
Date: 2021-05-11T12:05:14-07:00
New Revision: b49a798e71f922a68628ad9e31ca12fdb864c2f5
URL: https://github.com/llvm/llvm-project/commit/b49a798e71f922a68628ad9e31ca12fdb864c2f5
DIFF: https://github.com/llvm/llvm-project/commit/b49a798e71f922a68628ad9e31ca12fdb864c2f5.diff
LOG: [lld][WebAssembly] Remove relocation target verification
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
controversial 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 modification 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.
Differential Revision: https://reviews.llvm.org/D102265
Added:
Modified:
lld/test/wasm/reloc-addend.s
lld/wasm/InputChunks.cpp
lld/wasm/InputFiles.cpp
lld/wasm/InputFiles.h
Removed:
################################################################################
diff --git a/lld/test/wasm/reloc-addend.s b/lld/test/wasm/reloc-addend.s
index 56c329614da35..acd57ce357a72 100644
--- a/lld/test/wasm/reloc-addend.s
+++ b/lld/test/wasm/reloc-addend.s
@@ -23,9 +23,20 @@ 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
diff --git a/lld/wasm/InputChunks.cpp b/lld/wasm/InputChunks.cpp
index 14c019065ab42..f73e3362ae8e8 100644
--- a/lld/wasm/InputChunks.cpp
+++ b/lld/wasm/InputChunks.cpp
@@ -57,70 +57,6 @@ StringRef InputChunk::getComdatName() const {
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
@@ -134,10 +70,6 @@ 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();
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 99e2d0b8cb64a..bfb60a32e21dd 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -135,71 +135,6 @@ uint64_t ObjFile::calcNewAddend(const WasmRelocation &reloc) const {
}
}
-// 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 {
diff --git a/lld/wasm/InputFiles.h b/lld/wasm/InputFiles.h
index 78b3ceea0a053..dab44cd6830c2 100644
--- a/lld/wasm/InputFiles.h
+++ b/lld/wasm/InputFiles.h
@@ -122,7 +122,6 @@ class ObjFile : public InputFile {
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];
};
More information about the llvm-commits
mailing list