Skip to content

Commit

Permalink
Add evaluateWithPossibleThrowCompletion
Browse files Browse the repository at this point in the history
I'm going to need to add a lot of these things that might throw so I wanted
to start preparing the infrastructure for fixing facebookarchive#1264.

The idea is that code inside this scope might emit temporal operations that
might throw. In the future we can wrap these in a try/catch to extract a
caught error and a flag. These can be used to generate a
PossiblyNormalCompletion.

However, for now I just keep the existing strategy of erroring in this case
while I add more callers of this helper.
  • Loading branch information
sebmarkbage committed Jan 17, 2018
1 parent 9190b26 commit bc6340b
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 30 deletions.
17 changes: 5 additions & 12 deletions src/evaluators/CallExpression.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { AbruptCompletion, PossiblyNormalCompletion } from "../completions.js";
import type { Realm } from "../realm.js";
import type { LexicalEnvironment } from "../environment.js";
import { EnvironmentRecord } from "../environment.js";
import { TypesDomain, ValuesDomain } from "../domains/index.js";
import { Value } from "../values/index.js";
import { AbstractValue, BooleanValue, ConcreteValue, FunctionValue } from "../values/index.js";
import { Reference } from "../environment.js";
Expand Down Expand Up @@ -139,8 +140,7 @@ function EvaluateCall(
if (!Value.isTypeCompatibleWith(func.getType(), FunctionValue)) {
if (!realm.isInPureScope()) {
// If this is not a function, this call might throw which can change the state of the program.
// If this is called from a pure function it is safe, unless in a try/catch which has a
// separate error below.
// If this is called from a pure function we handlie it using evaluateWithPossiblyAbruptCompletion.
let error = new CompilerDiagnostic("might not be a function", loc, "PP0005", "RecoverableError");
if (realm.handleError(error) === "Fail") throw new FatalError();
}
Expand All @@ -149,16 +149,9 @@ function EvaluateCall(
} else {
// Assume that it is a safe function. TODO #705: really?
}
if (realm.isInPureTryStatement) {
// TODO(1264): We should be able to preserve error handling on abstract throw
// but currently we just issue a recoverable error instead.
let diag = new CompilerDiagnostic(
"Possibly throwing function call inside try/catch",
loc,
"PP0021",
"RecoverableError"
);
if (realm.handleError(diag) !== "Recover") throw new FatalError();
if (realm.isInPureScope()) {
// In pure functions we allow abstract functions to throw, which this might.
return realm.evaluateWithPossibleThrowCompletion(generateRuntimeCall, TypesDomain.topVal, ValuesDomain.topVal);
}
return generateRuntimeCall();
}
Expand Down
28 changes: 28 additions & 0 deletions src/realm.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
UndefinedValue,
Value,
} from "./values/index.js";
import type { TypesDomain, ValuesDomain } from "./domains/index.js";
import { LexicalEnvironment, Reference, GlobalEnvironmentRecord, DeclarativeEnvironmentRecord } from "./environment.js";
import type { Binding } from "./environment.js";
import { cloneDescriptor, Construct } from "./methods/index.js";
Expand Down Expand Up @@ -427,6 +428,33 @@ export class Realm {
return !!this.createdObjectsTrackedForLeaks;
}

// Evaluate some code that might generate temporal values knowing that it might end in an abrupt
// completion. We only need to support ThrowCompletion for now but this can be expanded to support other
// abrupt completions.
evaluateWithPossibleThrowCompletion(f: () => Value, thrownTypes: TypesDomain, thrownValues: ValuesDomain): Value {
// The cases when we need this are only when we might invoke unknown code such as abstract
// funtions, getters, custom coercion etc. It is possible we can use this in other cases
// where something might throw a built-in error but can never issue arbitrary code such as
// calling something that might not be a function. For now we only use it in pure functions.
invariant(this.isInPureScope(), "only abstract abrupt completion in pure functions");

// TODO(1264): We should create a new generator for this scope and wrap it in a try/catch.
// We could use the outcome of that as the join condition for a PossiblyNormalCompletion.
// We should then compose that with the saved completion and move on to the normal route.
// Currently we just issue a recoverable error instead if this might matter.
let value = f();
if (this.isInPureTryStatement) {
let diag = new CompilerDiagnostic(
"Possible throw inside try/catch is not yet supported",
this.currentLocation,
"PP0021",
"RecoverableError"
);
if (this.handleError(diag) !== "Recover") throw new FatalError();
}
return value;
}

// Evaluate the given ast in a sandbox and return the evaluation results
// in the form of a completion, a code generator, a map of changed variable
// bindings and a map of changed property bindings.
Expand Down
29 changes: 13 additions & 16 deletions src/values/AbstractObjectValue.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,22 +313,7 @@ export default class AbstractObjectValue extends AbstractValue {
$Get(P: PropertyKeyValue, Receiver: Value): Value {
if (P instanceof StringValue) P = P.value;
if (this.values.isTop()) {
if ((this.isSimpleObject() && this.isIntrinsic()) || this.$Realm.isInPureScope()) {
if (!this.isSimpleObject()) {
if (this.$Realm.isInPureTryStatement) {
// TODO(1264): We should be able to preserve error handling on abstract throw
// but currently we just issue a recoverable error instead.
let diag = new CompilerDiagnostic(
"Possibly throwing getter inside try/catch",
this.$Realm.currentLocation,
"PP0021",
"RecoverableError"
);
if (this.$Realm.handleError(diag) !== "Recover") throw new FatalError();
}
// This object might have leaked to a getter.
Leak.leakValue(this.$Realm, this);
}
let generateAbstractGet = () => {
let type = Value;
if (P === "length" && Value.isTypeCompatibleWith(this.getType(), ArrayValue)) type = NumberValue;
let object = this.kind === "sentinel ToObject" ? this.args[0] : this;
Expand All @@ -344,6 +329,18 @@ export default class AbstractObjectValue extends AbstractValue {
skipInvariant: true,
}
);
};
if (this.isSimpleObject() && this.isIntrinsic()) {
return generateAbstractGet();
} else if (this.$Realm.isInPureScope()) {
// This object might have leaked to a getter.
Leak.leakValue(this.$Realm, this);
// The getter might throw anything.
return this.$Realm.evaluateWithPossibleThrowCompletion(
generateAbstractGet,
TypesDomain.topVal,
ValuesDomain.topVal
);
}
AbstractValue.reportIntrospectionError(this, P);
throw new FatalError();
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/try-and-access-abstract-property.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// additional functions
// abstract effects
// recover-from-errors
// expected errors: [{location: {"start":{"line":14,"column":11},"end":{"line":14,"column":15},"identifierName":"obj1","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing getter inside try/catch"},{location: {"start":{"line":22,"column":11},"end":{"line":22,"column":15},"identifierName":"obj2","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing getter inside try/catch"}]
// expected errors: [{location: {"start":{"line":14,"column":11},"end":{"line":14,"column":15},"identifierName":"obj1","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"},{location: {"start":{"line":22,"column":11},"end":{"line":22,"column":15},"identifierName":"obj2","source":"test/error-handler/try-and-access-abstract-property.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"}]

let obj1 = global.__abstract ? __abstract('object', '({get foo() { return "bar"; }})') : {get foo() { return "bar"; }};
let obj2 = global.__abstract ? __abstract('object', '({foo:{bar:"baz"}})') : {foo:{bar:"baz"}};
Expand Down
2 changes: 1 addition & 1 deletion test/error-handler/try-and-call-abstract-function.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// additional functions
// abstract effects
// expected errors: [{location: {"start":{"line":26,"column":11},"end":{"line":26,"column":21},"identifierName":"abstractFn","source":"test/error-handler/try-and-call-abstract-function.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possibly throwing function call inside try/catch"}]
// expected errors: [{location: {"start":{"line":26,"column":11},"end":{"line":26,"column":21},"identifierName":"abstractFn","source":"test/error-handler/try-and-call-abstract-function.js"}, errorCode: "PP0021", severity: "RecoverableError", message: "Possible throw inside try/catch is not yet supported"}]

let abstractFn = global.__abstract ? __abstract('function', '(function() { return true; })') : function() { return true; };

Expand Down

0 comments on commit bc6340b

Please sign in to comment.