Skip to content

Commit 29c6ce5

Browse files
committed
[WebAssembly] Make assembler check for proper nesting of control flow.
Summary: It does so using a simple nesting stack, and gives clear errors upon violation. This is unique to wasm, since most CPUs do not have any nested constructs. Had to add an end of file check to the general assembler for this. Note: if/else/end instructions are not currently supported in our tablegen defs, so these tests will be enabled in a follow-up. They already pass the nesting check. Reviewers: dschuff, aheejin Subscribers: sbc100, jgravelle-google, sunfish, llvm-commits Differential Revision: https://reviews.llvm.org/D55797 llvm-svn: 350078
1 parent 82f43aa commit 29c6ce5

File tree

6 files changed

+151
-5
lines changed

6 files changed

+151
-5
lines changed

llvm/include/llvm/MC/MCParser/MCTargetAsmParser.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -490,6 +490,9 @@ class MCTargetAsmParser : public MCAsmParserExtension {
490490
MCContext &Ctx) {
491491
return nullptr;
492492
}
493+
494+
// For any checks or cleanups at the end of parsing.
495+
virtual void onEndOfFile() {}
493496
};
494497

495498
} // end namespace llvm

llvm/lib/MC/MCParser/AsmParser.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -899,6 +899,9 @@ bool AsmParser::Run(bool NoInitialTextSection, bool NoFinalize) {
899899
eatToEndOfStatement();
900900
}
901901

902+
getTargetParser().onEndOfFile();
903+
printPendingErrors();
904+
902905
// All errors should have been emitted.
903906
assert(!hasPendingError() && "unexpected error from parseStatement");
904907

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

Lines changed: 107 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,18 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
172172
Instructions,
173173
} CurrentState = FileStart;
174174

175+
// For ensuring blocks are properly nested.
176+
enum NestingType {
177+
Function,
178+
Block,
179+
Loop,
180+
Try,
181+
If,
182+
Else,
183+
Undefined,
184+
};
185+
std::vector<NestingType> NestingStack;
186+
175187
// We track this to see if a .functype following a label is the same,
176188
// as this is how we recognize the start of a function.
177189
MCSymbol *LastLabel = nullptr;
@@ -184,10 +196,6 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
184196
setAvailableFeatures(ComputeAvailableFeatures(STI.getFeatureBits()));
185197
}
186198

187-
void addSignature(std::unique_ptr<wasm::WasmSignature> &&Sig) {
188-
Signatures.push_back(std::move(Sig));
189-
}
190-
191199
#define GET_ASSEMBLER_HEADER
192200
#include "WebAssemblyGenAsmMatcher.inc"
193201

@@ -197,10 +205,60 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
197205
llvm_unreachable("ParseRegister is not implemented.");
198206
}
199207

200-
bool error(const StringRef &Msg, const AsmToken &Tok) {
208+
bool error(const Twine &Msg, const AsmToken &Tok) {
201209
return Parser.Error(Tok.getLoc(), Msg + Tok.getString());
202210
}
203211

212+
bool error(const Twine &Msg) {
213+
return Parser.Error(Lexer.getTok().getLoc(), Msg);
214+
}
215+
216+
void addSignature(std::unique_ptr<wasm::WasmSignature> &&Sig) {
217+
Signatures.push_back(std::move(Sig));
218+
}
219+
220+
std::pair<StringRef, StringRef> nestingString(NestingType NT) {
221+
switch (NT) {
222+
case Function:
223+
return {"function", "end_function"};
224+
case Block:
225+
return {"block", "end_block"};
226+
case Loop:
227+
return {"loop", "end_loop"};
228+
case Try:
229+
return {"try", "end_try"};
230+
case If:
231+
return {"if", "end_if"};
232+
case Else:
233+
return {"else", "end_if"};
234+
default:
235+
llvm_unreachable("unknown NestingType");
236+
}
237+
}
238+
239+
void push(NestingType NT) { NestingStack.push_back(NT); }
240+
241+
bool pop(StringRef Ins, NestingType NT1, NestingType NT2 = Undefined) {
242+
if (NestingStack.empty())
243+
return error(Twine("End of block construct with no start: ") + Ins);
244+
auto Top = NestingStack.back();
245+
if (Top != NT1 && Top != NT2)
246+
return error(Twine("Block construct type mismatch, expected: ") +
247+
nestingString(Top).second + ", instead got: " + Ins);
248+
NestingStack.pop_back();
249+
return false;
250+
}
251+
252+
bool ensureEmptyNestingStack() {
253+
auto err = !NestingStack.empty();
254+
while (!NestingStack.empty()) {
255+
error(Twine("Unmatched block construct(s) at function end: ") +
256+
nestingString(NestingStack.back()).first);
257+
NestingStack.pop_back();
258+
}
259+
return err;
260+
}
261+
204262
bool isNext(AsmToken::TokenKind Kind) {
205263
auto Ok = Lexer.is(Kind);
206264
if (Ok)
@@ -327,6 +385,45 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
327385
// If no '.', there is no type prefix.
328386
auto BaseName = NamePair.second.empty() ? NamePair.first : NamePair.second;
329387

388+
// If this instruction is part of a control flow structure, ensure
389+
// proper nesting.
390+
if (BaseName == "block") {
391+
push(Block);
392+
} else if (BaseName == "loop") {
393+
push(Loop);
394+
} else if (BaseName == "try") {
395+
push(Try);
396+
} else if (BaseName == "if") {
397+
push(If);
398+
} else if (BaseName == "else") {
399+
if (pop(BaseName, If))
400+
return true;
401+
push(Else);
402+
} else if (BaseName == "catch") {
403+
if (pop(BaseName, Try))
404+
return true;
405+
push(Try);
406+
} else if (BaseName == "catch_all") {
407+
if (pop(BaseName, Try))
408+
return true;
409+
push(Try);
410+
} else if (BaseName == "end_if") {
411+
if (pop(BaseName, If, Else))
412+
return true;
413+
} else if (BaseName == "end_try") {
414+
if (pop(BaseName, Try))
415+
return true;
416+
} else if (BaseName == "end_loop") {
417+
if (pop(BaseName, Loop))
418+
return true;
419+
} else if (BaseName == "end_block") {
420+
if (pop(BaseName, Block))
421+
return true;
422+
} else if (BaseName == "end_function") {
423+
if (pop(BaseName, Function) || ensureEmptyNestingStack())
424+
return true;
425+
}
426+
330427
while (Lexer.isNot(AsmToken::EndOfStatement)) {
331428
auto &Tok = Lexer.getTok();
332429
switch (Tok.getKind()) {
@@ -476,7 +573,10 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
476573
TOut.getStreamer().getContext().getOrCreateSymbol(SymName));
477574
if (CurrentState == Label && WasmSym == LastLabel) {
478575
// This .functype indicates a start of a function.
576+
if (ensureEmptyNestingStack())
577+
return true;
479578
CurrentState = FunctionStart;
579+
push(Function);
480580
}
481581
auto Signature = make_unique<wasm::WasmSignature>();
482582
if (parseSignature(Signature.get()))
@@ -565,6 +665,8 @@ class WebAssemblyAsmParser final : public MCTargetAsmParser {
565665
}
566666
llvm_unreachable("Implement any new match types added!");
567667
}
668+
669+
void onEndOfFile() override { ensureEmptyNestingStack(); }
568670
};
569671
} // end anonymous namespace
570672

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# RUN: not llvm-mc -triple=wasm32-unknown-unknown -mattr=+simd128,+nontrapping-fptoint,+exception-handling < %s 2>&1 | FileCheck %s
2+
3+
.text
4+
.section .text.main,"",@
5+
.type test0,@function
6+
# CHECK: End of block construct with no start: end_try
7+
end_try
8+
test0:
9+
.functype test0 () -> ()
10+
# CHECK: Block construct type mismatch, expected: end_function, instead got: end_loop
11+
end_loop
12+
block
13+
# CHECK: Block construct type mismatch, expected: end_block, instead got: end_if
14+
end_if
15+
try
16+
loop
17+
# CHECK: Block construct type mismatch, expected: end_loop, instead got: end_function
18+
# CHECK: error: Unmatched block construct(s) at function end: loop
19+
# CHECK: error: Unmatched block construct(s) at function end: try
20+
# CHECK: error: Unmatched block construct(s) at function end: block
21+
# CHECK: error: Unmatched block construct(s) at function end: function
22+
end_function
23+
.Lfunc_end0:
24+
.size test0, .Lfunc_end0-test0
25+

llvm/test/MC/WebAssembly/basic-assembly.s

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ test0:
5959
end_block # default jumps here.
6060
i32.const 3
6161
end_block # "switch" exit.
62+
#if # These are not in tablegen defs yet..
63+
#if
64+
#end_if
65+
#else
66+
#end_if
6267
f32x4.add
6368
# Test correct parsing of instructions with / and : in them:
6469
# TODO: enable once instruction has been added.
@@ -129,6 +134,11 @@ test0:
129134
# CHECK-NEXT: end_block # label3:
130135
# CHECK-NEXT: i32.const 3
131136
# CHECK-NEXT: end_block # label2:
137+
# DONT-CHECK-NEXT: if
138+
# DONT-CHECK-NEXT: if
139+
# DONT-CHECK-NEXT: end_if
140+
# DONT-CHECK-NEXT: else
141+
# DONT-CHECK-NEXT: end_if
132142
# CHECK-NEXT: f32x4.add
133143
# CHECK-NEXT: i32.trunc_s/f32
134144
# CHECK-NEXT: try

llvm/test/MC/WebAssembly/simd-encodings.s

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# RUN: llvm-mc -show-encoding -triple=wasm32-unkown-unknown -mattr=+sign-ext,+simd128 < %s | FileCheck %s
22

3+
main:
4+
.functype main () -> ()
5+
36
# CHECK: v128.load 48:p2align=0 # encoding: [0xfd,0x00,0x00,0x30]
47
v128.load 48
58

0 commit comments

Comments
 (0)