Modify how we do SetArgument when we inline varargs calls
authorsbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2019 02:41:38 +0000 (02:41 +0000)
committersbarati@apple.com <sbarati@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Apr 2019 02:41:38 +0000 (02:41 +0000)
commit02835894cc79de43af48d6e12fd98b301c4eeea0
tree6399740f43ebc0c56aec4730af613cf65f0ea4e7
parent13936008a28c0d9c2aedc41f03da21cf722c92a9
Modify how we do SetArgument when we inline varargs calls
https://bugs.webkit.org/show_bug.cgi?id=196712
<rdar://problem/49605012>

Reviewed by Michael Saboff.

JSTests:

* stress/get-stack-wrong-type-when-inline-varargs.js: Added.
(foo):

Source/JavaScriptCore:

When we inline varargs calls, we guarantee that the number of arguments that
go on the stack are somewhere between the "mandatoryMinimum" and the "limit - 1".
However, we can't statically guarantee that the arguments between these two
ranges was filled out by Load/ForwardVarargs. This is because in the general
case we don't know the argument count statically.

However, we used to always emit SetArgumentDefinitely up to "limit - 1" for
all arguments, even when some arguments aren't guaranteed to be in a valid
state. Emitting these SetArgumentDefinitely were helpful because they let us
handle variable liveness and OSR exit metadata. However, when we converted
to SSA, we ended up emitting a GetStack for each such SetArgumentDefinitely.

This is wrong, as we can't guarantee such SetArgumentDefinitely nodes are
actually looking at a range of the stack that are guaranteed to be initialized.
This patch introduces a new form of SetArgument node: SetArgumentMaybe. In terms
of OSR exit metadata and variable liveness tracking, it behaves like SetArgumentDefinitely.

However, it differs in a couple key ways:
1. In ThreadedCPS, GetLocal(@SetArgumentMaybe) is invalid IR, as this implies
you might be loading uninitialized stack. (This same rule applies when you do
the full data flow reachability analysis over CPS Phis.) If someone logically
wanted to emit code like this, the correct node to emit would be GetArgument,
not GetLocal. For similar reasons, PhantomLocal(@SetArgumentMaybe) is also
invalid IR.
2. To track liveness, Flush(@SetArgumentMaybe) is valid, and is the main user
of SetArgumentMaybe.
3. In SSA conversion, we don't lower SetArgumentMaybe to GetStack, as there
should be no data flow user of SetArgumentMaybe.

SetArgumentDefinitely guarantees that the stack slot is initialized.
SetArgumentMaybe makes no such guarantee.

* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::handleVarargsInlining):
* dfg/DFGCPSRethreadingPhase.cpp:
(JSC::DFG::CPSRethreadingPhase::freeUnnecessaryNodes):
(JSC::DFG::CPSRethreadingPhase::canonicalizeGetLocalFor):
(JSC::DFG::CPSRethreadingPhase::canonicalizeFlushOrPhantomLocalFor):
(JSC::DFG::CPSRethreadingPhase::canonicalizeLocalsInBlock):
(JSC::DFG::CPSRethreadingPhase::propagatePhis):
(JSC::DFG::CPSRethreadingPhase::computeIsFlushed):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGCommon.h:
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGInPlaceAbstractState.cpp:
(JSC::DFG::InPlaceAbstractState::endBasicBlock):
* dfg/DFGLiveCatchVariablePreservationPhase.cpp:
(JSC::DFG::LiveCatchVariablePreservationPhase::handleBlockForTryCatch):
* dfg/DFGMaximalFlushInsertionPhase.cpp:
(JSC::DFG::MaximalFlushInsertionPhase::treatRegularBlock):
(JSC::DFG::MaximalFlushInsertionPhase::treatRootBlock):
* dfg/DFGMayExit.cpp:
* dfg/DFGNode.cpp:
(JSC::DFG::Node::hasVariableAccessData):
* dfg/DFGNodeType.h:
* dfg/DFGPhantomInsertionPhase.cpp:
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSSAConversionPhase.cpp:
(JSC::DFG::SSAConversionPhase::run):
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGValidate.cpp:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244324 268f45cc-cd09-0410-ab3c-d52691b4dbfc
24 files changed:
JSTests/ChangeLog
JSTests/stress/get-stack-wrong-type-when-inline-varargs.js [new file with mode: 0644]
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGCPSRethreadingPhase.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGCommon.h
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
Source/JavaScriptCore/dfg/DFGLiveCatchVariablePreservationPhase.cpp
Source/JavaScriptCore/dfg/DFGMaximalFlushInsertionPhase.cpp
Source/JavaScriptCore/dfg/DFGMayExit.cpp
Source/JavaScriptCore/dfg/DFGNode.cpp
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGPhantomInsertionPhase.cpp
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSSAConversionPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/dfg/DFGValidate.cpp
Source/JavaScriptCore/ftl/FTLCapabilities.cpp