Skip to content

Commit

Permalink
Fix bug in JavaScriptLifter
Browse files Browse the repository at this point in the history 
When explicitly "un-inlining" expressions and storing them in a local
variable, we need to ensure that later uses of the same FuzzIL variable
still use the inlined expression and not the newly created variable as
that may no longer be visible. See the testcase for more details.
  • Loading branch information
Samuel Groß committed Jan 30, 2023
1 parent c10e81e commit 59d96b0
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 20 deletions.
43 changes: 35 additions & 8 deletions Sources/Fuzzilli/Lifting/JavaScriptLifter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -894,9 +894,11 @@ public class JavaScriptLifter: Lifter {

case .beginWhileLoop(let op):
// We should not inline expressions into the loop header as that would change the behavior of the program.
// Instead, we should create a LoopHeader block in which arbitrary expressions can be executed.
let lhs = w.retrieve(identifierFor: instr.input(0))
let rhs = w.retrieve(identifierFor: instr.input(1))
// To achieve that, we force all pending expressions to be emitted now.
// TODO: Instead, we should create a LoopHeader block in which arbitrary expressions can be executed.
w.emitPendingExpressions()
let lhs = w.retrieve(expressionFor: instr.input(0))
let rhs = w.retrieve(expressionFor: instr.input(1))
let COND = BinaryExpression.new() + lhs + " " + op.comparator.token + " " + rhs
w.emit("while (\(COND)) {")
w.enterNewBlock()
Expand All @@ -916,7 +918,7 @@ public class JavaScriptLifter: Lifter {
let lhs = w.retrieve(expressionFor: begin.input(0))
let rhs = w.retrieve(expressionFor: begin.input(1))
let COND = BinaryExpression.new() + lhs + " " + comparator.token + " " + rhs
w.emit("} while (\(COND));")
w.emit("} while (\(COND))")

case .beginForLoop(let op):
let I = w.declare(instr.innerOutput)
Expand Down Expand Up @@ -1252,9 +1254,34 @@ public class JavaScriptLifter: Lifter {
mutating func retrieve(identifierFor v: Variable) -> Expression {
var expr = retrieve(expressionFor: v)
if expr.type !== Identifier {
expressions.removeValue(forKey: v)
let LET = declarationKeyword(for: v)
let V = declare(v)
// When creating a temporary variable for the expression, we're _not_ replacing the existing
// expression with it since we cannot guarantee that the variable will still be visible at
// the next use. Consider the following example:
//
// v0 <- LoadInt(0)
// v1 <- LoadInt(10)
// BerginDoWhileLoop v0, '<', v1
// SetElement v1, '0', v0
// v2 <- Unary v1, '++'
// EndDoWhileLoop
//
// For the SetElement, we force the object to be in a local variable. However, in the do-while loop
// that variable would no longer be visible:
//
// let v0 = 0;
// do {
// const v1 = 10;
// v1[0] = v0;
// v0++;
// } while (v0 < v1)
//
// So instead, in the do-while loop we again need to use the inlined expression (`10`).
let LET = constKeyword
// We use a different naming scheme for these temporary variables since we may end up defining
// them multiple times (if the same expression is "un-inlined" multiple times).
// We could instead remember the existing local variable for as long as it is visible, but it's
// probably not worth the effort.
let V = "t" + String(writer.currentLineNumber)
emit("\(LET) \(V) = \(expr);")
expr = Identifier.new(V)
}
Expand Down Expand Up @@ -1334,7 +1361,7 @@ public class JavaScriptLifter: Lifter {

/// Emit all expressions that are still waiting to be inlined.
/// This is usually used because some other effectful piece of code is about to be emitted, so the pending expression must execute first.
private mutating func emitPendingExpressions() {
mutating func emitPendingExpressions() {
for v in pendingExpressions {
emitPendingExpression(forVariable: v)
}
Expand Down
6 changes: 3 additions & 3 deletions Sources/Fuzzilli/Lifting/ScriptWriter.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct ScriptWriter {
private let includeLineNumbers: Bool

/// Current line, used when including line numbers in the output.
private var curLine = 0
public private(set) var currentLineNumber = 0

public init (stripComments: Bool = false, includeLineNumbers: Bool = false, indent: Int = 4) {
self.indent = String(repeating: " ", count: indent)
Expand All @@ -41,8 +41,8 @@ struct ScriptWriter {
/// Emit one line of code.
mutating func emit<S: StringProtocol>(_ line: S) {
assert(!line.contains("\n"))
curLine += 1
if includeLineNumbers { code += "\(String(format: "%3i", curLine)). " }
currentLineNumber += 1
if includeLineNumbers { code += "\(String(format: "%3i", currentLineNumber)). " }
code += currentIndention + line + "\n"
}

Expand Down
75 changes: 66 additions & 9 deletions Tests/FuzzilliTests/LifterTest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ class LifterTests: XCTestCase {

let expected = """
const v1 = new Object();
const v6 = SomeObj.foo.bar.baz;
v1.r = v6(42, 42);
const t1 = SomeObj.foo.bar.baz;
v1.r = t1(42, 42);
const v9 = Math.random();
SideEffect();
v1.s = v9 + 13.37;
Expand Down Expand Up @@ -257,6 +257,40 @@ class LifterTests: XCTestCase {
XCTAssertEqual(actual, expected)
}

func testExpressionInlining8() {
let fuzzer = makeMockFuzzer()
let b = fuzzer.makeBuilder()

// This test ensures that expression inlining works corretly even when
// expressions are explicitly "un-inlined", for example by a SetElement
// operation where we force the object to be a variable.
let i1 = b.loadInt(0)
let i2 = b.loadInt(10)
b.buildDoWhileLoop(i1, .lessThan, i2) {
// The SetElement will "un-inline" i2, but for the do-while loop we'll still need the inlined expression (`10`).
b.setElement(0, of: i2, to: i1)
b.setElement(1, of: i2, to: i1)
b.unary(.PostInc, i1)
}

let program = b.finalize()
let actual = fuzzer.lifter.lift(program)

let expected = """
let v0 = 0;
do {
const t2 = 10;
t2[0] = v0;
const t4 = 10;
t4[1] = v0;
v0++;
} while (v0 < 10)
"""

XCTAssertEqual(actual, expected)
}

func testObjectLiteralLifting() {
let fuzzer = makeMockFuzzer()
let b = fuzzer.makeBuilder()
Expand Down Expand Up @@ -1014,8 +1048,8 @@ class LifterTests: XCTestCase {
};
delete o3.foo;
delete o3["bar"];
const v10 = [301,4,68,22];
delete v10[3];
const t6 = [301,4,68,22];
delete t6[3];
"""

Expand All @@ -1040,8 +1074,8 @@ class LifterTests: XCTestCase {

let expected = """
eval("print('Hello World!')");
const v4 = this.eval;
v4("print('Hello World!')");
const t1 = this.eval;
t1("print('Hello World!')");
this.eval("print('Hello World!')");
"""
Expand Down Expand Up @@ -1517,7 +1551,30 @@ class LifterTests: XCTestCase {
XCTAssertEqual(actual, expected)
}

func testDoWhileLifting() {
func testWhileLoopLifting() {
let fuzzer = makeMockFuzzer()
let b = fuzzer.makeBuilder()

let loopVar = b.loadInt(0)
b.buildWhileLoop(loopVar, .lessThan, b.loadInt(100)) {
b.unary(.PostInc, loopVar)
}

let program = b.finalize()
let actual = fuzzer.lifter.lift(program)

let expected = """
let v0 = 0;
while (v0 < 100) {
v0++;
}
"""

XCTAssertEqual(actual, expected)
}

func testDoWhileLoopLifting() {
// Do-While loops require special handling as the loop condition is kept
// in BeginDoWhileLoop but only emitted during lifting of EndDoWhileLoop
let fuzzer = makeMockFuzzer()
Expand All @@ -1541,9 +1598,9 @@ class LifterTests: XCTestCase {
let v2 = 0;
do {
v2++;
} while (v2 < 1337);
} while (v2 < 1337)
v0++;
} while (v0 < 42);
} while (v0 < 42)
"""

Expand Down

0 comments on commit 59d96b0

Please sign in to comment.