Skip to content

Commit 48cf047

Browse files
Samuel GroßV8-internal LUCI CQ
Samuel Groß
authored and
V8-internal LUCI CQ
committed
Improve handling of block bodies in parser
We now have a common helper function (visitBody) to process the body of various statements instead of repeating the same code fragment. Similarly, we now also have a common visitParameters function. Finally, this change also adds a test to make sure that we don't introduce additional block statements due to the way statement bodies are represented in the AST. See the comment in the test for more details. Change-Id: Id494669c02ec36132daf7327bf2654fc21e13e0e Reviewed-on: https://chrome-internal-review.googlesource.com/c/v8/fuzzilli/+/7934187 Reviewed-by: Carl Smith <[email protected]> Commit-Queue: Samuel Groß <[email protected]>
1 parent 50d2fcc commit 48cf047

File tree

3 files changed

+104
-30
lines changed

3 files changed

+104
-30
lines changed

Diff for: Sources/Fuzzilli/Compiler/Compiler.swift

+1-1
Original file line numberDiff line numberDiff line change
@@ -915,7 +915,7 @@ public class JavaScriptCompiler {
915915
guard let body = arrowFunction.body else { throw CompilerError.invalidNodeError("missing body in arrow function") }
916916
switch body {
917917
case .block(let block):
918-
try compileStatement(block)
918+
try compileBody(block)
919919
case .expression(let expr):
920920
let result = try compileExpression(expr)
921921
emit(Return(hasReturnValue: true), withInputs: [result])

Diff for: Sources/Fuzzilli/Compiler/Parser/parser.js

+39-29
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ function tryReadFile(path) {
3434

3535
// Parse the given JavaScript script and return an AST compatible with Fuzzilli's protobuf-based AST format.
3636
function parse(script, proto) {
37-
let ast = Parser.parse(script, { plugins: ["v8intrinsic"] });
38-
37+
let ast = Parser.parse(script, { plugins: ["v8intrinsic"] });
38+
3939
function assertNoError(err) {
4040
if (err) throw err;
4141
}
@@ -77,6 +77,20 @@ function parse(script, proto) {
7777
return make('Parameter', { name: param.name });
7878
}
7979

80+
function visitParameters(params) {
81+
return params.map(visitParameter)
82+
}
83+
84+
// Processes the body of a block statement node and returns a list of statements.
85+
function visitBody(node) {
86+
assert(node.type === 'BlockStatement', "Expected block statement, found " + node.type);
87+
let statements = [];
88+
for (let stmt of node.body) {
89+
statements.push(visitStatement(stmt));
90+
}
91+
return statements;
92+
}
93+
8094
function visitVariableDeclaration(node) {
8195
let kind;
8296
if (node.kind === "var") {
@@ -102,22 +116,18 @@ function parse(script, proto) {
102116
return { kind, declarations };
103117
}
104118

105-
106119
function visitStatement(node) {
107120
switch (node.type) {
108121
case 'EmptyStatement': {
109122
return makeStatement('EmptyStatement', {});
110123
}
111124
case 'BlockStatement': {
112-
let body = [];
113-
for (let stmt of node.body) {
114-
body.push(visitStatement(stmt));
115-
}
116-
return makeStatement('BlockStatement', {body});
125+
let body = visitBody(node);
126+
return makeStatement('BlockStatement', { body });
117127
}
118128
case 'ExpressionStatement': {
119-
let expr = visitExpression(node.expression);
120-
return makeStatement('ExpressionStatement', {expression: expr});
129+
let expression = visitExpression(node.expression);
130+
return makeStatement('ExpressionStatement', { expression });
121131
}
122132
case 'VariableDeclaration': {
123133
return makeStatement('VariableDeclaration', visitVariableDeclaration(node));
@@ -133,9 +143,9 @@ function parse(script, proto) {
133143
} else if (node.async) {
134144
type = 2; //"ASYNC";
135145
}
136-
let parameters = node.params.map(visitParameter);
146+
let parameters = visitParameters(node.params);
137147
assert(node.body.type === 'BlockStatement', "Expected block statement as function declaration body, found " + node.body.type);
138-
let body = node.body.body.map(visitStatement);
148+
let body = visitBody(node.body);
139149
return makeStatement('FunctionDeclaration', { name, type, parameters, body });
140150
}
141151
case 'ClassDeclaration': {
@@ -182,29 +192,29 @@ function parse(script, proto) {
182192
assert(name === 'constructor', "Expected name to be exactly 'constructor'");
183193
assert(!isStatic, "Expected isStatic to be false");
184194

185-
let parameters = method.params.map(visitParameter);
186-
let body = method.body.body.map(visitStatement);
195+
let parameters = visitParameters(method.params);
196+
let body = visitBody(method.body);
187197
field.ctor = make('ClassConstructor', { parameters, body });
188198
} else if (method.kind === 'method') {
189199
assert(method.body.type === 'BlockStatement', "Expected method.body.type to be exactly 'BlockStatement'");
190200

191-
let parameters = method.params.map(visitParameter);
192-
let body = method.body.body.map(visitStatement);
201+
let parameters = visitParameters(method.params);
202+
let body = visitBody(method.body);
193203
field.method = make('ClassMethod', { name, isStatic, parameters, body });
194204
} else if (method.kind === 'get') {
195205
assert(method.params.length === 0, "Expected method.params.length to be exactly 0");
196206
assert(!method.generator && !method.async, "Expected both conditions to hold: !method.generator and !method.async");
197207
assert(method.body.type === 'BlockStatement', "Expected method.body.type to be exactly 'BlockStatement'");
198208

199-
let body = method.body.body.map(visitStatement);
209+
let body = visitBody(method.body);
200210
field.getter = make('ClassGetter', { name, isStatic, body });
201211
} else if (method.kind === 'set') {
202212
assert(method.params.length === 1, "Expected method.params.length to be exactly 1");
203213
assert(!method.generator && !method.async, "Expected both conditions to hold: !method.generator and !method.async");
204214
assert(method.body.type === 'BlockStatement', "Expected method.body.type to be exactly 'BlockStatement'");
205215

206216
let parameter = visitParameter(method.params[0]);
207-
let body = method.body.body.map(visitStatement);
217+
let body = visitBody(method.body);
208218
field.setter = make('ClassSetter', { name, isStatic, parameter, body });
209219
} else {
210220
throw "Unknown method kind: " + method.kind;
@@ -299,7 +309,7 @@ function parse(script, proto) {
299309
case 'TryStatement': {
300310
assert(node.block.type === 'BlockStatement', "Expected block statement as body of a try block");
301311
let tryStatement = {}
302-
tryStatement.body = node.block.body.map(visitStatement);
312+
tryStatement.body = visitBody(node.block);
303313
assert(node.handler !== null || node.finalizer !== null, "TryStatements require either a handler or a finalizer (or both)")
304314
if (node.handler !== null) {
305315
assert(node.handler.type === 'CatchClause', "Expected catch clause as try handler");
@@ -308,13 +318,13 @@ function parse(script, proto) {
308318
if (node.handler.param !== null) {
309319
catchClause.parameter = visitParameter(node.handler.param);
310320
}
311-
catchClause.body = node.handler.body.body.map(visitStatement);
321+
catchClause.body = visitBody(node.handler.body);
312322
tryStatement.catch = make('CatchClause', catchClause);
313323
}
314324
if (node.finalizer !== null) {
315325
assert(node.finalizer.type === 'BlockStatement', "Expected block statement as body of finally block");
316326
let finallyClause = {};
317-
finallyClause.body = node.finalizer.body.map(visitStatement);
327+
finallyClause.body = visitBody(node.finalizer);
318328
tryStatement.finally = make('FinallyClause', finallyClause);
319329
}
320330
return makeStatement('TryStatement', tryStatement);
@@ -442,23 +452,23 @@ function parse(script, proto) {
442452
} else if (method.async) {
443453
out.type = 2; //"ASYNC";
444454
}
445-
out.parameters = method.params.map(visitParameter);
446-
out.body = method.body.body.map(visitStatement);
455+
out.parameters = visitParameters(method.params);
456+
out.body = visitBody(method.body);
447457
field.method = make('ObjectMethod', out);
448458
} else if (method.kind === 'get') {
449459
assert(method.params.length === 0, "Expected method.params.length to be exactly 0");
450460
assert(!method.generator && !method.async, "Expected both conditions to hold: !method.generator and !method.async");
451461
assert(method.body.type === 'BlockStatement', "Expected method.body.type to be exactly 'BlockStatement'");
452462

453-
out.body = method.body.body.map(visitStatement);
463+
out.body = visitBody(method.body);
454464
field.getter = make('ObjectGetter', out);
455465
} else if (method.kind === 'set') {
456466
assert(method.params.length === 1, "Expected method.params.length to be exactly 1");
457467
assert(!method.generator && !method.async, "Expected both conditions to hold: !method.generator and !method.async");
458468
assert(method.body.type === 'BlockStatement', "Expected method.body.type to be exactly 'BlockStatement'");
459469

460470
out.parameter = visitParameter(method.params[0]);
461-
out.body = method.body.body.map(visitStatement);
471+
out.body = visitBody(method.body);
462472
field.setter = make('ObjectSetter', out);
463473
} else {
464474
throw "Unknown method kind: " + method.kind;
@@ -489,9 +499,9 @@ function parse(script, proto) {
489499
} else if (node.async) {
490500
type = 2; //"ASYNC";
491501
}
492-
let parameters = node.params.map(visitParameter);
502+
let parameters = visitParameters(node.params);
493503
assert(node.body.type === 'BlockStatement', "Expected block statement as function expression body, found " + node.body.type);
494-
let body = node.body.body.map(visitStatement);
504+
let body = visitBody(node.body);
495505
return makeExpression('FunctionExpression', { type, parameters, body });
496506
}
497507
case 'ArrowFunctionExpression': {
@@ -501,7 +511,7 @@ function parse(script, proto) {
501511
if (node.async) {
502512
type = 2; //"ASYNC";
503513
}
504-
let parameters = node.params.map(visitParameter);
514+
let parameters = visitParameters(node.params);
505515
let out = { type, parameters };
506516
if (node.body.type === 'BlockStatement') {
507517
out.block = visitStatement(node.body);
@@ -621,7 +631,7 @@ protobuf.load(astProtobufDefinitionPath, function(err, root) {
621631

622632
// Uncomment this to print the AST to stdout (will be very verbose).
623633
//console.log(JSON.stringify(ast, null, 2));
624-
634+
625635
const AST = root.lookupType('compiler.protobuf.AST');
626636
let buffer = AST.encode(ast).finish();
627637

Diff for: Tests/FuzzilliTests/CompilerTests/no_added_blocks.js

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
if (typeof output === 'undefined') output = console.log;
2+
3+
// This tests makes sure that we don't create additional block statements during compilation.
4+
// For example, a typical AST for an if statement (ignoring the condition) would look like this:
5+
//
6+
// IfStatement
7+
// |
8+
// BlockStatement
9+
// / | \
10+
// Foo Bar Baz
11+
//
12+
// In that case, we want to generate the following IL code:
13+
//
14+
// BeginIf
15+
// Foo
16+
// Bar
17+
// Baz
18+
// EndIf
19+
//
20+
// And not
21+
//
22+
// BeginIf
23+
// BeginBlock
24+
// Foo
25+
// Bar
26+
// Baz
27+
// EndBlock
28+
// EndIf
29+
//
30+
function test() {
31+
function f1() {}
32+
function* f2() {}
33+
async function f3() {}
34+
let f4 = () => {};
35+
{}
36+
if (true) {}
37+
else {}
38+
for (let i = 0; i < 1; i++) {}
39+
for (let p of {}) {}
40+
for (let p in {}) {}
41+
while (false) {}
42+
do {} while (false);
43+
try {} catch (e) {} finally {}
44+
with ({}) {}
45+
let o = {
46+
m() {},
47+
get a() {},
48+
set a(v) {}
49+
};
50+
class C {
51+
constructor() {}
52+
m() {}
53+
get a() {}
54+
set a(v) {}
55+
static n() {}
56+
static get b() {}
57+
static set b(v) {}
58+
static {}
59+
}
60+
}
61+
62+
let source = test.toString();
63+
let num_braces = source.split('{').length - 1;
64+
output(num_braces);

0 commit comments

Comments
 (0)