Bug 1593175 - Check mightHaveSideEffects in CodeGenerator::visitValueToString too. r=anba
authorJan de Mooij <jdemooij@mozilla.com>
Fri, 01 Nov 2019 08:37:02 +0000
changeset 500342 ec146b86914704bac51d5f370ffac7a1af6deb1c
parent 500341 ae7103e15afdd30a2848da6b0f42e640337da96d
child 500343 62a3bc7b41375fb3ac4aae73bcf88abd0fb6c437
push id114164
push useraiakab@mozilla.com
push dateTue, 05 Nov 2019 10:06:15 +0000
treeherdermozilla-inbound@4d585c7edc76 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersanba
bugs1593175
milestone72.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 1593175 - Check mightHaveSideEffects in CodeGenerator::visitValueToString too. r=anba Differential Revision: https://phabricator.services.mozilla.com/D51379
js/src/jit-test/tests/ion/bug1593175.js
js/src/jit/CodeGenerator.cpp
js/src/jit/MIR.h
new file mode 100644
--- /dev/null
+++ b/js/src/jit-test/tests/ion/bug1593175.js
@@ -0,0 +1,10 @@
+function f() {
+    f;
+}
+f();
+f();
+function g() {
+    typeof(f = []) + f > 2;
+}
+g();
+g();
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -1711,51 +1711,55 @@ void CodeGenerator::visitValueToString(L
     masm.movePtr(ImmGCPtr(names.false_), output);
     masm.jump(&done);
     masm.bind(&true_);
     masm.movePtr(ImmGCPtr(names.true_), output);
     masm.jump(&done);
     masm.bind(&notBoolean);
   }
 
-  // Object
-  if (lir->mir()->input()->mightBeType(MIRType::Object)) {
-    if (lir->mir()->supportSideEffects()) {
-      masm.branchTestObject(Assembler::Equal, tag, ool->entry());
-    } else {
-      // Bail.
-      MOZ_ASSERT(lir->mir()->needsSnapshot());
-      Label bail;
-      masm.branchTestObject(Assembler::Equal, tag, &bail);
-      bailoutFrom(&bail, lir->snapshot());
-    }
-  }
-
-  // Symbol
-  if (lir->mir()->input()->mightBeType(MIRType::Symbol)) {
-    if (lir->mir()->supportSideEffects()) {
-      masm.branchTestSymbol(Assembler::Equal, tag, ool->entry());
-    } else {
-      // Bail.
-      MOZ_ASSERT(lir->mir()->needsSnapshot());
-      Label bail;
-      masm.branchTestSymbol(Assembler::Equal, tag, &bail);
-      bailoutFrom(&bail, lir->snapshot());
+  // Note: when phis are involved, |mir->input()->mightBeType(MIRType::Object)|
+  // could return true even though |mir->mightHaveSideEffects()| is (still)
+  // false. In this case objects/symbols are not actually possible so we check
+  // |mir->mightHaveSideEffects()| first to avoid assertion failures.
+  if (lir->mir()->mightHaveSideEffects()) {
+    // Object
+    if (lir->mir()->input()->mightBeType(MIRType::Object)) {
+      if (lir->mir()->supportSideEffects()) {
+        masm.branchTestObject(Assembler::Equal, tag, ool->entry());
+      } else {
+        // Bail.
+        MOZ_ASSERT(lir->mir()->needsSnapshot());
+        Label bail;
+        masm.branchTestObject(Assembler::Equal, tag, &bail);
+        bailoutFrom(&bail, lir->snapshot());
+      }
+    }
+
+    // Symbol
+    if (lir->mir()->input()->mightBeType(MIRType::Symbol)) {
+      if (lir->mir()->supportSideEffects()) {
+        masm.branchTestSymbol(Assembler::Equal, tag, ool->entry());
+      } else {
+        // Bail.
+        MOZ_ASSERT(lir->mir()->needsSnapshot());
+        Label bail;
+        masm.branchTestSymbol(Assembler::Equal, tag, &bail);
+        bailoutFrom(&bail, lir->snapshot());
+      }
     }
   }
 
   // BigInt
   if (lir->mir()->input()->mightBeType(MIRType::BigInt)) {
     // No fastpath currently implemented.
     masm.branchTestBigInt(Assembler::Equal, tag, ool->entry());
   }
 
-#ifdef DEBUG
   masm.assumeUnreachable("Unexpected type for LValueToString.");
-#endif
 
   masm.bind(&done);
   masm.bind(ool->rejoin());
 }
 
 void CodeGenerator::visitValueToObject(LValueToObject* lir) {
   ValueOperand input = ToValue(lir, LValueToObject::Input);
   Register output = ToRegister(lir->output());
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -4258,16 +4258,18 @@ class MToString : public MUnaryInstructi
 
   AliasSet getAliasSet() const override {
     if (supportSideEffects() && mightHaveSideEffects_) {
       return AliasSet::Store(AliasSet::Any);
     }
     return AliasSet::None();
   }
 
+  bool mightHaveSideEffects() const { return mightHaveSideEffects_; }
+
   bool supportSideEffects() const {
     return sideEffects_ == SideEffectHandling::Supported;
   }
 
   bool needsSnapshot() const {
     return sideEffects_ == SideEffectHandling::Bailout && mightHaveSideEffects_;
   }