Bug 1365782 - Bailout from MConcat instead of throwing r=jandem

MozReview-Commit-ID: BdjMzfJjez

--HG--
extra : rebase_source : 3dba45abb6ce6f1e5d046c38eaf6f34eed473012

js/src/jit-test/tests/ion/bug1365782-1.js

@@ -0,0 +1,27 @@
+// --ion-eager --ion-offthread-compile=off
+
+var threw = 0;
+
+function f(t) {
+    for (var i = 0; i < 2; i++) {
+        try {
+            var x = 1;
+            Array(1);
+            x = 2;
+            x = t + t; // May throw if too large
+        } catch (e) {
+            assertEq(x, 2);
+            threw++;
+        }
+    }
+}
+
+var x = '$';
+for (var i = 0; i < 32; ++i) {
+    with({}){}
+    f(x);
+    try { x = x + x; } catch (e) { }
+}
+
+// Sanity check that throws happen
+assertEq(threw > 0, true);

js/src/jit-test/tests/ion/bug1365782-2.js

@@ -0,0 +1,25 @@
+// --ion-eager --ion-offthread-compile=off
+
+function f(t) {
+    var would_throw = false;
+    try { t + t; } catch (e) { would_throw = true; }
+
+    var s = 0;
+    for (var i = 0; i < 2; i++) {
+        if (!would_throw) {
+            var x = 1;
+            Array(1);
+            x = 2;
+            x = t + t; // May throw if too large
+            s += x.length;
+        }
+    }
+    return s;
+}
+
+var x = '$';
+for (var i = 0; i < 32; ++i) {
+    with({}){}
+    f(x);
+    try { x = x + x; } catch (e) { }
+}

js/src/jit-test/tests/ion/dce-with-rinstructions.js

@@ -1383,8 +1383,8 @@ for (j = 100 - max; j < 100; j++) {
     rmod_object(i);
     rnot_number(i);
     rnot_object(i);
-    rconcat_string(i);
-    rconcat_number(i);
+ // rconcat_string(i);  // Disabled by Bug 1365782
+ // rconcat_number(i);  // Disabled by Bug 1365782
     rstring_length(i);
     rarguments_length_1(i);
     rarguments_length_3(i, 0, 1);

js/src/jit/BaselineBailouts.cpp

@@ -2000,6 +2000,7 @@ jit::FinishBailoutToBaseline(BaselineBailoutInfo* bailoutInfo)
       case Bailout_Debugger:
       case Bailout_UninitializedThis:
       case Bailout_BadDerivedConstructorReturn:
+      case Bailout_NotPure:
         // Do nothing.
         break;
 

js/src/jit/CodeGenerator.cpp

@@ -7436,21 +7436,29 @@ CodeGenerator::visitIsNullOrLikeUndefinedAndBranchT(LIsNullOrLikeUndefinedAndBra
     }
 }
 
-typedef JSString* (*ConcatStringsFn)(JSContext*, HandleString, HandleString);
-static const VMFunction ConcatStringsInfo =
-    FunctionInfo<ConcatStringsFn>(ConcatStrings<CanGC>, "ConcatStrings");
+typedef bool (*ConcatStringsPureFn)(JSContext*, JSString*, JSString*, JSString**);
+static const VMFunction ConcatStringsPureInfo =
+    FunctionInfo<ConcatStringsPureFn>(ConcatStringsPure, "ConcatStringsPure");
 
 void
 CodeGenerator::emitConcat(LInstruction* lir, Register lhs, Register rhs, Register output)
 {
-    OutOfLineCode* ool = oolCallVM(ConcatStringsInfo, lir, ArgList(lhs, rhs),
+    OutOfLineCode* ool = oolCallVM(ConcatStringsPureInfo, lir, ArgList(lhs, rhs),
                                    StoreRegisterTo(output));
 
+    Label done, bail;
     JitCode* stringConcatStub = gen->compartment->jitCompartment()->stringConcatStubNoBarrier();
     masm.call(stringConcatStub);
-    masm.branchTestPtr(Assembler::Zero, output, output, ool->entry());
+    masm.branchTestPtr(Assembler::NonZero, output, output, &done);
 
+    // If the concat would otherwise throw, we instead return nullptr and use
+    // this to signal a bailout. This allows MConcat to be movable.
+    masm.jump(ool->entry());
     masm.bind(ool->rejoin());
+    masm.branchTestPtr(Assembler::Zero, output, output, &bail);
+
+    masm.bind(&done);
+    bailoutFrom(&bail, lir->snapshot());
 }
 
 void

js/src/jit/IonTypes.h

@@ -121,6 +121,11 @@ enum BailoutKind
     // Derived constructors must return object or undefined
     Bailout_BadDerivedConstructorReturn,
 
+    // Operation would throw or GC to complete. Bailout and retry the operation
+    // in baseline. Allows instructions to be hoisted while exceptions throw
+    // from correct location.
+    Bailout_NotPure,
+
     // We hit this code for the first time.
     Bailout_FirstExecution,
 
@@ -223,6 +228,8 @@ BailoutKindString(BailoutKind kind)
         return "Bailout_UninitializedThis";
       case Bailout_BadDerivedConstructorReturn:
         return "Bailout_BadDerivedConstructorReturn";
+      case Bailout_NotPure:
+        return "Bailout_NotPure";
       case Bailout_FirstExecution:
         return "Bailout_FirstExecution";
 

js/src/jit/Lowering.cpp

@@ -1967,6 +1967,7 @@ LIRGenerator::visitConcat(MConcat* ins)
                                         tempFixed(CallTempReg2),
                                         tempFixed(CallTempReg3),
                                         tempFixed(CallTempReg4));
+    assignSnapshot(lir, Bailout_NotPure);
     defineFixed(lir, ins, LAllocation(AnyRegister(CallTempReg5)));
     assignSafepoint(lir, ins);
 }

js/src/jit/MIR.h

@@ -7329,6 +7329,11 @@ class MConcat
 
         setMovable();
         setResultType(MIRType::String);
+
+        // StringConcat throws a catchable exception. Even though we bail to
+        // baseline in that case, we must set Guard to ensure the bailout
+        // happens.
+        setGuard();
     }
 
   public:

js/src/jit/VMFunctions.cpp

@@ -1790,5 +1790,20 @@ TypeOfObject(JSObject* obj, JSRuntime* rt)
     return TypeName(type, *rt->commonNames);
 }
 
+bool
+ConcatStringsPure(JSContext* cx, JSString* lhs, JSString* rhs, JSString** res)
+{
+    JS::AutoCheckCannotGC nogc;
+
+    // ConcatStrings without GC or throwing. If this fails, we set result to
+    // nullptr and let caller do a bailout. Return true to indicate no exception
+    // thrown.
+    *res = ConcatStrings<NoGC>(cx, lhs, rhs);
+
+    MOZ_ASSERT(!cx->isExceptionPending());
+    return true;
+}
+
+
 } // namespace jit
 } // namespace js

js/src/jit/VMFunctions.h

@@ -443,6 +443,7 @@ template <> struct OutParamToDataType<uint32_t*> { static const DataType result
 template <> struct OutParamToDataType<uint8_t**> { static const DataType result = Type_Pointer; };
 template <> struct OutParamToDataType<bool*> { static const DataType result = Type_Bool; };
 template <> struct OutParamToDataType<double*> { static const DataType result = Type_Double; };
+template <> struct OutParamToDataType<JSString**> { static const DataType result = Type_Pointer; };
 template <> struct OutParamToDataType<MutableHandleValue> { static const DataType result = Type_Handle; };
 template <> struct OutParamToDataType<MutableHandleObject> { static const DataType result = Type_Handle; };
 template <> struct OutParamToDataType<MutableHandleString> { static const DataType result = Type_Handle; };
@@ -869,6 +870,9 @@ ObjectHasGetterSetter(JSContext* cx, JSObject* obj, Shape* propShape);
 JSString*
 TypeOfObject(JSObject* obj, JSRuntime* rt);
 
+bool
+ConcatStringsPure(JSContext* cx, JSString* lhs, JSString* rhs, JSString** res);
+
 } // namespace jit
 } // namespace js