Bug 1547130 - Always use a tdzCache for class scopes. r=jorendorff
authorAshley Hauck <khyperia@mozilla.com>
Wed, 01 May 2019 22:11:41 +0000
changeset 531008 b562a1384c61efebb21871410a9053a4bcd8b92a
parent 531007 78d0aff2fdc10f4c2485eb921db69ec136e7e178
child 531009 81f1c693b2dd58da05c719269dfbde369ed127db
push id11265
push userffxbld-merge
push dateMon, 13 May 2019 10:53:39 +0000
treeherdermozilla-beta@77e0fe8dbdd3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1547130, 1542448
milestone68.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 1547130 - Always use a tdzCache for class scopes. r=jorendorff This was incorrectly implemented in bug 1542448 - for a class without a name, the .initializers varaible would correctly use the class scope, but would incorrectly use the tdzCache of the *surrounding* scope. Having two distinct .initializer variables use the same tdzCache caused the crash in this bug. Differential Revision: https://phabricator.services.mozilla.com/D29574
js/src/frontend/BytecodeEmitter.cpp
js/src/frontend/ObjectEmitter.cpp
js/src/frontend/ObjectEmitter.h
js/src/jit-test/tests/fields/bug1547130.js
--- a/js/src/frontend/BytecodeEmitter.cpp
+++ b/js/src/frontend/BytecodeEmitter.cpp
@@ -8750,19 +8750,17 @@ bool BytecodeEmitter::emitClass(
     if (names->outerBinding()) {
       MOZ_ASSERT(names->outerBinding()->name());
       MOZ_ASSERT(names->outerBinding()->name() == innerName);
       kind = ClassEmitter::Kind::Declaration;
     }
   }
 
   if (!classNode->isEmptyScope()) {
-    if (!ce.emitScope(classNode->scopeBindings(),
-                      classNode->names() ? ClassEmitter::HasName::Yes
-                                         : ClassEmitter::HasName::No)) {
+    if (!ce.emitScope(classNode->scopeBindings())) {
       //            [stack]
       return false;
     }
   }
 
   // This is kind of silly. In order to the get the home object defined on
   // the constructor, we have to make it second, but we want the prototype
   // on top for EmitPropertyList, because we expect static properties to be
--- a/js/src/frontend/ObjectEmitter.cpp
+++ b/js/src/frontend/ObjectEmitter.cpp
@@ -478,24 +478,21 @@ void AutoSaveLocalStrictMode::restore() 
 ClassEmitter::ClassEmitter(BytecodeEmitter* bce)
     : PropertyEmitter(bce),
       strictMode_(bce->sc),
       name_(bce->cx),
       nameForAnonymousClass_(bce->cx) {
   isClass_ = true;
 }
 
-bool ClassEmitter::emitScope(JS::Handle<LexicalScope::Data*> scopeBindings,
-                             HasName hasName) {
+bool ClassEmitter::emitScope(JS::Handle<LexicalScope::Data*> scopeBindings) {
   MOZ_ASSERT(propertyState_ == PropertyState::Start);
   MOZ_ASSERT(classState_ == ClassState::Start);
 
-  if (hasName == HasName::Yes) {
-    tdzCacheForInnerName_.emplace(bce_);
-  }
+  tdzCache_.emplace(bce_);
 
   innerScope_.emplace(bce_);
   if (!innerScope_->enterLexical(bce_, ScopeKind::Lexical, scopeBindings)) {
     return false;
   }
 
 #ifdef DEBUG
   classState_ = ClassState::Scope;
@@ -764,17 +761,17 @@ bool ClassEmitter::emitEnd(Kind kind) {
   //                [stack] CTOR HOMEOBJ
 
   if (!bce_->emit1(JSOP_POP)) {
     //              [stack] CTOR
     return false;
   }
 
   if (name_) {
-    MOZ_ASSERT(tdzCacheForInnerName_.isSome());
+    MOZ_ASSERT(tdzCache_.isSome());
     MOZ_ASSERT(innerScope_.isSome());
 
     if (!bce_->emitLexicalInitialization(name_)) {
       //            [stack] CTOR
       return false;
     }
 
     if (!innerScope_->leave(bce_)) {
@@ -790,30 +787,31 @@ bool ClassEmitter::emitEnd(Kind kind) {
       // Only class statements make outer bindings, and they do not leave
       // themselves on the stack.
       if (!bce_->emit1(JSOP_POP)) {
         //          [stack]
         return false;
       }
     }
 
-    tdzCacheForInnerName_.reset();
+    tdzCache_.reset();
   } else if (innerScope_.isSome()) {
     //              [stack] CTOR
     MOZ_ASSERT(kind == Kind::Expression);
-    MOZ_ASSERT(tdzCacheForInnerName_.isNothing());
+    MOZ_ASSERT(tdzCache_.isSome());
 
     if (!innerScope_->leave(bce_)) {
       return false;
     }
     innerScope_.reset();
+    tdzCache_.reset();
   } else {
     //              [stack] CTOR
     MOZ_ASSERT(kind == Kind::Expression);
-    MOZ_ASSERT(tdzCacheForInnerName_.isNothing());
+    MOZ_ASSERT(tdzCache_.isNothing());
   }
 
   //                [stack] # class declaration
   //                [stack]
   //                [stack] # class expression
   //                [stack] CTOR
 
   strictMode_.restore();
--- a/js/src/frontend/ObjectEmitter.h
+++ b/js/src/frontend/ObjectEmitter.h
@@ -432,69 +432,69 @@ class MOZ_RAII AutoSaveLocalStrictMode {
 };
 
 // Class for emitting bytecode for JS class.
 //
 // Usage: (check for the return value is omitted for simplicity)
 //
 //   `class {}`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::No);
+//     ce.emitScope(scopeBindings);
 //     ce.emitClass(nullptr, nullptr, false);
 //
 //     ce.emitInitDefaultConstructor(Some(offset_of_class),
 //                                   Some(offset_of_closing_bracket));
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::No);
+//     ce.emitScope(scopeBindings);
 //     ce.emitClass(nullptr, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::Yes);
+//     ce.emitScope(scopeBindings);
 //     ce.emitClass(atom_of_X, nullptr, false);
 //
 //     ce.emitInitDefaultConstructor(Some(offset_of_class),
 //                                   Some(offset_of_closing_bracket));
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::Yes);
+//     ce.emitScope(scopeBindings);
 //     ce.emitClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X extends Y { constructor() { ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::Yes);
+//     ce.emitScope(scopeBindings);
 //
 //     emit(Y);
 //     ce.emitDerivedClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     ce.emitInitConstructor(/* needsHomeObject = */ false);
 //
 //     ce.emitEnd(ClassEmitter::Kind::Expression);
 //
 //   `class X extends Y { constructor() { ... super.f(); ... } }`
 //     ClassEmitter ce(this);
-//     ce.emitScope(scopeBindings, HasName::Yes);
+//     ce.emitScope(scopeBindings);
 //
 //     emit(Y);
 //     ce.emitDerivedClass(atom_of_X, nullptr, false);
 //
 //     emit(function_for_constructor);
 //     // pass true if constructor contains super.prop access
 //     ce.emitInitConstructor(/* needsHomeObject = */ true);
 //
@@ -597,17 +597,17 @@ class MOZ_STACK_CLASS ClassEmitter : pub
   //
   //   cons.prototype = homeObject;
   //   homeObject.constructor = cons;
   //
   //   EmitPropertyList(...)
 
   bool isDerived_ = false;
 
-  mozilla::Maybe<TDZCheckCache> tdzCacheForInnerName_;
+  mozilla::Maybe<TDZCheckCache> tdzCache_;
   mozilla::Maybe<EmitterScope> innerScope_;
   AutoSaveLocalStrictMode strictMode_;
 
 #ifdef DEBUG
   // The state of this emitter.
   //
   // +-------+
   // | Start |-+------------------------>+-+
@@ -660,19 +660,17 @@ class MOZ_STACK_CLASS ClassEmitter : pub
 
   JS::Rooted<JSAtom*> name_;
   JS::Rooted<JSAtom*> nameForAnonymousClass_;
   bool hasNameOnStack_ = false;
 
  public:
   explicit ClassEmitter(BytecodeEmitter* bce);
 
-  enum class HasName : bool { No, Yes };
-  bool emitScope(JS::Handle<LexicalScope::Data*> scopeBindings,
-                 HasName hasName);
+  bool emitScope(JS::Handle<LexicalScope::Data*> scopeBindings);
 
   // @param name
   //        Name of the class (nullptr if this is anonymous class)
   // @param nameForAnonymousClass
   //        Statically inferred name of the class (only for anonymous classes)
   // @param hasNameOnStack
   //        If true the name is on the stack (only for anonymous classes)
   MOZ_MUST_USE bool emitClass(JS::Handle<JSAtom*> name,
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/fields/bug1547130.js
@@ -0,0 +1,3 @@
+// |jit-test| --enable-experimental-fields
+
+[ class  { i32a = [ i32a ] = c27 } ] && class { c27 = [ c27 ] = c27 }