From b9409d87745389c92c4251066c7b7db54b5aa430 Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Sat, 13 Jul 2019 23:20:19 -0400 Subject: [PATCH] 1357506 - Remove assert that constructorBox can only be set once when parsing classes. Both asm.js and syntax parsing can abort and rewind parsing of an inner function. The bookkeeping to make sure that a class's constructor FunctionBox is only set once is not worth it -- duplicate constructor definitions already throw an early error. --- js/src/frontend/Parser.cpp | 17 +++-------------- js/src/frontend/Parser.h | 21 +++------------------ js/src/frontend/SharedContext.h | 1 - js/src/jit-test/tests/class/bug1357506.js | 8 ++++++++ 4 files changed, 14 insertions(+), 33 deletions(-) create mode 100644 js/src/jit-test/tests/class/bug1357506.js diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index ec4a975e64..01ab3f64ca 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -547,7 +547,7 @@ FunctionBox::initWithEnclosingParseContext(ParseContext* enclosing, FunctionSynt if (kind == ClassConstructor || kind == DerivedClassConstructor) { auto stmt = enclosing->findInnermostStatement(); MOZ_ASSERT(stmt); - stmt->setConstructorBox(this); + stmt->constructorBox = this; if (kind == DerivedClassConstructor) { setDerivedClassConstructor(); @@ -573,16 +573,6 @@ FunctionBox::initWithEnclosingParseContext(ParseContext* enclosing, FunctionSynt } } -void -FunctionBox::resetForAbortedSyntaxParse(ParseContext* enclosing, FunctionSyntaxKind kind) -{ - if (kind == ClassConstructor || kind == DerivedClassConstructor) { - auto stmt = enclosing->findInnermostStatement(); - MOZ_ASSERT(stmt); - stmt->clearConstructorBoxForAbortedSyntaxParse(this); - } -} - void FunctionBox::initWithEnclosingScope(Scope* enclosingScope) { @@ -3407,7 +3397,6 @@ Parser::trySyntaxParseInnerFunction(ParseNode* pn, HandleFunct // correctness. parser->clearAbortedSyntaxParse(); usedNames.rewind(token); - funbox->resetForAbortedSyntaxParse(pc, kind); MOZ_ASSERT_IF(parser->context->isJSContext(), !parser->context->asJSContext()->isExceptionPending()); break; @@ -7078,7 +7067,7 @@ Parser::classDefinition(YieldHandling yieldHandling, errorAt(nameOffset, JSMSG_BAD_METHOD_DEF); return null(); } - if (classStmt.constructorBox()) { + if (classStmt.constructorBox) { errorAt(nameOffset, JSMSG_DUPLICATE_PROPERTY, "constructor"); return null(); } @@ -7125,7 +7114,7 @@ Parser::classDefinition(YieldHandling yieldHandling, // Amend the toStringEnd offset for the constructor now that we've // finished parsing the class. uint32_t classEndOffset = pos().end; - if (FunctionBox* ctorbox = classStmt.constructorBox()) { + if (FunctionBox* ctorbox = classStmt.constructorBox) { if (ctorbox->function()->isInterpretedLazy()) ctorbox->function()->lazyScript()->setToStringEnd(classEndOffset); ctorbox->toStringEnd = classEndOffset; diff --git a/js/src/frontend/Parser.h b/js/src/frontend/Parser.h index 65e46a65ed..7866bc4fd7 100644 --- a/js/src/frontend/Parser.h +++ b/js/src/frontend/Parser.h @@ -85,29 +85,14 @@ class ParseContext : public Nestable } }; - class ClassStatement : public Statement + struct ClassStatement : public Statement { - FunctionBox* constructorBox_; + FunctionBox* constructorBox; - public: explicit ClassStatement(ParseContext* pc) : Statement(pc, StatementKind::Class), - constructorBox_(nullptr) + constructorBox(nullptr) { } - - void clearConstructorBoxForAbortedSyntaxParse(FunctionBox* funbox) { - MOZ_ASSERT(constructorBox_ == funbox); - constructorBox_ = nullptr; - } - - void setConstructorBox(FunctionBox* funbox) { - MOZ_ASSERT(!constructorBox_); - constructorBox_ = funbox; - } - - FunctionBox* constructorBox() const { - return constructorBox_; - } }; // The intra-function scope stack. diff --git a/js/src/frontend/SharedContext.h b/js/src/frontend/SharedContext.h index 3499a53fb0..013444690c 100644 --- a/js/src/frontend/SharedContext.h +++ b/js/src/frontend/SharedContext.h @@ -503,7 +503,6 @@ class FunctionBox : public ObjectBox, public SharedContext void initFromLazyFunction(); void initStandaloneFunction(Scope* enclosingScope); void initWithEnclosingParseContext(ParseContext* enclosing, FunctionSyntaxKind kind); - void resetForAbortedSyntaxParse(ParseContext* enclosing, FunctionSyntaxKind kind); ObjectBox* toObjectBox() override { return this; } JSFunction* function() const { return &object->as(); } diff --git a/js/src/jit-test/tests/class/bug1357506.js b/js/src/jit-test/tests/class/bug1357506.js new file mode 100644 index 0000000000..52a5643e62 --- /dev/null +++ b/js/src/jit-test/tests/class/bug1357506.js @@ -0,0 +1,8 @@ +// Test that constructors that abort due to asm.js do not assert due to the +// parser keeping track of the FunctionBox corresponding to the constructor. + +class a { + constructor() { + "use asm"; + } +}