Bug 1365782 - Bailout from MConcat instead of throwing draft
authorTed Campbell <tcampbell@mozilla.com>
Thu, 18 May 2017 21:55:05 -0400
changeset 583018 d06889fdd929ff5a1c54b63aa5b531c3bdce4230
parent 582730 5bc1c758ab57c1885dceab4e7837e58af27b998c
child 629939 2d555d997ed508845008de36f1284490f40de767
push id60267
push userbmo:tcampbell@mozilla.com
push dateTue, 23 May 2017 15:15:48 +0000
bugs1365782
milestone55.0a1
Bug 1365782 - Bailout from MConcat instead of throwing MozReview-Commit-ID: BdjMzfJjez
js/src/jit-test/tests/ion/bug1365782-1.js
js/src/jit-test/tests/ion/bug1365782-2.js
js/src/jit-test/tests/ion/dce-with-rinstructions.js
js/src/jit/BaselineBailouts.cpp
js/src/jit/CodeGenerator.cpp
js/src/jit/IonTypes.h
js/src/jit/Lowering.cpp
js/src/jit/MIR.h
js/src/jit/VMFunctions.cpp
js/src/jit/VMFunctions.h
new file mode 100644
--- /dev/null
+++ b/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);
new file mode 100644
--- /dev/null
+++ b/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) { }
+}
--- a/js/src/jit-test/tests/ion/dce-with-rinstructions.js
+++ b/js/src/jit-test/tests/ion/dce-with-rinstructions.js
@@ -1378,18 +1378,18 @@ for (j = 100 - max; j < 100; j++) {
     rimul_object(i);
     rdiv_number(i);
     rdiv_float(i);
     rdiv_object(i);
     rmod_number(i);
     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);
     rinline_arguments_length_1(i);
     rinline_arguments_length_3(i, 0, 1);
     rfloor_number(i);
     rfloor_object(i);
     rceil_number(i);
--- a/js/src/jit/BaselineBailouts.cpp
+++ b/js/src/jit/BaselineBailouts.cpp
@@ -1995,16 +1995,17 @@ jit::FinishBailoutToBaseline(BaselineBai
       case Bailout_NonObjectInput:
       case Bailout_NonStringInput:
       case Bailout_NonSymbolInput:
       case Bailout_UnexpectedSimdInput:
       case Bailout_NonSharedTypedArrayInput:
       case Bailout_Debugger:
       case Bailout_UninitializedThis:
       case Bailout_BadDerivedConstructorReturn:
+      case Bailout_NotPure:
         // Do nothing.
         break;
 
       case Bailout_FirstExecution:
         // Do not return directly, as this was not frequent in the first place,
         // thus rely on the check for frequent bailouts to recompile the current
         // script.
         break;
--- a/js/src/jit/CodeGenerator.cpp
+++ b/js/src/jit/CodeGenerator.cpp
@@ -7431,31 +7431,39 @@ CodeGenerator::visitIsNullOrLikeUndefine
         Register scratch = ToRegister(lir->temp());
         testObjectEmulatesUndefined(input, ifTrueLabel, ifFalseLabel, scratch, ool);
     } else {
         MOZ_ASSERT(lhsType == MIRType::ObjectOrNull);
         testZeroEmitBranch(Assembler::Equal, input, ifTrue, ifFalse);
     }
 }
 
-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
 CodeGenerator::visitConcat(LConcat* lir)
 {
     Register lhs = ToRegister(lir->lhs());
     Register rhs = ToRegister(lir->rhs());
 
--- a/js/src/jit/IonTypes.h
+++ b/js/src/jit/IonTypes.h
@@ -116,16 +116,21 @@ enum BailoutKind
     Bailout_Debugger,
 
     // |this| used uninitialized in a derived constructor
     Bailout_UninitializedThis,
 
     // 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,
 
     // END Normal bailouts
 
     // Bailouts caused by invalid assumptions based on Baseline code.
     // Causes immediate invalidation.
 
@@ -218,16 +223,18 @@ BailoutKindString(BailoutKind kind)
       case Bailout_NonSharedTypedArrayInput:
         return "Bailout_NonSharedTypedArrayInput";
       case Bailout_Debugger:
         return "Bailout_Debugger";
       case Bailout_UninitializedThis:
         return "Bailout_UninitializedThis";
       case Bailout_BadDerivedConstructorReturn:
         return "Bailout_BadDerivedConstructorReturn";
+      case Bailout_NotPure:
+        return "Bailout_NotPure";
       case Bailout_FirstExecution:
         return "Bailout_FirstExecution";
 
       // Bailouts caused by invalid assumptions.
       case Bailout_OverflowInvalidate:
         return "Bailout_OverflowInvalidate";
       case Bailout_NonStringInputInvalidate:
         return "Bailout_NonStringInputInvalidate";
--- a/js/src/jit/Lowering.cpp
+++ b/js/src/jit/Lowering.cpp
@@ -1962,16 +1962,17 @@ LIRGenerator::visitConcat(MConcat* ins)
 
     LConcat* lir = new(alloc()) LConcat(useFixedAtStart(lhs, CallTempReg0),
                                         useFixedAtStart(rhs, CallTempReg1),
                                         tempFixed(CallTempReg0),
                                         tempFixed(CallTempReg1),
                                         tempFixed(CallTempReg2),
                                         tempFixed(CallTempReg3),
                                         tempFixed(CallTempReg4));
+    assignSnapshot(lir, Bailout_NotPure);
     defineFixed(lir, ins, LAllocation(AnyRegister(CallTempReg5)));
     assignSafepoint(lir, ins);
 }
 
 void
 LIRGenerator::visitCharCodeAt(MCharCodeAt* ins)
 {
     MDefinition* str = ins->getOperand(0);
--- a/js/src/jit/MIR.h
+++ b/js/src/jit/MIR.h
@@ -7324,16 +7324,21 @@ class MConcat
     MConcat(MDefinition* left, MDefinition* right)
       : MBinaryInstruction(left, right)
     {
         // At least one input should be definitely string
         MOZ_ASSERT(left->type() == MIRType::String || right->type() == MIRType::String);
 
         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:
     INSTRUCTION_HEADER(Concat)
     TRIVIAL_NEW_WRAPPERS
 
     MDefinition* foldsTo(TempAllocator& alloc) override;
     bool congruentTo(const MDefinition* ins) const override {
--- a/js/src/jit/VMFunctions.cpp
+++ b/js/src/jit/VMFunctions.cpp
@@ -1785,10 +1785,25 @@ HasOwnNativeDataProperty(JSContext* cx, 
 
 JSString*
 TypeOfObject(JSObject* obj, JSRuntime* rt)
 {
     JSType type = js::TypeOfObject(obj);
     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
--- a/js/src/jit/VMFunctions.h
+++ b/js/src/jit/VMFunctions.h
@@ -438,16 +438,17 @@ template <class T> struct TypeToRootType
 
 template <class> struct OutParamToDataType { static const DataType result = Type_Void; };
 template <> struct OutParamToDataType<Value*> { static const DataType result = Type_Value; };
 template <> struct OutParamToDataType<int*> { static const DataType result = Type_Int32; };
 template <> struct OutParamToDataType<uint32_t*> { static const DataType result = Type_Int32; };
 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; };
 
 template <class> struct OutParamToRootType {
     static const VMFunction::RootType result = VMFunction::RootNone;
 };
 template <> struct OutParamToRootType<MutableHandleValue> {
@@ -864,12 +865,15 @@ bool
 SetNativeDataProperty(JSContext* cx, JSObject* obj, PropertyName* name, Value* val);
 
 bool
 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
 
 #endif /* jit_VMFunctions_h */