Skip to content

Commit

Permalink
Merge r182827 - DFG register fillSpeculate*() functions should valida…
Browse files Browse the repository at this point in the history 
…te incoming spill format is compatible with requested fill format

https://bugs.webkit.org/show_bug.cgi?id=143727

Reviewed by Geoffrey Garen.

Used the result of AbstractInterpreter<>::filter() to check that the current spill format is compatible
with the requested fill format.  If filter() reports a contradiction, then we force an OSR exit.
Removed individual checks made redundant by the new check.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateInt52):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
  • Loading branch information
@msaboff @carlosgcampos
msaboff authored and carlosgcampos committed May 11, 2015
1 parent 2071bfe commit 77a7385
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 101 deletions.
21 changes: 21 additions & 0 deletions Source/JavaScriptCore/ChangeLog
View file
@@ -1,3 +1,24 @@
2015-04-14 Michael Saboff <msaboff@apple.com>

DFG register fillSpeculate*() functions should validate incoming spill format is compatible with requested fill format
https://bugs.webkit.org/show_bug.cgi?id=143727

Reviewed by Geoffrey Garen.

Used the result of AbstractInterpreter<>::filter() to check that the current spill format is compatible
with the requested fill format. If filter() reports a contradiction, then we force an OSR exit.
Removed individual checks made redundant by the new check.

* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::fillSpeculateInt32Internal):
(JSC::DFG::SpeculativeJIT::fillSpeculateInt52):
(JSC::DFG::SpeculativeJIT::fillSpeculateCell):
(JSC::DFG::SpeculativeJIT::fillSpeculateBoolean):

2015-04-10 Csaba Osztrogonác <ossy@webkit.org>

[ARM] Fix calleeSaveRegisters() on non iOS platforms after r180516
Expand Down
67 changes: 19 additions & 48 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
View file
Expand Up @@ -732,16 +732,16 @@ GPRReg SpeculativeJIT::fillSpeculateInt32Internal(Edge edge, DataFormat& returnF
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
ASSERT(edge.useKind() != KnownInt32Use || !(value.m_type & ~SpecInt32));
m_interpreter.filter(value, SpecInt32);
VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

if (edge->hasConstant() && !edge->isInt32Constant()) {
if (m_interpreter.filter(value, SpecInt32) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
returnFormat = DataFormatInt32;
return allocate();
}


VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
if (edge->hasConstant()) {
Expand All @@ -756,12 +756,6 @@ GPRReg SpeculativeJIT::fillSpeculateInt32Internal(Edge edge, DataFormat& returnF

DataFormat spillFormat = info.spillFormat();

if (spillFormat == DataFormatCell) {
terminateSpeculativeExecution(BadType, JSValueRegs(), edge);
returnFormat = DataFormatInt32;
return allocate();
}

ASSERT_UNUSED(spillFormat, (spillFormat & DataFormatJS) || spillFormat == DataFormatInt32);

// If we know this was spilled as an integer we can fill without checking.
Expand Down Expand Up @@ -807,10 +801,6 @@ GPRReg SpeculativeJIT::fillSpeculateInt32Internal(Edge edge, DataFormat& returnF
case DataFormatJSDouble:
case DataFormatJSCell:
case DataFormatJSBoolean:
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
returnFormat = DataFormatInt32;
return allocate();

case DataFormatDouble:
case DataFormatStorage:
default:
Expand Down Expand Up @@ -869,26 +859,17 @@ GPRReg SpeculativeJIT::fillSpeculateCell(Edge edge)
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
ASSERT((edge.useKind() != KnownCellUse && edge.useKind() != KnownStringUse) || !(value.m_type & ~SpecCell));
m_interpreter.filter(value, SpecCell);
VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

if (edge->hasConstant() && !edge->isCellConstant()) {
// Protect the silent spill/fill logic by failing early. If we "speculate" on
// the constant then the silent filler may think that we have a cell and a
// constant, so it will try to fill this as an cell constant. Bad things will
// happen.

if (m_interpreter.filter(value, SpecCell) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
if (info.spillFormat() == DataFormatInt32) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

if (edge->hasConstant()) {
JSValue jsValue = edge->asJSValue();
GPRReg gpr = allocate();
Expand Down Expand Up @@ -946,9 +927,6 @@ GPRReg SpeculativeJIT::fillSpeculateCell(Edge edge)
case DataFormatJSDouble:
case DataFormatJSBoolean:
case DataFormatBoolean:
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();

case DataFormatDouble:
case DataFormatStorage:
RELEASE_ASSERT_NOT_REACHED();
Expand All @@ -963,27 +941,23 @@ GPRReg SpeculativeJIT::fillSpeculateBoolean(Edge edge)
{
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
m_interpreter.filter(value, SpecBoolean);

if (m_interpreter.filter(value, SpecBoolean) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
if (info.spillFormat() == DataFormatInt32) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

if (edge->hasConstant()) {
JSValue jsValue = edge->asJSValue();
GPRReg gpr = allocate();
if (jsValue.isBoolean()) {
m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
m_jit.move(MacroAssembler::TrustedImm32(jsValue.asBoolean()), gpr);
info.fillBoolean(*m_stream, gpr);
return gpr;
}
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
m_jit.move(MacroAssembler::TrustedImm32(jsValue.asBoolean()), gpr);
info.fillBoolean(*m_stream, gpr);
return gpr;
}

Expand Down Expand Up @@ -1027,9 +1001,6 @@ GPRReg SpeculativeJIT::fillSpeculateBoolean(Edge edge)
case DataFormatJSDouble:
case DataFormatJSCell:
case DataFormatCell:
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();

case DataFormatDouble:
case DataFormatStorage:
RELEASE_ASSERT_NOT_REACHED();
Expand Down
80 changes: 27 additions & 53 deletions Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
View file
Expand Up @@ -708,20 +708,16 @@ GPRReg SpeculativeJIT::fillSpeculateInt32Internal(Edge edge, DataFormat& returnF
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
ASSERT(edge.useKind() != KnownInt32Use || !(value.m_type & ~SpecInt32));
m_interpreter.filter(value, SpecInt32);
VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

if (edge->hasConstant() && !edge->isInt32Constant()) {
// Protect the silent spill/fill logic by failing early. If we "speculate" on
// the constant then the silent filler may think that we have an int32 and a
// constant, so it will try to fill this as an int32 constant. Bad things will
// happen.
if (m_interpreter.filter(value, SpecInt32) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
returnFormat = DataFormatInt32;
return allocate();
}


VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
GPRReg gpr = allocate();
Expand Down Expand Up @@ -820,12 +816,7 @@ GPRReg SpeculativeJIT::fillSpeculateInt32Internal(Edge edge, DataFormat& returnF
case DataFormatCell:
case DataFormatBoolean:
case DataFormatJSCell:
case DataFormatJSBoolean: {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
returnFormat = DataFormatInt32;
return allocate();
}

case DataFormatJSBoolean:
case DataFormatDouble:
case DataFormatStorage:
case DataFormatInt52:
Expand Down Expand Up @@ -858,17 +849,17 @@ GPRReg SpeculativeJIT::fillSpeculateInt52(Edge edge, DataFormat desiredFormat)
{
ASSERT(desiredFormat == DataFormatInt52 || desiredFormat == DataFormatStrictInt52);
AbstractValue& value = m_state.forNode(edge);
m_interpreter.filter(value, SpecMachineInt);

if (m_interpreter.filter(value, SpecMachineInt) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
if (edge->hasConstant() && !edge->isMachineIntConstant()) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

GPRReg gpr = allocate();

if (edge->hasConstant()) {
Expand Down Expand Up @@ -987,16 +978,15 @@ GPRReg SpeculativeJIT::fillSpeculateCell(Edge edge)
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
ASSERT((edge.useKind() != KnownCellUse && edge.useKind() != KnownStringUse) || !(value.m_type & ~SpecCell));
m_interpreter.filter(value, SpecCell);
VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

if (edge->hasConstant() && !edge->isCellConstant()) {
// Better to fail early on constants.
if (m_interpreter.filter(value, SpecCell) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
GPRReg gpr = allocate();
Expand All @@ -1008,12 +998,7 @@ GPRReg SpeculativeJIT::fillSpeculateCell(Edge edge)
info.fillJSValue(*m_stream, gpr, DataFormatJSCell);
return gpr;
}

if (!(info.spillFormat() & DataFormatJS)) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return gpr;
}


m_gprs.retain(gpr, virtualRegister, SpillOrderSpilled);
m_jit.load64(JITCompiler::addressFor(virtualRegister), gpr);

Expand Down Expand Up @@ -1049,11 +1034,7 @@ GPRReg SpeculativeJIT::fillSpeculateCell(Edge edge)
case DataFormatInt32:
case DataFormatJSDouble:
case DataFormatJSBoolean:
case DataFormatBoolean: {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

case DataFormatBoolean:
case DataFormatDouble:
case DataFormatStorage:
case DataFormatInt52:
Expand All @@ -1070,28 +1051,24 @@ GPRReg SpeculativeJIT::fillSpeculateBoolean(Edge edge)
{
AbstractValue& value = m_state.forNode(edge);
SpeculatedType type = value.m_type;
m_interpreter.filter(value, SpecBoolean);

if (m_interpreter.filter(value, SpecBoolean) == Contradiction) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

VirtualRegister virtualRegister = edge->virtualRegister();
GenerationInfo& info = generationInfoFromVirtualRegister(virtualRegister);

switch (info.registerFormat()) {
case DataFormatNone: {
if (info.spillFormat() == DataFormatInt32) {
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();
}

GPRReg gpr = allocate();

if (edge->hasConstant()) {
JSValue jsValue = edge->asJSValue();
if (jsValue.isBoolean()) {
m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
m_jit.move(MacroAssembler::TrustedImm64(JSValue::encode(jsValue)), gpr);
info.fillJSValue(*m_stream, gpr, DataFormatJSBoolean);
return gpr;
}
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
m_gprs.retain(gpr, virtualRegister, SpillOrderConstant);
m_jit.move(MacroAssembler::TrustedImm64(JSValue::encode(jsValue)), gpr);
info.fillJSValue(*m_stream, gpr, DataFormatJSBoolean);
return gpr;
}
DFG_ASSERT(m_jit.graph(), m_currentNode, info.spillFormat() & DataFormatJS);
Expand Down Expand Up @@ -1132,9 +1109,6 @@ GPRReg SpeculativeJIT::fillSpeculateBoolean(Edge edge)
case DataFormatJSDouble:
case DataFormatJSCell:
case DataFormatCell:
terminateSpeculativeExecution(Uncountable, JSValueRegs(), 0);
return allocate();

case DataFormatDouble:
case DataFormatStorage:
case DataFormatInt52:
Expand Down

0 comments on commit 77a7385

Please sign in to comment.