Bug 1451343 - Part 2: Check annotated closed vars against known uses. (r=Yoric)
authorEric Faust <efaustbmo@gmail.com>
Mon, 30 Apr 2018 23:37:41 -0700
changeset 472557 cf0675c2f36251c5c9871987a02efe4be1e25dc0
parent 472556 03130e40886bf76d8d0de4ce16b7a042fd54b4f4
child 472558 294524718c9af630cee7efc3b80f27f267ccb0c3
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersYoric
bugs1451343
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1451343 - Part 2: Check annotated closed vars against known uses. (r=Yoric)
js/src/frontend/BinSource-auto.cpp
js/src/frontend/BinSource.cpp
js/src/frontend/BinSource.h
js/src/frontend/BinSource.yaml
--- a/js/src/frontend/BinSource-auto.cpp
+++ b/js/src/frontend/BinSource-auto.cpp
@@ -3261,16 +3261,17 @@ BinASTParser<Tok>::parseInterfaceAssignm
 
 
     RootedAtom name(cx_);
     MOZ_TRY_VAR(name, tokenizer_->readAtom());
 
 
     if (!IsIdentifier(name))
         return raiseError("Invalid identifier");
+    BINJS_TRY(usedNames_.noteUse(cx_, name, parseContext_->scriptId(), parseContext_->innermostScope()->id()));
     BINJS_TRY_DECL(result, factory_.newName(name->asPropertyName(), tokenizer_->pos(start), cx_));
     return result;
 }
 
 
 /*
  interface AssignmentTargetPropertyIdentifier : Node {
     AssignmentTargetIdentifier binding;
@@ -3709,16 +3710,17 @@ BinASTParser<Tok>::parseInterfaceBlock(c
     MOZ_TRY(parseOptionalAssertedBlockScope());
 
 
 
 
     BINJS_MOZ_TRY_DECL(statements, parseListOfStatement());
 
 
+    MOZ_TRY(checkClosedVars(currentScope));
     BINJS_TRY_DECL(bindings, NewLexicalScopeData(cx_, currentScope, alloc_, parseContext_));
     BINJS_TRY_DECL(result, factory_.newLexicalScope(*bindings, statements));
     return result;
 }
 
 
 /*
  interface BreakStatement : Node {
@@ -5618,16 +5620,17 @@ BinASTParser<Tok>::parseInterfaceIdentif
 
 
     RootedAtom name(cx_);
     MOZ_TRY_VAR(name, tokenizer_->readAtom());
 
 
     if (!IsIdentifier(name))
         return raiseError("Invalid identifier");
+    BINJS_TRY(usedNames_.noteUse(cx_, name, parseContext_->scriptId(), parseContext_->innermostScope()->id()));
     BINJS_TRY_DECL(result, factory_.newName(name->asPropertyName(), tokenizer_->pos(start), cx_));
     return result;
 }
 
 
 /*
  interface IfStatement : Node {
     Expression test;
@@ -6442,17 +6445,17 @@ BinASTParser<Tok>::parseInterfaceScript(
     BINJS_MOZ_TRY_DECL(directives, parseListOfDirective());
 
 
 
 
     BINJS_MOZ_TRY_DECL(statements, parseListOfStatement());
 
 
-    BINJS_MOZ_TRY_DECL(result, appendDirectivesToBody(/* body = */ statements, /* directives = */ directives));
+    MOZ_TRY(checkClosedVars(parseContext_->varScope())); BINJS_MOZ_TRY_DECL(result, appendDirectivesToBody(/* body = */ statements, /* directives = */ directives));
     return result;
 }
 
 
 /*
  interface ShorthandProperty : Node {
     IdentifierExpression name;
  }
--- a/js/src/frontend/BinSource.cpp
+++ b/js/src/frontend/BinSource.cpp
@@ -149,16 +149,19 @@ BinASTParser<Tok>::buildFunctionBox(Gene
     funbox->initWithEnclosingParseContext(parseContext_, syntax);
     return funbox;
 }
 
 template<typename Tok> JS::Result<ParseNode*>
 BinASTParser<Tok>::buildFunction(const size_t start, const BinKind kind, ParseNode* name,
                                  ParseNode* params, ParseNode* body, FunctionBox* funbox)
 {
+    // Check all our bindings before doing anything else.
+    MOZ_TRY(checkFunctionClosedVars());
+
     TokenPos pos = tokenizer_->pos(start);
 
     funbox->function()->setArgCount(params ? uint16_t(params->pn_count) : 0);
 
     // ParseNode represents the body as concatenated after the params.
     params->appendWithoutOrderAssumption(body);
 
     BINJS_TRY_DECL(result, kind == BinKind::FunctionDeclaration
@@ -280,16 +283,44 @@ BinASTParser<Tok>::checkBinding(JSAtom* 
 
     auto ptr = scope.lookupDeclaredName(name->asPropertyName());
     if (!ptr)
         return raiseMissingVariableInAssertedScope(name);
 
     return Ok();
 }
 
+template<typename Tok> JS::Result<Ok>
+BinASTParser<Tok>::checkClosedVars(ParseContext::Scope& scope)
+{
+    for (ParseContext::Scope::BindingIter bi = scope.bindings(parseContext_); bi; bi++) {
+        if (UsedNamePtr p = usedNames_.lookup(bi.name())) {
+            bool closedOver;
+            p->value().noteBoundInScope(parseContext_->scriptId(), scope.id(), &closedOver);
+            if (closedOver && !bi.closedOver())
+                return raiseInvalidClosedVar(bi.name());
+        }
+    }
+
+    return Ok();
+}
+
+template<typename Tok> JS::Result<Ok>
+BinASTParser<Tok>::checkFunctionClosedVars()
+{
+    MOZ_ASSERT(parseContext_->isFunctionBox());
+
+    MOZ_TRY(checkClosedVars(*parseContext_->innermostScope()));
+    MOZ_TRY(checkClosedVars(parseContext_->functionScope()));
+    if (parseContext_->functionBox()->function()->isNamedLambda())
+        MOZ_TRY(checkClosedVars(parseContext_->namedLambdaScope()));
+
+    return Ok();
+}
+
 template<typename Tok> JS::Result<ParseNode*>
 BinASTParser<Tok>::appendDirectivesToBody(ParseNode* body, ParseNode* directives)
 {
     ParseNode* result = body;
     if (directives && directives->pn_count >= 1) {
         MOZ_ASSERT(directives->isArity(PN_LIST));
 
         // Convert directive list to a list of strings.
@@ -313,16 +344,22 @@ BinASTParser<Tok>::appendDirectivesToBod
         result->checkListConsistency();
 #endif // defined(DEBUG)
     }
 
     return result;
 }
 
 template<typename Tok> mozilla::GenericErrorResult<JS::Error&>
+BinASTParser<Tok>::raiseInvalidClosedVar(JSAtom* name)
+{
+    return raiseError("Captured variable was not declared as captured");
+}
+
+template<typename Tok> mozilla::GenericErrorResult<JS::Error&>
 BinASTParser<Tok>::raiseUndeclaredCapture(JSAtom* name)
 {
     // As below, don't put the name in a message.
     return raiseError("Captured variable undeclared in scope");
 }
 
 template<typename Tok> mozilla::GenericErrorResult<JS::Error&>
 BinASTParser<Tok>::raiseMissingVariableInAssertedScope(JSAtom* name)
--- a/js/src/frontend/BinSource.h
+++ b/js/src/frontend/BinSource.h
@@ -153,16 +153,17 @@ class BinASTParser : public BinASTParser
   private:
     MOZ_MUST_USE JS::Result<ParseNode*> parseAux(const uint8_t* start, const size_t length);
 
     // --- Raise errors.
     //
     // These methods return a (failed) JS::Result for convenience.
 
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseUndeclaredCapture(JSAtom* name);
+    MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseInvalidClosedVar(JSAtom* name);
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseMissingVariableInAssertedScope(JSAtom* name);
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseMissingDirectEvalInAssertedScope();
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseInvalidKind(const char* superKind,
         const BinKind kind);
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseInvalidVariant(const char* kind,
         const BinVariant value);
     MOZ_MUST_USE mozilla::GenericErrorResult<JS::Error&> raiseMissingField(const char* kind,
         const BinField field);
@@ -175,31 +176,39 @@ class BinASTParser : public BinASTParser
 
     // Ensure that this parser will never be used again.
     void poison();
 
     // Auto-generated methods
 #include "frontend/BinSource-auto.h"
 
     // --- Auxiliary parsing functions
+
+    // Build a function object for a function-producing production. Called AFTER creating the scope.
     JS::Result<ParseNode*>
     buildFunction(const size_t start, const BinKind kind, ParseNode* name, ParseNode* params,
         ParseNode* body, FunctionBox* funbox);
     JS::Result<FunctionBox*>
     buildFunctionBox(GeneratorKind generatorKind, FunctionAsyncKind functionAsyncKind, FunctionSyntaxKind syntax, ParseNode* name);
 
     // Parse full scope information to a specific var scope / let scope combination.
     MOZ_MUST_USE JS::Result<Ok> parseAndUpdateScope(ParseContext::Scope& varScope,
         ParseContext::Scope& letScope);
     // Parse a list of names and add it to a given scope.
     MOZ_MUST_USE JS::Result<Ok> parseAndUpdateScopeNames(ParseContext::Scope& scope,
         DeclarationKind kind);
     MOZ_MUST_USE JS::Result<Ok> parseAndUpdateCapturedNames(const BinKind kind);
     MOZ_MUST_USE JS::Result<Ok> checkBinding(JSAtom* name);
 
+    // When leaving a scope, check that none of its bindings are known closed over and un-marked.
+    MOZ_MUST_USE JS::Result<Ok> checkClosedVars(ParseContext::Scope& scope);
+
+    // As a convenience, a helper that checks the body, parameter, and recursive binding scopes.
+    MOZ_MUST_USE JS::Result<Ok> checkFunctionClosedVars();
+
     // --- Utilities.
 
     MOZ_MUST_USE JS::Result<ParseNode*> appendDirectivesToBody(ParseNode* body,
         ParseNode* directives);
 
   private: // Implement ErrorReporter
     const ReadOnlyCompileOptions& options_;
 
--- a/js/src/frontend/BinSource.yaml
+++ b/js/src/frontend/BinSource.yaml
@@ -247,16 +247,17 @@ AssertedVarScope:
 AssignmentExpression:
     build: |
         BINJS_TRY_DECL(result, factory_.newAssignment(ParseNodeKind::Assign, binding, expression));
 
 AssignmentTargetIdentifier:
     build: |
         if (!IsIdentifier(name))
             return raiseError("Invalid identifier");
+        BINJS_TRY(usedNames_.noteUse(cx_, name, parseContext_->scriptId(), parseContext_->innermostScope()->id()));
         BINJS_TRY_DECL(result, factory_.newName(name->asPropertyName(), tokenizer_->pos(start), cx_));
 
 BindingIdentifier:
     build: |
         if (!IsIdentifier(name))
             return raiseError("Invalid identifier");
         BINJS_TRY_DECL(result, factory_.newName(name->asPropertyName(), tokenizer_->pos(start), cx_));
 
@@ -357,16 +358,17 @@ BinaryExpression:
         }
 
 Block:
     init: |
         ParseContext::Statement stmt(parseContext_, StatementKind::Block);
         ParseContext::Scope currentScope(cx_, parseContext_, usedNames_);
         BINJS_TRY(currentScope.init(parseContext_));
     build: |
+        MOZ_TRY(checkClosedVars(currentScope));
         BINJS_TRY_DECL(bindings, NewLexicalScopeData(cx_, currentScope, alloc_, parseContext_));
         BINJS_TRY_DECL(result, factory_.newLexicalScope(*bindings, statements));
 
 BreakStatement:
     fields:
         label:
             block:
                 replace: |
@@ -682,16 +684,17 @@ FunctionBody:
     build: |
         BINJS_MOZ_TRY_DECL(result, appendDirectivesToBody(/* body = */ statements, /* directives = */ directives));
 
 
 IdentifierExpression:
     build: |
         if (!IsIdentifier(name))
             return raiseError("Invalid identifier");
+        BINJS_TRY(usedNames_.noteUse(cx_, name, parseContext_->scriptId(), parseContext_->innermostScope()->id()));
         BINJS_TRY_DECL(result, factory_.newName(name->asPropertyName(), tokenizer_->pos(start), cx_));
 
 IfStatement:
     build: |
         BINJS_TRY_DECL(result, factory_.newIfStatement(start, test, consequent, alternate));
 
 LabelledStatement:
     fields:
@@ -848,16 +851,17 @@ ReturnStatement:
         }
 
         parseContext_->functionBox()->usesReturn = true;
     build:
         BINJS_TRY_DECL(result, factory_.newReturnStatement(expression, tokenizer_->pos(start)));
 
 Script:
     build:
+        MOZ_TRY(checkClosedVars(parseContext_->varScope()));
         BINJS_MOZ_TRY_DECL(result, appendDirectivesToBody(/* body = */ statements, /* directives = */ directives));
 
 Setter:
     inherits: Method
     init: |
         const auto isAsync = false;
         const auto isGenerator = false;
     build: |