Skip to content

Commit

Permalink
[WebAssembly] Use location of operand for operand-based type check er…
Browse files Browse the repository at this point in the history 
…rors

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
  • Loading branch information
@asb
asb committed Mar 23, 2022
1 parent 5bcc90e commit 0126375
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 56 deletions.
2 changes: 1 addition & 1 deletion llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
View file
Expand Up @@ -1017,7 +1017,7 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
Inst.setOpcode(Opc64);
}
}
if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst))
if (!SkipTypeCheck && TC.typeCheck(IDLoc, Inst, Operands))
return true;
Out.emitInstruction(Inst, getSTI());
if (CurrentState == EndFunction) {
Expand Down
33 changes: 18 additions & 15 deletions llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.cpp
View file
Expand Up @@ -210,50 +210,51 @@ bool WebAssemblyAsmTypeCheck::endOfFunction(SMLoc ErrorLoc) {
return false;
}

bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst) {
bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst,
OperandVector &Operands) {
auto Opc = Inst.getOpcode();
auto Name = GetMnemonic(Opc);
dumpTypeStack("typechecking " + Name + ": ");
wasm::ValType Type;
if (Name == "local.get") {
if (getLocal(ErrorLoc, Inst, Type))
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
return true;
Stack.push_back(Type);
} else if (Name == "local.set") {
if (getLocal(ErrorLoc, Inst, Type))
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, Type))
return true;
} else if (Name == "local.tee") {
if (getLocal(ErrorLoc, Inst, Type))
if (getLocal(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, Type))
return true;
Stack.push_back(Type);
} else if (Name == "global.get") {
if (getGlobal(ErrorLoc, Inst, Type))
if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
return true;
Stack.push_back(Type);
} else if (Name == "global.set") {
if (getGlobal(ErrorLoc, Inst, Type))
if (getGlobal(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, Type))
return true;
} else if (Name == "table.get") {
if (getTable(ErrorLoc, Inst, Type))
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, wasm::ValType::I32))
return true;
Stack.push_back(Type);
} else if (Name == "table.set") {
if (getTable(ErrorLoc, Inst, Type))
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, Type))
return true;
if (popType(ErrorLoc, wasm::ValType::I32))
return true;
} else if (Name == "table.fill") {
if (getTable(ErrorLoc, Inst, Type))
if (getTable(Operands[1]->getStartLoc(), Inst, Type))
return true;
if (popType(ErrorLoc, wasm::ValType::I32))
return true;
Expand Down Expand Up @@ -281,25 +282,27 @@ bool WebAssemblyAsmTypeCheck::typeCheck(SMLoc ErrorLoc, const MCInst &Inst) {
return true;
} else if (Name == "call" || Name == "return_call") {
const MCSymbolRefExpr *SymRef;
if (getSymRef(ErrorLoc, Inst, SymRef))
if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
return true;
auto WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
auto Sig = WasmSym->getSignature();
if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_FUNCTION)
return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
" missing .functype");
return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
WasmSym->getName() +
" missing .functype");
if (checkSig(ErrorLoc, *Sig)) return true;
if (Name == "return_call" && endOfFunction(ErrorLoc))
return true;
} else if (Name == "catch") {
const MCSymbolRefExpr *SymRef;
if (getSymRef(ErrorLoc, Inst, SymRef))
if (getSymRef(Operands[1]->getStartLoc(), Inst, SymRef))
return true;
const auto *WasmSym = cast<MCSymbolWasm>(&SymRef->getSymbol());
const auto *Sig = WasmSym->getSignature();
if (!Sig || WasmSym->getType() != wasm::WASM_SYMBOL_TYPE_TAG)
return typeError(ErrorLoc, StringRef("symbol ") + WasmSym->getName() +
" missing .tagtype");
return typeError(Operands[1]->getStartLoc(), StringRef("symbol ") +
WasmSym->getName() +
" missing .tagtype");
// catch instruction pushes values whose types are specified in the tag's
// "params" part
Stack.insert(Stack.end(), Sig->Params.begin(), Sig->Params.end());
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmTypeCheck.h
View file
Expand Up @@ -16,9 +16,10 @@
#ifndef LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H
#define LLVM_LIB_TARGET_WEBASSEMBLY_ASMPARSER_TYPECHECK_H

#include "llvm/MC/MCParser/MCAsmParser.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/BinaryFormat/Wasm.h"
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/MC/MCParser/MCAsmParser.h"
#include "llvm/MC/MCParser/MCTargetAsmParser.h"
#include "llvm/MC/MCSymbol.h"

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

void Clear() {
Stack.clear();
Expand Down
64 changes: 27 additions & 37 deletions llvm/test/MC/WebAssembly/type-checker-errors.s
View file
Expand Up @@ -6,15 +6,13 @@

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

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

Expand All @@ -35,8 +33,7 @@ local_set_type_mismatch:

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

Expand All @@ -57,29 +54,27 @@ local_tee_type_mismatch:

global_get_missing_globaltype:
.functype global_get_missing_globaltype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
global.get foo
end_function

global_get_expected_expression_operand:
.functype global_get_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
global.get 1
end_function

global_set_missing_globaltype:
.functype global_set_missing_globaltype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .globaltype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .globaltype
global.set foo
end_function

global_set_expected_expression_operand:
.functype global_set_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
global.set 1
end_function

Expand All @@ -100,15 +95,14 @@ global_set_type_mismatch:

table_get_expected_expression_operand:
.functype table_get_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:13: error: expected expression operand
table.get 1
end_function

table_get_missing_tabletype:
.functype table_get_missing_tabletype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
table.get foo
end_function

Expand All @@ -129,15 +123,14 @@ table_get_type_mismatch:

table_set_expected_expression_operand:
.functype table_set_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:13: error: expected expression operand
table.set 1
end_function

table_set_missing_tabletype:
.functype table_set_missing_tabletype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
# FIXME: Error location should be at beginning ofoperand.
# CHECK: :[[@LINE+1]]:16: error: symbol foo missing .tabletype
table.set foo
end_function

Expand Down Expand Up @@ -171,15 +164,14 @@ table_set_type_mismatch_2:

table_fill_expected_expression_operand:
.functype table_fill_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:14: error: expected expression operand
table.fill 1
end_function

table_fill_missing_tabletype:
.functype table_fill_missing_tabletype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol foo missing .tabletype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:17: error: symbol foo missing .tabletype
table.fill foo
end_function

Expand Down Expand Up @@ -397,8 +389,7 @@ return_call_indirect_empty_stack_while_popping_2:

call_expected_expression_operand:
.functype call_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:8: error: expected expression operand
call 1
end_function

Expand Down Expand Up @@ -427,14 +418,14 @@ call_superfluous_value_at_end:

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

return_call_expected_expression_operand:
.functype return_call_expected_expression_operand () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:15: error: expected expression operand
return_call 1
end_function

Expand All @@ -453,25 +444,24 @@ return_call_type_mismatch:

return_call_missing_functype:
.functype return_call_missing_functype () -> ()
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol no_functype missing .functype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:26: error: symbol no_functype missing .functype
return_call no_functype
end_function

catch_expected_expression_operand:
.functype catch_expected_expression_operand () -> ()
try
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: expected expression operand
# CHECK: :[[@LINE+1]]:9: error: expected expression operand
catch 1
end_try
end_function

catch_missing_tagtype:
.functype catch_missing_tagtype () -> ()
try
# FIXME: Error location should be at operand.
# CHECK: :[[@LINE+1]]:3: error: symbol no_tagtype missing .tagtype
# FIXME: Error location should be at beginning of operand.
# CHECK: :[[@LINE+1]]:19: error: symbol no_tagtype missing .tagtype
catch no_tagtype
end_try
end_function
Expand Down

0 comments on commit 0126375

Please sign in to comment.