DFG string concatenation shouldn't be playing fast and loose with effects and OSR exit
https://bugs.webkit.org/show_bug.cgi?id=148338
Reviewed by Michael Saboff and Saam Barati.
Prior to this change, DFG string concatenation appeared to have various different ways of
creating an OSR exit right after a side effect. That's bad, because the exit will cause
us to reexecute the side effect. The code appears to have some hacks for avoiding this,
but some cases are basically unavoidable, like the OOM case of string concatenation: in
trunk that could cause two executions of the toString operation.
This changes the string concatenation code to either be speculative or effectful but
never both. It's already the case that when this code needs to be effectful, it also
needs to be slow (it does int->string conversions, calls JS functions, etc). So, this is
a small price to pay for sanity.
The biggest part of this change is the introduction of StrCat, which is like MakeRope but
does toString conversions on its own instead of relying on separate nodes. StrCat can
take either 2 or 3 children. It's the effectful but not speculative version of MakeRope.
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGBackwardsPropagationPhase.cpp:
(JSC::DFG::BackwardsPropagationPhase::propagate):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
(JSC::DFG::FixupPhase::convertStringAddUse):
(JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
(JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
(JSC::DFG::FixupPhase::canOptimizeStringObjectAccess):
* dfg/DFGNodeType.h:
* dfg/DFGOperations.cpp:
* dfg/DFGOperations.h:
* dfg/DFGPredictionPropagationPhase.cpp:
(JSC::DFG::PredictionPropagationPhase::propagate):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.h:
(JSC::DFG::SpeculativeJIT::callOperation):
(JSC::DFG::JSValueOperand::JSValueOperand):
(JSC::DFG::JSValueOperand::~JSValueOperand):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGValidate.cpp:
(JSC::DFG::Validate::validate):
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLIntrinsicRepository.h:
* ftl/FTLLowerDFGToLLVM.cpp:
(JSC::FTL::DFG::LowerDFGToLLVM::compileNode):
(JSC::FTL::DFG::LowerDFGToLLVM::compileValueAdd):
(JSC::FTL::DFG::LowerDFGToLLVM::compileStrCat):
(JSC::FTL::DFG::LowerDFGToLLVM::compileArithAddOrSub):
* jit/JITOperations.h:
* tests/stress/exception-effect-strcat.js: Added. This test previously failed.
* tests/stress/exception-in-strcat-string-overflow.js: Added. An earlier version of this patch made this fail.
* tests/stress/exception-in-strcat.js: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@188825 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
index ca116a1..43b08d4 100644
--- a/Source/JavaScriptCore/ChangeLog
+++ b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,71 @@
+2015-08-22 Filip Pizlo <fpizlo@apple.com>
+
+ DFG string concatenation shouldn't be playing fast and loose with effects and OSR exit
+ https://bugs.webkit.org/show_bug.cgi?id=148338
+
+ Reviewed by Michael Saboff and Saam Barati.
+
+ Prior to this change, DFG string concatenation appeared to have various different ways of
+ creating an OSR exit right after a side effect. That's bad, because the exit will cause
+ us to reexecute the side effect. The code appears to have some hacks for avoiding this,
+ but some cases are basically unavoidable, like the OOM case of string concatenation: in
+ trunk that could cause two executions of the toString operation.
+
+ This changes the string concatenation code to either be speculative or effectful but
+ never both. It's already the case that when this code needs to be effectful, it also
+ needs to be slow (it does int->string conversions, calls JS functions, etc). So, this is
+ a small price to pay for sanity.
+
+ The biggest part of this change is the introduction of StrCat, which is like MakeRope but
+ does toString conversions on its own instead of relying on separate nodes. StrCat can
+ take either 2 or 3 children. It's the effectful but not speculative version of MakeRope.
+
+ * dfg/DFGAbstractInterpreterInlines.h:
+ (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+ * dfg/DFGBackwardsPropagationPhase.cpp:
+ (JSC::DFG::BackwardsPropagationPhase::propagate):
+ * dfg/DFGByteCodeParser.cpp:
+ (JSC::DFG::ByteCodeParser::parseBlock):
+ * dfg/DFGClobberize.h:
+ (JSC::DFG::clobberize):
+ * dfg/DFGDoesGC.cpp:
+ (JSC::DFG::doesGC):
+ * dfg/DFGFixupPhase.cpp:
+ (JSC::DFG::FixupPhase::fixupNode):
+ (JSC::DFG::FixupPhase::convertStringAddUse):
+ (JSC::DFG::FixupPhase::fixupToStringOrCallStringConstructor):
+ (JSC::DFG::FixupPhase::attemptToMakeFastStringAdd):
+ (JSC::DFG::FixupPhase::canOptimizeStringObjectAccess):
+ * dfg/DFGNodeType.h:
+ * dfg/DFGOperations.cpp:
+ * dfg/DFGOperations.h:
+ * dfg/DFGPredictionPropagationPhase.cpp:
+ (JSC::DFG::PredictionPropagationPhase::propagate):
+ * dfg/DFGSafeToExecute.h:
+ (JSC::DFG::safeToExecute):
+ * dfg/DFGSpeculativeJIT.h:
+ (JSC::DFG::SpeculativeJIT::callOperation):
+ (JSC::DFG::JSValueOperand::JSValueOperand):
+ (JSC::DFG::JSValueOperand::~JSValueOperand):
+ * dfg/DFGSpeculativeJIT32_64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::compile):
+ * dfg/DFGValidate.cpp:
+ (JSC::DFG::Validate::validate):
+ * ftl/FTLCapabilities.cpp:
+ (JSC::FTL::canCompile):
+ * ftl/FTLIntrinsicRepository.h:
+ * ftl/FTLLowerDFGToLLVM.cpp:
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileNode):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileValueAdd):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileStrCat):
+ (JSC::FTL::DFG::LowerDFGToLLVM::compileArithAddOrSub):
+ * jit/JITOperations.h:
+ * tests/stress/exception-effect-strcat.js: Added. This test previously failed.
+ * tests/stress/exception-in-strcat-string-overflow.js: Added. An earlier version of this patch made this fail.
+ * tests/stress/exception-in-strcat.js: Added.
+
2015-08-22 Andreas Kling <akling@apple.com>
[JSC] Static hash tables should be 100% compile-time constant.
diff --git a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
index 5e2a60b..64cf43c 100644
--- a/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
+++ b/Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
@@ -418,6 +418,12 @@
forNode(node).setType(m_graph, SpecString | SpecBytecodeNumber);
break;
}
+
+ case StrCat: {
+ clobberWorld(node->origin.semantic, clobberLimit);
+ forNode(node).setType(m_graph, SpecString);
+ break;
+ }
case ArithAdd: {
JSValue left = forNode(node->child1()).value();
diff --git a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
index 2768f61..bf65188 100644
--- a/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGBackwardsPropagationPhase.cpp
@@ -246,7 +246,7 @@
node->child2()->mergeFlags(flags);
break;
}
-
+
case ArithAdd: {
if (isNotNegZero(node->child1().node()) || isNotNegZero(node->child2().node()))
flags &= ~NodeBytecodeNeedsNegZero;
diff --git a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
index 25d0abd..533e097 100644
--- a/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
+++ b/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
@@ -3289,37 +3289,31 @@
int startOperand = currentInstruction[2].u.operand;
int numOperands = currentInstruction[3].u.operand;
#if CPU(X86)
- // X86 doesn't have enough registers to compile MakeRope with three arguments.
- // Rather than try to be clever, we just make MakeRope dumber on this processor.
- const unsigned maxRopeArguments = 2;
+ // X86 doesn't have enough registers to compile MakeRope with three arguments. The
+ // StrCat we emit here may be turned into a MakeRope. Rather than try to be clever,
+ // we just make StrCat dumber on this processor.
+ const unsigned maxArguments = 2;
#else
- const unsigned maxRopeArguments = 3;
+ const unsigned maxArguments = 3;
#endif
- auto toStringNodes = std::make_unique<Node*[]>(numOperands);
- for (int i = 0; i < numOperands; i++)
- toStringNodes[i] = addToGraph(ToString, get(VirtualRegister(startOperand - i)));
-
- for (int i = 0; i < numOperands; i++)
- addToGraph(Phantom, toStringNodes[i]);
-
Node* operands[AdjacencyList::Size];
unsigned indexInOperands = 0;
for (unsigned i = 0; i < AdjacencyList::Size; ++i)
operands[i] = 0;
for (int operandIdx = 0; operandIdx < numOperands; ++operandIdx) {
- if (indexInOperands == maxRopeArguments) {
- operands[0] = addToGraph(MakeRope, operands[0], operands[1], operands[2]);
+ if (indexInOperands == maxArguments) {
+ operands[0] = addToGraph(StrCat, operands[0], operands[1], operands[2]);
for (unsigned i = 1; i < AdjacencyList::Size; ++i)
operands[i] = 0;
indexInOperands = 1;
}
ASSERT(indexInOperands < AdjacencyList::Size);
- ASSERT(indexInOperands < maxRopeArguments);
- operands[indexInOperands++] = toStringNodes[operandIdx];
+ ASSERT(indexInOperands < maxArguments);
+ operands[indexInOperands++] = get(VirtualRegister(startOperand - operandIdx));
}
set(VirtualRegister(currentInstruction[1].u.operand),
- addToGraph(MakeRope, operands[0], operands[1], operands[2]));
+ addToGraph(StrCat, operands[0], operands[1], operands[2]));
NEXT_OPCODE(op_strcat);
}
diff --git a/Source/JavaScriptCore/dfg/DFGClobberize.h b/Source/JavaScriptCore/dfg/DFGClobberize.h
index 790d655..baa1a4c 100644
--- a/Source/JavaScriptCore/dfg/DFGClobberize.h
+++ b/Source/JavaScriptCore/dfg/DFGClobberize.h
@@ -392,6 +392,14 @@
write(Heap);
return;
+ case StrCat:
+ // This is pretty weird. In fact, StrCat has very limited effectfulness because we only
+ // pass it primitive values. But, right now, the compiler isn't smart enough to know this
+ // and that's probably OK.
+ read(World);
+ write(Heap);
+ return;
+
case GetGetter:
read(GetterSetter_getter);
def(HeapLocation(GetterLoc, GetterSetter_getter, node->child1()), LazyNode(node));
diff --git a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
index e1e409f..c6a4e62 100644
--- a/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
+++ b/Source/JavaScriptCore/dfg/DFGDoesGC.cpp
@@ -239,6 +239,7 @@
case ToIndexString:
case MaterializeNewObject:
case MaterializeCreateActivation:
+ case StrCat:
return true;
case MultiPutByOffset:
diff --git a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
index ff675d9..917e841 100644
--- a/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
@@ -151,29 +151,16 @@
break;
}
- // FIXME: Optimize for the case where one of the operands is the
- // empty string. Also consider optimizing for the case where we don't
- // believe either side is the emtpy string. Both of these things should
- // be easy.
-
- if (node->child1()->shouldSpeculateString()
- && attemptToMakeFastStringAdd<StringUse>(node, node->child1(), node->child2()))
+ if (attemptToMakeFastStringAdd(node))
break;
- if (node->child2()->shouldSpeculateString()
- && attemptToMakeFastStringAdd<StringUse>(node, node->child2(), node->child1()))
- break;
- if (node->child1()->shouldSpeculateStringObject()
- && attemptToMakeFastStringAdd<StringObjectUse>(node, node->child1(), node->child2()))
- break;
- if (node->child2()->shouldSpeculateStringObject()
- && attemptToMakeFastStringAdd<StringObjectUse>(node, node->child2(), node->child1()))
- break;
- if (node->child1()->shouldSpeculateStringOrStringObject()
- && attemptToMakeFastStringAdd<StringOrStringObjectUse>(node, node->child1(), node->child2()))
- break;
- if (node->child2()->shouldSpeculateStringOrStringObject()
- && attemptToMakeFastStringAdd<StringOrStringObjectUse>(node, node->child2(), node->child1()))
- break;
+
+ // We could attempt to turn this into a StrCat here. But for now, that wouldn't
+ // significantly reduce the number of branches required.
+ break;
+ }
+
+ case StrCat: {
+ attemptToMakeFastStringAdd(node);
break;
}
@@ -1432,8 +1419,6 @@
return;
}
- // FIXME: We ought to be able to have a ToPrimitiveToString node.
-
observeUseKindOnNode<useKind>(edge.node());
createToString<useKind>(node, edge);
}
@@ -1524,47 +1509,44 @@
return;
}
}
-
- template<UseKind leftUseKind>
- bool attemptToMakeFastStringAdd(Node* node, Edge& left, Edge& right)
+
+ bool attemptToMakeFastStringAdd(Node* node)
{
- ASSERT(leftUseKind == StringUse || leftUseKind == StringObjectUse || leftUseKind == StringOrStringObjectUse);
-
- if (isStringObjectUse<leftUseKind>() && !canOptimizeStringObjectAccess(node->origin.semantic))
+ bool goodToGo = true;
+ m_graph.doToChildren(
+ node,
+ [&] (Edge& edge) {
+ if (edge->shouldSpeculateString())
+ return;
+ if (canOptimizeStringObjectAccess(node->origin.semantic)) {
+ if (edge->shouldSpeculateStringObject())
+ return;
+ if (edge->shouldSpeculateStringOrStringObject())
+ return;
+ }
+ goodToGo = false;
+ });
+ if (!goodToGo)
return false;
-
- convertStringAddUse<leftUseKind>(node, left);
-
- if (right->shouldSpeculateString())
- convertStringAddUse<StringUse>(node, right);
- else if (right->shouldSpeculateStringObject() && canOptimizeStringObjectAccess(node->origin.semantic))
- convertStringAddUse<StringObjectUse>(node, right);
- else if (right->shouldSpeculateStringOrStringObject() && canOptimizeStringObjectAccess(node->origin.semantic))
- convertStringAddUse<StringOrStringObjectUse>(node, right);
- else {
- // At this point we know that the other operand is something weird. The semantically correct
- // way of dealing with this is:
- //
- // MakeRope(@left, ToString(ToPrimitive(@right)))
- //
- // So that's what we emit. NB, we need to do all relevant type checks on @left before we do
- // anything to @right, since ToPrimitive may be effectful.
-
- Node* toPrimitive = m_insertionSet.insertNode(
- m_indexInBlock, resultOfToPrimitive(right->prediction()), ToPrimitive,
- node->origin, Edge(right.node()));
- Node* toString = m_insertionSet.insertNode(
- m_indexInBlock, SpecString, ToString, node->origin, Edge(toPrimitive));
-
- fixupToPrimitive(toPrimitive);
- // Don't fix up ToString. ToString and ToPrimitive are originated from the same bytecode and
- // ToPrimitive may have an observable side effect. ToString should not be converted into Check
- // with speculative type check because OSR exit reproduce an observable side effect done in
- // ToPrimitive.
-
- right.setNode(toString);
- }
+ m_graph.doToChildren(
+ node,
+ [&] (Edge& edge) {
+ if (edge->shouldSpeculateString()) {
+ convertStringAddUse<StringUse>(node, edge);
+ return;
+ }
+ ASSERT(canOptimizeStringObjectAccess(node->origin.semantic));
+ if (edge->shouldSpeculateStringObject()) {
+ convertStringAddUse<StringObjectUse>(node, edge);
+ return;
+ }
+ if (edge->shouldSpeculateStringOrStringObject()) {
+ convertStringAddUse<StringOrStringObjectUse>(node, edge);
+ return;
+ }
+ RELEASE_ASSERT_NOT_REACHED();
+ });
convertToMakeRope(node);
return true;
@@ -1600,11 +1582,15 @@
return false;
Structure* stringObjectStructure = m_graph.globalObjectFor(codeOrigin)->stringObjectStructure();
+ m_graph.registerStructure(stringObjectStructure);
ASSERT(stringObjectStructure->storedPrototype().isObject());
ASSERT(stringObjectStructure->storedPrototype().asCell()->classInfo() == StringPrototype::info());
-
- JSObject* stringPrototypeObject = asObject(stringObjectStructure->storedPrototype());
- Structure* stringPrototypeStructure = stringPrototypeObject->structure();
+
+ FrozenValue* stringPrototypeObjectValue =
+ m_graph.freeze(stringObjectStructure->storedPrototype());
+ StringPrototype* stringPrototypeObject =
+ stringPrototypeObjectValue->dynamicCast<StringPrototype*>();
+ Structure* stringPrototypeStructure = stringPrototypeObjectValue->structure();
if (m_graph.registerStructure(stringPrototypeStructure) != StructureRegisteredAndWatched)
return false;
diff --git a/Source/JavaScriptCore/dfg/DFGNodeType.h b/Source/JavaScriptCore/dfg/DFGNodeType.h
index 0775c68..ccfe0e3 100644
--- a/Source/JavaScriptCore/dfg/DFGNodeType.h
+++ b/Source/JavaScriptCore/dfg/DFGNodeType.h
@@ -160,6 +160,9 @@
/* Add of values may either be arithmetic, or result in string concatenation. */\
macro(ValueAdd, NodeResultJS | NodeMustGenerate) \
\
+ /* Add of values that always convers its inputs to strings. May have two or three kids. */\
+ macro(StrCat, NodeResultJS | NodeMustGenerate) \
+ \
/* Property access. */\
/* PutByValAlias indicates a 'put' aliases a prior write to the same property. */\
/* Since a put to 'length' may invalidate optimizations here, */\
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.cpp b/Source/JavaScriptCore/dfg/DFGOperations.cpp
index 34a0976..61d618e 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.cpp
+++ b/Source/JavaScriptCore/dfg/DFGOperations.cpp
@@ -1108,6 +1108,49 @@
return JSRopeString::create(vm, a, b, c);
}
+JSCell* JIT_OPERATION operationStrCat2(ExecState* exec, EncodedJSValue a, EncodedJSValue b)
+{
+ VM& vm = exec->vm();
+ NativeCallFrameTracer tracer(&vm, exec);
+
+ JSString* str1 = JSValue::decode(a).toString(exec);
+ if (exec->hadException())
+ return nullptr;
+ JSString* str2 = JSValue::decode(b).toString(exec);
+ if (exec->hadException())
+ return nullptr;
+
+ if (sumOverflows<int32_t>(str1->length(), str2->length())) {
+ throwOutOfMemoryError(exec);
+ return nullptr;
+ }
+
+ return JSRopeString::create(vm, str1, str2);
+}
+
+JSCell* JIT_OPERATION operationStrCat3(ExecState* exec, EncodedJSValue a, EncodedJSValue b, EncodedJSValue c)
+{
+ VM& vm = exec->vm();
+ NativeCallFrameTracer tracer(&vm, exec);
+
+ JSString* str1 = JSValue::decode(a).toString(exec);
+ if (exec->hadException())
+ return nullptr;
+ JSString* str2 = JSValue::decode(b).toString(exec);
+ if (exec->hadException())
+ return nullptr;
+ JSString* str3 = JSValue::decode(c).toString(exec);
+ if (exec->hadException())
+ return nullptr;
+
+ if (sumOverflows<int32_t>(str1->length(), str2->length(), str3->length())) {
+ throwOutOfMemoryError(exec);
+ return nullptr;
+ }
+
+ return JSRopeString::create(vm, str1, str2, str3);
+}
+
char* JIT_OPERATION operationFindSwitchImmTargetForDouble(
ExecState* exec, EncodedJSValue encodedValue, size_t tableIndex)
{
diff --git a/Source/JavaScriptCore/dfg/DFGOperations.h b/Source/JavaScriptCore/dfg/DFGOperations.h
index 55b4015..a21cc45 100644
--- a/Source/JavaScriptCore/dfg/DFGOperations.h
+++ b/Source/JavaScriptCore/dfg/DFGOperations.h
@@ -125,6 +125,8 @@
JSCell* JIT_OPERATION operationCallStringConstructor(ExecState*, EncodedJSValue);
JSCell* JIT_OPERATION operationMakeRope2(ExecState*, JSString*, JSString*);
JSCell* JIT_OPERATION operationMakeRope3(ExecState*, JSString*, JSString*, JSString*);
+JSCell* JIT_OPERATION operationStrCat2(ExecState*, EncodedJSValue, EncodedJSValue);
+JSCell* JIT_OPERATION operationStrCat3(ExecState*, EncodedJSValue, EncodedJSValue, EncodedJSValue);
char* JIT_OPERATION operationFindSwitchImmTargetForDouble(ExecState*, EncodedJSValue, size_t tableIndex);
char* JIT_OPERATION operationSwitchString(ExecState*, size_t tableIndex, JSString*);
int32_t JIT_OPERATION operationSwitchStringAndGetBranchOffset(ExecState*, size_t tableIndex, JSString*);
diff --git a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
index 00e4e86..d41c45d 100644
--- a/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
+++ b/Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
@@ -495,7 +495,8 @@
case StringCharAt:
case CallStringConstructor:
case ToString:
- case MakeRope: {
+ case MakeRope:
+ case StrCat: {
changed |= setPrediction(SpecString);
break;
}
diff --git a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
index 7869b2c..4cb3566 100644
--- a/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
+++ b/Source/JavaScriptCore/dfg/DFGSafeToExecute.h
@@ -234,6 +234,7 @@
case LogicalNot:
case ToPrimitive:
case ToString:
+ case StrCat:
case CallStringConstructor:
case NewStringObject:
case MakeRope:
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
index 115ccb2..03346b9 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
@@ -1374,11 +1374,21 @@
m_jit.setupArgumentsWithExecState(arg1);
return appendCallSetResult(operation, result);
}
+ JITCompiler::Call callOperation(C_JITOperation_EJJ operation, GPRReg result, GPRReg arg1, GPRReg arg2)
+ {
+ m_jit.setupArgumentsWithExecState(arg1, arg2);
+ return appendCallSetResult(operation, result);
+ }
JITCompiler::Call callOperation(C_JITOperation_EJJC operation, GPRReg result, GPRReg arg1, GPRReg arg2, GPRReg arg3)
{
m_jit.setupArgumentsWithExecState(arg1, arg2, arg3);
return appendCallSetResult(operation, result);
}
+ JITCompiler::Call callOperation(C_JITOperation_EJJJ operation, GPRReg result, GPRReg arg1, GPRReg arg2, GPRReg arg3)
+ {
+ m_jit.setupArgumentsWithExecState(arg1, arg2, arg3);
+ return appendCallSetResult(operation, result);
+ }
JITCompiler::Call callOperation(C_JITOperation_EJZ operation, GPRReg result, GPRReg arg1, GPRReg arg2)
{
m_jit.setupArgumentsWithExecState(arg1, arg2);
@@ -1705,6 +1715,19 @@
m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag);
return appendCallSetResult(operation, result);
}
+
+ JITCompiler::Call callOperation(C_JITOperation_EJJ operation, GPRReg result, GPRReg arg1Tag, GPRReg arg1Payload, GPRReg arg2Tag, GPRReg arg2Payload)
+ {
+ m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag, arg2Payload, arg2Tag);
+ return appendCallSetResult(operation, result);
+ }
+
+ JITCompiler::Call callOperation(C_JITOperation_EJJJ operation, GPRReg result, GPRReg arg1Tag, GPRReg arg1Payload, GPRReg arg2Tag, GPRReg arg2Payload, GPRReg arg3Tag, GPRReg arg3Payload)
+ {
+ m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag, arg2Payload, arg2Tag, arg3Payload, arg3Tag);
+ return appendCallSetResult(operation, result);
+ }
+
JITCompiler::Call callOperation(S_JITOperation_EJ operation, GPRReg result, GPRReg arg1Tag, GPRReg arg1Payload)
{
m_jit.setupArgumentsWithExecState(EABI_32BIT_DUMMY_ARG arg1Payload, arg1Tag);
@@ -2493,6 +2516,8 @@
#endif
{
ASSERT(m_jit);
+ if (!edge)
+ return;
ASSERT_UNUSED(mode, mode == ManualOperandSpeculation || edge.useKind() == UntypedUse);
#if USE(JSVALUE64)
if (jit->isFilled(node()))
@@ -2507,6 +2532,8 @@
~JSValueOperand()
{
+ if (!m_edge)
+ return;
#if USE(JSVALUE64)
ASSERT(m_gprOrInvalid != InvalidGPRReg);
m_jit->unlock(m_gprOrInvalid);
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
index 2ce4cc3..253ac6d 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
@@ -2057,6 +2057,38 @@
break;
}
+ case StrCat: {
+ JSValueOperand op1(this, node->child1());
+ JSValueOperand op2(this, node->child2());
+ JSValueOperand op3(this, node->child3());
+
+ GPRReg op1TagGPR = op1.tagGPR();
+ GPRReg op1PayloadGPR = op1.payloadGPR();
+ GPRReg op2TagGPR = op2.tagGPR();
+ GPRReg op2PayloadGPR = op2.payloadGPR();
+ GPRReg op3TagGPR;
+ GPRReg op3PayloadGPR;
+ if (node->child3()) {
+ op3TagGPR = op3.tagGPR();
+ op3PayloadGPR = op3.payloadGPR();
+ } else {
+ op3TagGPR = InvalidGPRReg;
+ op3PayloadGPR = InvalidGPRReg;
+ }
+
+ flushRegisters();
+
+ GPRFlushedCallResult result(this);
+ if (node->child3())
+ callOperation(operationStrCat3, result.gpr(), op1TagGPR, op1PayloadGPR, op2TagGPR, op2PayloadGPR, op3TagGPR, op3PayloadGPR);
+ else
+ callOperation(operationStrCat2, result.gpr(), op1TagGPR, op1PayloadGPR, op2TagGPR, op2PayloadGPR);
+ m_jit.exceptionCheck();
+
+ cellResult(result.gpr(), node);
+ break;
+ }
+
case ArithAdd:
compileAdd(node);
break;
diff --git a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
index 7f90bef..f571254 100644
--- a/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
+++ b/Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
@@ -2189,6 +2189,32 @@
break;
}
+ case StrCat: {
+ JSValueOperand op1(this, node->child1());
+ JSValueOperand op2(this, node->child2());
+ JSValueOperand op3(this, node->child3());
+
+ GPRReg op1GPR = op1.gpr();
+ GPRReg op2GPR = op2.gpr();
+ GPRReg op3GPR;
+ if (node->child3())
+ op3GPR = op3.gpr();
+ else
+ op3GPR = InvalidGPRReg;
+
+ flushRegisters();
+
+ GPRFlushedCallResult result(this);
+ if (node->child3())
+ callOperation(operationStrCat3, result.gpr(), op1GPR, op2GPR, op3GPR);
+ else
+ callOperation(operationStrCat2, result.gpr(), op1GPR, op2GPR);
+ m_jit.exceptionCheck();
+
+ cellResult(result.gpr(), node);
+ break;
+ }
+
case ArithAdd:
compileAdd(node);
break;
diff --git a/Source/JavaScriptCore/dfg/DFGValidate.cpp b/Source/JavaScriptCore/dfg/DFGValidate.cpp
index eca843b..f8b1184 100644
--- a/Source/JavaScriptCore/dfg/DFGValidate.cpp
+++ b/Source/JavaScriptCore/dfg/DFGValidate.cpp
@@ -237,6 +237,7 @@
case CompareGreaterEq:
case CompareEq:
case CompareStrictEq:
+ case StrCat:
VALIDATE((node), !!node->child1());
VALIDATE((node), !!node->child2());
break;
diff --git a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
index 5c28978..c85545b 100644
--- a/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
+++ b/Source/JavaScriptCore/ftl/FTLCapabilities.cpp
@@ -78,6 +78,7 @@
case GetGlobalVar:
case PutGlobalVar:
case ValueAdd:
+ case StrCat:
case ArithAdd:
case ArithClz32:
case ArithSub:
diff --git a/Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h b/Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h
index 6ab8596..43ddfc3 100644
--- a/Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h
+++ b/Source/JavaScriptCore/ftl/FTLIntrinsicRepository.h
@@ -63,6 +63,8 @@
macro(C_JITOperation_ECZC, functionType(intPtr, intPtr, intPtr, int32, intPtr)) \
macro(C_JITOperation_EGC, functionType(intPtr, intPtr, intPtr, intPtr)) \
macro(C_JITOperation_EJ, functionType(intPtr, intPtr, int64)) \
+ macro(C_JITOperation_EJJ, functionType(intPtr, intPtr, int64, int64)) \
+ macro(C_JITOperation_EJJJ, functionType(intPtr, intPtr, int64, int64, int64)) \
macro(C_JITOperation_EJssJss, functionType(intPtr, intPtr, intPtr, intPtr)) \
macro(C_JITOperation_EJssJssJss, functionType(intPtr, intPtr, intPtr, intPtr, intPtr)) \
macro(C_JITOperation_ESt, functionType(intPtr, intPtr, intPtr)) \
diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
index eedf9db..add6c48 100644
--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToLLVM.cpp
@@ -454,6 +454,9 @@
case ValueAdd:
compileValueAdd();
break;
+ case StrCat:
+ compileStrCat();
+ break;
case ArithAdd:
case ArithSub:
compileArithAddOrSub();
@@ -1315,6 +1318,22 @@
lowJSValue(m_node->child1()), lowJSValue(m_node->child2())));
}
+ void compileStrCat()
+ {
+ LValue result;
+ if (m_node->child3()) {
+ result = vmCall(
+ m_out.operation(operationStrCat3), m_callFrame,
+ lowJSValue(m_node->child1()), lowJSValue(m_node->child2()),
+ lowJSValue(m_node->child3()));
+ } else {
+ result = vmCall(
+ m_out.operation(operationStrCat2), m_callFrame,
+ lowJSValue(m_node->child1()), lowJSValue(m_node->child2()));
+ }
+ setJSValue(result);
+ }
+
void compileArithAddOrSub()
{
bool isSub = m_node->op() == ArithSub;
diff --git a/Source/JavaScriptCore/jit/JITOperations.h b/Source/JavaScriptCore/jit/JITOperations.h
index 73990e9..0320592 100644
--- a/Source/JavaScriptCore/jit/JITOperations.h
+++ b/Source/JavaScriptCore/jit/JITOperations.h
@@ -145,7 +145,9 @@
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJscC)(ExecState*, JSScope*, JSCell*);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJZ)(ExecState*, EncodedJSValue, int32_t);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJZC)(ExecState*, EncodedJSValue, int32_t, JSCell*);
+typedef JSCell* JIT_OPERATION (*C_JITOperation_EJJ)(ExecState*, EncodedJSValue, EncodedJSValue);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJJC)(ExecState*, EncodedJSValue, EncodedJSValue, JSCell*);
+typedef JSCell* JIT_OPERATION (*C_JITOperation_EJJJ)(ExecState*, EncodedJSValue, EncodedJSValue, EncodedJSValue);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJscZ)(ExecState*, JSScope*, int32_t);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJssSt)(ExecState*, JSString*, Structure*);
typedef JSCell* JIT_OPERATION (*C_JITOperation_EJssJss)(ExecState*, JSString*, JSString*);
diff --git a/Source/JavaScriptCore/tests/stress/exception-effect-strcat.js b/Source/JavaScriptCore/tests/stress/exception-effect-strcat.js
new file mode 100644
index 0000000..92772b9
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/exception-effect-strcat.js
@@ -0,0 +1,42 @@
+function foo(a, b) {
+ return a + b;
+}
+
+noInline(foo);
+
+var bStr = "b";
+for (var i = 0; i < 30; ++i)
+ bStr = bStr + bStr;
+
+var effects = 0;
+var b = {toString: function() { effects++; return bStr; }};
+
+for (var i = 0; i < 10000; ++i) {
+ effects = 0;
+ var result = foo("a", b);
+ if (result.length != "a".length + bStr.length)
+ throw "Error: bad result in loop: " + result;
+ if (effects != 1)
+ throw "Error: bad number of effects: " + effects;
+}
+
+// Create a large string.
+var a = "a";
+for (var i = 0; i < 30; ++i)
+ a = a + a;
+
+effects = 0;
+var result = null;
+var didThrow = false;
+try {
+ result = foo(a, b);
+} catch (e) {
+ didThrow = true;
+}
+
+if (!didThrow)
+ throw "Error: did not throw.";
+if (result !== null)
+ throw "Error: did set result to something: " + result;
+if (effects != 1)
+ throw "Error: bad number of effects at end: " + effects;
diff --git a/Source/JavaScriptCore/tests/stress/exception-in-strcat-string-overflow.js b/Source/JavaScriptCore/tests/stress/exception-in-strcat-string-overflow.js
new file mode 100644
index 0000000..26c48fc
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/exception-in-strcat-string-overflow.js
@@ -0,0 +1,37 @@
+function foo(a, b) {
+ return a + "x" + b;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var b;
+ var expected;
+ if (i & 1) {
+ b = 42;
+ expected = "ax42";
+ } else {
+ b = "b";
+ expected = "axb";
+ }
+ var result = foo("a", b);
+ if (result != expected)
+ throw "Error: bad result: " + result;
+}
+
+var longStr = "l";
+for (var i = 0; i < 30; ++i)
+ longStr = longStr + longStr;
+
+var result = null;
+var didThrow = false;
+try {
+ result = foo(longStr, longStr);
+} catch (e) {
+ didThrow = true;
+}
+
+if (!didThrow)
+ throw "Error: did not throw";
+if (result !== null)
+ throw "Error: did set result: " + result;
diff --git a/Source/JavaScriptCore/tests/stress/exception-in-strcat.js b/Source/JavaScriptCore/tests/stress/exception-in-strcat.js
new file mode 100644
index 0000000..3306c4d
--- /dev/null
+++ b/Source/JavaScriptCore/tests/stress/exception-in-strcat.js
@@ -0,0 +1,24 @@
+function foo(a, b) {
+ return a + "x" + b;
+}
+
+noInline(foo);
+
+for (var i = 0; i < 10000; ++i) {
+ var result = foo({toString: function() { return "a"; }}, 42);
+ if (result != "ax42")
+ throw "Error: bad result: " + result;
+}
+
+var result = null;
+var didThrow = false;
+try {
+ result = foo({toString: function() { throw "error"; }}, 42);
+} catch (e) {
+ didThrow = true;
+}
+
+if (!didThrow)
+ throw "Error: did not throw";
+if (result !== null)
+ throw "Error: did set result: " + result;
