Skip to content

Commit 0126375

Browse files
asbasb
committed
[WebAssembly] Use location of operand for operand-based type check errors
This addresses a series of FIXMEs introduced in D122020. A follow-up patch (D122128) addresses the bug that is exposed by this change (an issue with source location information when lexing identifiers). Differential Revision: https://reviews.llvm.org/D122127
1 parent 5bcc90e commit 0126375

File tree

4 files changed

+50
-56
lines changed

4 files changed

+50
-56
lines changed

‎llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1017,7 +1017,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
10171017
Inst.setOpcode(Opc64);
10181018
}
10191019
}
1020-
if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst))
1020+
if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst, Operands))
10211021
return true;
10221022
Out.emitInstruction(Inst, getSTI());
10231023
if (CurrentState == EndFunction) {

‎llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -210,50 +210,51 @@ bool WebAssemblyAsmTypeCheck::endOfFunction(SMLoc ErrorLoc) {
210210
return false;
211211
}
212212

213-
bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst) {
213+
bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
214+
OperandVector &Operands) {
214215
auto Opc = Inst.getOpcode();
215216
auto Name = GetMnemonic(Opc);
216217
dumpTypeStack("typechecking " + Name + ": ");
217218
wasm::ValType Type;
218219
if (Name == "local.get") {
219-
if (getLocal(ErrorLoc, Inst, Type))
220+
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
220221
return true;
221222
Stack.push_back(Type);
222223
} else if (Name == "local.set") {
223-
if (getLocal(ErrorLoc, Inst, Type))
224+
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
224225
return true;
225226
if (popType(ErrorLoc, Type))
226227
return true;
227228
} else if (Name == "local.tee") {
228-
if (getLocal(ErrorLoc, Inst, Type))
229+
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
229230
return true;
230231
if (popType(ErrorLoc, Type))
231232
return true;
232233
Stack.push_back(Type);
233234
} else if (Name == "global.get") {
234-
if (getGlobal(ErrorLoc, Inst, Type))
235+
if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
235236
return true;
236237
Stack.push_back(Type);
237238
} else if (Name == "global.set") {
238-
if (getGlobal(ErrorLoc, Inst, Type))
239+
if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
239240
return true;
240241
if (popType(ErrorLoc, Type))
241242
return true;
242243
} else if (Name == "table.get") {
243-
if (getTable(ErrorLoc, Inst, Type))
244+
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
244245
return true;
245246
if (popType(ErrorLoc, wasm::ValType::I32))
246247
return true;
247248
Stack.push_back(Type);
248249
} else if (Name == "table.set") {
249-
if (getTable(ErrorLoc, Inst, Type))
250+
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
250251
return true;
251252
if (popType(ErrorLoc, Type))
252253
return true;
253254
if (popType(ErrorLoc, wasm::ValType::I32))
254255
return true;
255256
} else if (Name == "table.fill") {
256-
if (getTable(ErrorLoc, Inst, Type))
257+
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
257258
return true;
258259
if (popType(ErrorLoc, wasm::ValType::I32))
259260
return true;
@@ -281,25 +282,27 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst) {
281282
return true;
282283
} else if (Name == "call" || Name == "return_call") {
283284
const MCSymbolRefExpr *SymRef;
284-
if (getSymRef(ErrorLoc, Inst, SymRef))
285+
if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
285286
return true;
286287
auto WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
287288
auto Sig = WasmSym->getSignature();
288289
if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_FUNCTION)
289-
return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
290-
" missing .functype");
290+
return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
291+
WasmSym->getName() +
292+
" missing .functype");
291293
if (checkSig(ErrorLoc, *Sig)) return true;
292294
if (Name == "return_call" && endOfFunction(ErrorLoc))
293295
return true;
294296
} else if (Name == "catch") {
295297
const MCSymbolRefExpr *SymRef;
296-
if (getSymRef(ErrorLoc, Inst, SymRef))
298+
if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
297299
return true;
298300
const auto *WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
299301
const auto *Sig = WasmSym->getSignature();
300302
if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_TAG)
301-
return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
302-
" missing .tagtype");
303+
return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
304+
WasmSym->getName() +
305+
" missing .tagtype");
303306
// catch instruction pushes values whose types are specified in the tag's
304307
// "params" part
305308
Stack.insert(Stack.end(), Sig->Params.begin(), Sig->Params.end());

‎llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
#ifndef LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H
1717
#define LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H
1818

19-
#include "llvm/MC/MCParser/MCAsmParser.h"
20-
#include "llvm/MC/MCInstrInfo.h"
2119
#include "llvm/BinaryFormat/Wasm.h"
20+
#include "llvm/MC/MCInstrInfo.h"
21+
#include "llvm/MC/MCParser/MCAsmParser.h"
22+
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
2223
#include "llvm/MC/MCSymbol.h"
2324

2425
namespace llvm {
@@ -53,7 +54,7 @@ class WebAssemblyAsmTypeCheck final {
5354
void localDecl(const SmallVector<wasm::ValType, 4> &Locals);
5455
void setLastSig(const wasm::WasmSignature &Sig) { LastSig = Sig; }
5556
bool endOfFunction(SMLoc ErrorLoc);
56-
bool typeCheck(SMLoc ErrorLoc, const MCInst &Inst);
57+
bool typeCheck(SMLoc ErrorLoc, const MCInst &Inst, OperandVector &Operands);
5758

5859
void Clear() {
5960
Stack.clear();

‎llvm/test/MC/WebAssembly/type-checker-errors.s

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@
66

77
local_get_no_local_type:
88
.functype local_get_no_local_type () -> ()
9-
# FIXME: Error location should be at operand.
10-
# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
9+
# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
1110
local.get 0
1211
end_function
1312

1413
local_set_no_local_type:
1514
.functype local_set_no_local_type () -> ()
16-
# FIXME: Error location should be at operand.
17-
# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
15+
# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
1816
local.set 0
1917
end_function
2018

@@ -35,8 +33,7 @@ local_set_type_mismatch:
3533

3634
local_tee_no_local_type:
3735
.functype local_tee_no_local_type () -> ()
38-
# FIXME: Error location should be at operand.
39-
# CHECK: :[[@LINE+1]]:3: error: no local type specified for index 0
36+
# CHECK: :[[@LINE+1]]:13: error: no local type specified for index 0
4037
local.tee 0
4138
end_function
4239

@@ -57,29 +54,27 @@ local_tee_type_mismatch:
5754

5855
global_get_missing_globaltype:
5956
.functype global_get_missing_globaltype () -> ()
60-
# FIXME: Error location should be at operand.
61-
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
57+
# FIXME: Error location should be at beginning of operand.
58+
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
6259
global.get foo
6360
end_function
6461

6562
global_get_expected_expression_operand:
6663
.functype global_get_expected_expression_operand () -> ()
67-
# FIXME: Error location should be at operand.
68-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
64+
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
6965
global.get 1
7066
end_function
7167

7268
global_set_missing_globaltype:
7369
.functype global_set_missing_globaltype () -> ()
74-
# FIXME: Error location should be at operand.
75-
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
70+
# FIXME: Error location should be at beginning of operand.
71+
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
7672
global.set foo
7773
end_function
7874

7975
global_set_expected_expression_operand:
8076
.functype global_set_expected_expression_operand () -> ()
81-
# FIXME: Error location should be at operand.
82-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
77+
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
8378
global.set 1
8479
end_function
8580

@@ -100,15 +95,14 @@ global_set_type_mismatch:
10095

10196
table_get_expected_expression_operand:
10297
.functype table_get_expected_expression_operand () -> ()
103-
# FIXME: Error location should be at operand.
104-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
98+
# CHECK: :[[@LINE+1]]:13: error: expected expression operand
10599
table.get 1
106100
end_function
107101

108102
table_get_missing_tabletype:
109103
.functype table_get_missing_tabletype () -> ()
110-
# FIXME: Error location should be at operand.
111-
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
104+
# FIXME: Error location should be at beginning of operand.
105+
# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
112106
table.get foo
113107
end_function
114108

@@ -129,15 +123,14 @@ table_get_type_mismatch:
129123

130124
table_set_expected_expression_operand:
131125
.functype table_set_expected_expression_operand () -> ()
132-
# FIXME: Error location should be at operand.
133-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
126+
# CHECK: :[[@LINE+1]]:13: error: expected expression operand
134127
table.set 1
135128
end_function
136129

137130
table_set_missing_tabletype:
138131
.functype table_set_missing_tabletype () -> ()
139-
# FIXME: Error location should be at operand.
140-
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
132+
# FIXME: Error location should be at beginning ofoperand.
133+
# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
141134
table.set foo
142135
end_function
143136

@@ -171,15 +164,14 @@ table_set_type_mismatch_2:
171164

172165
table_fill_expected_expression_operand:
173166
.functype table_fill_expected_expression_operand () -> ()
174-
# FIXME: Error location should be at operand.
175-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
167+
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
176168
table.fill 1
177169
end_function
178170

179171
table_fill_missing_tabletype:
180172
.functype table_fill_missing_tabletype () -> ()
181-
# FIXME: Error location should be at operand.
182-
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
173+
# FIXME: Error location should be at beginning of operand.
174+
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .tabletype
183175
table.fill foo
184176
end_function
185177

@@ -397,8 +389,7 @@ return_call_indirect_empty_stack_while_popping_2:
397389

398390
call_expected_expression_operand:
399391
.functype call_expected_expression_operand () -> ()
400-
# FIXME: Error location should be at operand.
401-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
392+
# CHECK: :[[@LINE+1]]:8: error: expected expression operand
402393
call 1
403394
end_function
404395

@@ -427,14 +418,14 @@ call_superfluous_value_at_end:
427418

428419
call_missing_functype:
429420
.functype call_missing_functype () -> ()
430-
# CHECK: :[[@LINE+1]]:3: error: symbol no_functype missing .functype
421+
# FIXME: Error location should be at beginning of operand.
422+
# CHECK: :[[@LINE+1]]:19: error: symbol no_functype missing .functype
431423
call no_functype
432424
end_function
433425

434426
return_call_expected_expression_operand:
435427
.functype return_call_expected_expression_operand () -> ()
436-
# FIXME: Error location should be at operand.
437-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
428+
# CHECK: :[[@LINE+1]]:15: error: expected expression operand
438429
return_call 1
439430
end_function
440431

@@ -453,25 +444,24 @@ return_call_type_mismatch:
453444

454445
return_call_missing_functype:
455446
.functype return_call_missing_functype () -> ()
456-
# FIXME: Error location should be at operand.
457-
# CHECK: :[[@LINE+1]]:3: error: symbol no_functype missing .functype
447+
# FIXME: Error location should be at beginning of operand.
448+
# CHECK: :[[@LINE+1]]:26: error: symbol no_functype missing .functype
458449
return_call no_functype
459450
end_function
460451

461452
catch_expected_expression_operand:
462453
.functype catch_expected_expression_operand () -> ()
463454
try
464-
# FIXME: Error location should be at operand.
465-
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
455+
# CHECK: :[[@LINE+1]]:9: error: expected expression operand
466456
catch 1
467457
end_try
468458
end_function
469459

470460
catch_missing_tagtype:
471461
.functype catch_missing_tagtype () -> ()
472462
try
473-
# FIXME: Error location should be at operand.
474-
# CHECK: :[[@LINE+1]]:3: error: symbol no_tagtype missing .tagtype
463+
# FIXME: Error location should be at beginning of operand.
464+
# CHECK: :[[@LINE+1]]:19: error: symbol no_tagtype missing .tagtype
475465
catch no_tagtype
476466
end_try
477467
end_function

0 commit comments

Comments
 (0)