From e45c6a6768fe4ce314c2f6e218c49c582b8c81ac Mon Sep 17 00:00:00 2001 From: Ted Campbell Date: Thu, 18 May 2017 21:55:05 -0400 Subject: [PATCH] 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 | 27 +++++++++++++++++++ js/src/jit-test/tests/ion/bug1365782-2.js | 25 +++++++++++++++++ .../tests/ion/dce-with-rinstructions.js | 4 +-- js/src/jit/BaselineBailouts.cpp | 1 + js/src/jit/CodeGenerator.cpp | 18 +++++++++---- js/src/jit/IonTypes.h | 7 +++++ js/src/jit/Lowering.cpp | 1 + js/src/jit/MIR.h | 5 ++++ js/src/jit/VMFunctions.cpp | 15 +++++++++++ js/src/jit/VMFunctions.h | 4 +++ 10 files changed, 100 insertions(+), 7 deletions(-) create mode 100644 js/src/jit-test/tests/ion/bug1365782-1.js create mode 100644 js/src/jit-test/tests/ion/bug1365782-2.js diff --git a/js/src/jit-test/tests/ion/bug1365782-1.js b/js/src/jit-test/tests/ion/bug1365782-1.js new file mode 100644 index 000000000000..a45431af6f01 --- /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); diff --git a/js/src/jit-test/tests/ion/bug1365782-2.js b/js/src/jit-test/tests/ion/bug1365782-2.js new file mode 100644 index 000000000000..e507acc4b13a --- /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) { } +} diff --git a/js/src/jit-test/tests/ion/dce-with-rinstructions.js b/js/src/jit-test/tests/ion/dce-with-rinstructions.js index 8d34dcc88262..add671093e67 100644 --- a/js/src/jit-test/tests/ion/dce-with-rinstructions.js +++ b/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); diff --git a/js/src/jit/BaselineBailouts.cpp b/js/src/jit/BaselineBailouts.cpp index c1e368543e18..e3f6eb41638d 100644 --- a/js/src/jit/BaselineBailouts.cpp +++ b/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; diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index e7aabe757833..f8210c69f30a 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -7436,21 +7436,29 @@ CodeGenerator::visitIsNullOrLikeUndefinedAndBranchT(LIsNullOrLikeUndefinedAndBra } } -typedef JSString* (*ConcatStringsFn)(JSContext*, HandleString, HandleString); -static const VMFunction ConcatStringsInfo = - FunctionInfo(ConcatStrings, "ConcatStrings"); +typedef bool (*ConcatStringsPureFn)(JSContext*, JSString*, JSString*, JSString**); +static const VMFunction ConcatStringsPureInfo = + FunctionInfo(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 diff --git a/js/src/jit/IonTypes.h b/js/src/jit/IonTypes.h index 295f5cbb9da6..b2c349f5a32f 100644 --- a/js/src/jit/IonTypes.h +++ b/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"; diff --git a/js/src/jit/Lowering.cpp b/js/src/jit/Lowering.cpp index e19d33bb9e35..5465cb1a7498 100644 --- a/js/src/jit/Lowering.cpp +++ b/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); } diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 0f497d3356a5..376527346b2c 100644 --- a/js/src/jit/MIR.h +++ b/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: diff --git a/js/src/jit/VMFunctions.cpp b/js/src/jit/VMFunctions.cpp index 028b71428799..b7f0e14d563d 100644 --- a/js/src/jit/VMFunctions.cpp +++ b/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(cx, lhs, rhs); + + MOZ_ASSERT(!cx->isExceptionPending()); + return true; +} + + } // namespace jit } // namespace js diff --git a/js/src/jit/VMFunctions.h b/js/src/jit/VMFunctions.h index 01f8fc3bf549..02df34500a3e 100644 --- a/js/src/jit/VMFunctions.h +++ b/js/src/jit/VMFunctions.h @@ -443,6 +443,7 @@ template <> struct OutParamToDataType { static const DataType result template <> struct OutParamToDataType { static const DataType result = Type_Pointer; }; template <> struct OutParamToDataType { static const DataType result = Type_Bool; }; template <> struct OutParamToDataType { static const DataType result = Type_Double; }; +template <> struct OutParamToDataType { static const DataType result = Type_Pointer; }; template <> struct OutParamToDataType { static const DataType result = Type_Handle; }; template <> struct OutParamToDataType { static const DataType result = Type_Handle; }; template <> struct OutParamToDataType { 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