Skip to content

Commit b49a798

Browse files
committed
[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
1 parent 9558b60 commit b49a798

File tree

4 files changed

+11
-134
lines changed

4 files changed

+11
-134
lines changed

lld/test/wasm/reloc-addend.s

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,20 @@ bar:
2323
.int32 foo+64
2424
.size bar, 4
2525

26+
# Check that negative addends also work here
27+
.section .data.negative_addend,"",@
28+
.p2align 2
29+
negative_addend:
30+
.int32 foo-16
31+
.size negative_addend, 4
32+
2633
# CHECK: - Type: DATA
2734
# CHECK-NEXT: Relocations:
2835
# CHECK-NEXT: - Type: R_WASM_MEMORY_ADDR_I32
2936
# CHECK-NEXT: Index: 0
3037
# CHECK-NEXT: Offset: 0x6
3138
# CHECK-NEXT: Addend: 64
39+
# CHECK-NEXT: - Type: R_WASM_MEMORY_ADDR_I32
40+
# CHECK-NEXT: Index: 0
41+
# CHECK-NEXT: Offset: 0xF
42+
# CHECK-NEXT: Addend: -16

lld/wasm/InputChunks.cpp

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -57,70 +57,6 @@ StringRef InputChunk::getComdatName() const {
5757
return file->getWasmObj()->linkingData().Comdats[index];
5858
}
5959

60-
void InputChunk::verifyRelocTargets() const {
61-
for (const WasmRelocation &rel : relocations) {
62-
uint64_t existingValue;
63-
unsigned bytesRead = 0;
64-
unsigned paddedLEBWidth = 5;
65-
auto offset = rel.Offset - getInputSectionOffset();
66-
const uint8_t *loc = data().data() + offset;
67-
switch (rel.Type) {
68-
case R_WASM_TYPE_INDEX_LEB:
69-
case R_WASM_FUNCTION_INDEX_LEB:
70-
case R_WASM_GLOBAL_INDEX_LEB:
71-
case R_WASM_EVENT_INDEX_LEB:
72-
case R_WASM_MEMORY_ADDR_LEB:
73-
case R_WASM_TABLE_NUMBER_LEB:
74-
existingValue = decodeULEB128(loc, &bytesRead);
75-
break;
76-
case R_WASM_MEMORY_ADDR_LEB64:
77-
existingValue = decodeULEB128(loc, &bytesRead);
78-
paddedLEBWidth = 10;
79-
break;
80-
case R_WASM_TABLE_INDEX_SLEB:
81-
case R_WASM_TABLE_INDEX_REL_SLEB:
82-
case R_WASM_MEMORY_ADDR_SLEB:
83-
case R_WASM_MEMORY_ADDR_REL_SLEB:
84-
case R_WASM_MEMORY_ADDR_TLS_SLEB:
85-
existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
86-
break;
87-
case R_WASM_TABLE_INDEX_SLEB64:
88-
case R_WASM_MEMORY_ADDR_SLEB64:
89-
case R_WASM_MEMORY_ADDR_REL_SLEB64:
90-
existingValue = static_cast<uint64_t>(decodeSLEB128(loc, &bytesRead));
91-
paddedLEBWidth = 10;
92-
break;
93-
case R_WASM_TABLE_INDEX_I32:
94-
case R_WASM_MEMORY_ADDR_I32:
95-
case R_WASM_FUNCTION_OFFSET_I32:
96-
case R_WASM_SECTION_OFFSET_I32:
97-
case R_WASM_GLOBAL_INDEX_I32:
98-
case R_WASM_MEMORY_ADDR_LOCREL_I32:
99-
existingValue = read32le(loc);
100-
break;
101-
case R_WASM_TABLE_INDEX_I64:
102-
case R_WASM_MEMORY_ADDR_I64:
103-
case R_WASM_FUNCTION_OFFSET_I64:
104-
existingValue = read64le(loc);
105-
break;
106-
default:
107-
llvm_unreachable("unknown relocation type");
108-
}
109-
110-
if (bytesRead && bytesRead != paddedLEBWidth)
111-
warn("expected LEB at relocation site be 5/10-byte padded");
112-
113-
if (rel.Type != R_WASM_GLOBAL_INDEX_LEB &&
114-
rel.Type != R_WASM_GLOBAL_INDEX_I32) {
115-
auto expectedValue = file->calcExpectedValue(rel);
116-
if (expectedValue != existingValue)
117-
warn(toString(this) + ": unexpected existing value for " +
118-
relocTypeToString(rel.Type) + ": existing=" +
119-
Twine(existingValue) + " expected=" + Twine(expectedValue));
120-
}
121-
}
122-
}
123-
12460
// Copy this input chunk to an mmap'ed output file and apply relocations.
12561
void InputChunk::writeTo(uint8_t *buf) const {
12662
// Copy contents
@@ -134,10 +70,6 @@ void InputChunk::relocate(uint8_t *buf) const {
13470
if (relocations.empty())
13571
return;
13672

137-
#ifndef NDEBUG
138-
verifyRelocTargets();
139-
#endif
140-
14173
LLVM_DEBUG(dbgs() << "applying relocations: " << toString(this)
14274
<< " count=" << relocations.size() << "\n");
14375
int32_t inputSectionOffset = getInputSectionOffset();

lld/wasm/InputFiles.cpp

Lines changed: 0 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -135,71 +135,6 @@ uint64_t ObjFile::calcNewAddend(const WasmRelocation &reloc) const {
135135
}
136136
}
137137

138-
// Calculate the value we expect to find at the relocation location.
139-
// This is used as a sanity check before applying a relocation to a given
140-
// location. It is useful for catching bugs in the compiler and linker.
141-
uint64_t ObjFile::calcExpectedValue(const WasmRelocation &reloc) const {
142-
switch (reloc.Type) {
143-
case R_WASM_TABLE_INDEX_I32:
144-
case R_WASM_TABLE_INDEX_I64:
145-
case R_WASM_TABLE_INDEX_SLEB:
146-
case R_WASM_TABLE_INDEX_SLEB64: {
147-
const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
148-
return tableEntries[sym.Info.ElementIndex];
149-
}
150-
case R_WASM_TABLE_INDEX_REL_SLEB: {
151-
const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
152-
return tableEntriesRel[sym.Info.ElementIndex];
153-
}
154-
case R_WASM_MEMORY_ADDR_LEB:
155-
case R_WASM_MEMORY_ADDR_LEB64:
156-
case R_WASM_MEMORY_ADDR_SLEB:
157-
case R_WASM_MEMORY_ADDR_SLEB64:
158-
case R_WASM_MEMORY_ADDR_REL_SLEB:
159-
case R_WASM_MEMORY_ADDR_REL_SLEB64:
160-
case R_WASM_MEMORY_ADDR_I32:
161-
case R_WASM_MEMORY_ADDR_I64:
162-
case R_WASM_MEMORY_ADDR_TLS_SLEB:
163-
case R_WASM_MEMORY_ADDR_LOCREL_I32: {
164-
const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
165-
if (sym.isUndefined())
166-
return 0;
167-
const WasmSegment &segment =
168-
wasmObj->dataSegments()[sym.Info.DataRef.Segment];
169-
if (segment.Data.Offset.Opcode == WASM_OPCODE_I32_CONST)
170-
return segment.Data.Offset.Value.Int32 + sym.Info.DataRef.Offset +
171-
reloc.Addend;
172-
else if (segment.Data.Offset.Opcode == WASM_OPCODE_I64_CONST)
173-
return segment.Data.Offset.Value.Int64 + sym.Info.DataRef.Offset +
174-
reloc.Addend;
175-
else
176-
llvm_unreachable("unknown init expr opcode");
177-
}
178-
case R_WASM_FUNCTION_OFFSET_I32:
179-
case R_WASM_FUNCTION_OFFSET_I64: {
180-
const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
181-
InputFunction *f =
182-
functions[sym.Info.ElementIndex - wasmObj->getNumImportedFunctions()];
183-
return f->getFunctionInputOffset() + f->getFunctionCodeOffset() +
184-
reloc.Addend;
185-
}
186-
case R_WASM_SECTION_OFFSET_I32:
187-
return reloc.Addend;
188-
case R_WASM_TYPE_INDEX_LEB:
189-
return reloc.Index;
190-
case R_WASM_FUNCTION_INDEX_LEB:
191-
case R_WASM_GLOBAL_INDEX_LEB:
192-
case R_WASM_GLOBAL_INDEX_I32:
193-
case R_WASM_EVENT_INDEX_LEB:
194-
case R_WASM_TABLE_NUMBER_LEB: {
195-
const WasmSymbol &sym = wasmObj->syms()[reloc.Index];
196-
return sym.Info.ElementIndex;
197-
}
198-
default:
199-
llvm_unreachable("unknown relocation type");
200-
}
201-
}
202-
203138
// Translate from the relocation's index into the final linked output value.
204139
uint64_t ObjFile::calcNewValue(const WasmRelocation &reloc, uint64_t tombstone,
205140
const InputChunk *chunk) const {

lld/wasm/InputFiles.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ class ObjFile : public InputFile {
122122
uint64_t calcNewValue(const WasmRelocation &reloc, uint64_t tombstone,
123123
const InputChunk *chunk) const;
124124
uint64_t calcNewAddend(const WasmRelocation &reloc) const;
125-
uint64_t calcExpectedValue(const WasmRelocation &reloc) const;
126125
Symbol *getSymbol(const WasmRelocation &reloc) const {
127126
return symbols[reloc.Index];
128127
};

0 commit comments

Comments
 (0)