Title: [181948] releases/WebKitGTK/webkit-2.8/Source/_javascript_Core
- Revision
- 181948
- Author
- carlo...@webkit.org
- Date
- 2015-03-25 04:23:54 -0700 (Wed, 25 Mar 2015)
Log Message
Merge r181835 - DFG OSR exit shouldn't assume that the frame count for exit is greater than the frame count in DFG
https://bugs.webkit.org/show_bug.cgi?id=142948
Reviewed by Sam Weinig.
It's necessary to ensure that the stack pointer accounts for the extent of our stack usage
since a signal may clobber the area below the stack pointer. When the DFG is executing,
the stack pointer accounts for the DFG's worst-case stack usage. When we OSR exit back to
baseline, we will use a different amount of stack. This is because baseline is a different
compiler. It will make different decisions. So it will use a different amount of stack.
This gets tricky when we are in the process of doing an OSR exit, because we are sort of
incrementally transforming the stack from how it looked in the DFG to how it will look in
baseline. The most conservative approach would be to set the stack pointer to the max of
DFG and baseline.
When this code was written, a reckless assumption was made: that the stack usage in
baseline is always at least as large as the stack usage in DFG. Based on this incorrect
assumption, the code first adjusts the stack pointer to account for the baseline stack
usage. This sort of usually works, because usually baseline does happen to use more stack.
But that's not an invariant. Nobody guarantees this. We will never make any changes that
would make this be guaranteed, because that would be antithetical to how optimizing
compilers work. The DFG should be allowed to use however much stack it decides that it
should use in order to get good performance, and it shouldn't try to guarantee that it
always uses less stack than baseline.
As such, we must always assume that the frame size for DFG execution (i.e.
frameRegisterCount) and the frame size in baseline once we exit (i.e.
requiredRegisterCountForExit) are two independent quantities and they have no
relationship.
Fortunately, though, this code can be made correct by just moving the stack adjustment to
just before we do conversions. This is because we have since changed the OSR exit
algorithm to first lift up all state from the DFG state into a scratch buffer, and then to
drop it out of the scratch buffer and into the stack according to the baseline layout. The
point just before conversions is the point where we have finished reading the DFG frame
and will not read it anymore, and we haven't started writing the baseline frame. So, at
this point it is safe to set the stack pointer to account for the frame size at exit.
This is benign because baseline happens to create larger frames than DFG.
* dfg/DFGOSRExitCompiler32_64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompiler64.cpp:
(JSC::DFG::OSRExitCompiler::compileExit):
* dfg/DFGOSRExitCompilerCommon.cpp:
(JSC::DFG::adjustAndJumpToTarget):
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/ChangeLog (181947 => 181948)
--- releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/ChangeLog 2015-03-25 11:19:12 UTC (rev 181947)
+++ releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/ChangeLog 2015-03-25 11:23:54 UTC (rev 181948)
@@ -1,3 +1,53 @@
+2015-03-22 Filip Pizlo <fpi...@apple.com>
+
+ DFG OSR exit shouldn't assume that the frame count for exit is greater than the frame count in DFG
+ https://bugs.webkit.org/show_bug.cgi?id=142948
+
+ Reviewed by Sam Weinig.
+
+ It's necessary to ensure that the stack pointer accounts for the extent of our stack usage
+ since a signal may clobber the area below the stack pointer. When the DFG is executing,
+ the stack pointer accounts for the DFG's worst-case stack usage. When we OSR exit back to
+ baseline, we will use a different amount of stack. This is because baseline is a different
+ compiler. It will make different decisions. So it will use a different amount of stack.
+
+ This gets tricky when we are in the process of doing an OSR exit, because we are sort of
+ incrementally transforming the stack from how it looked in the DFG to how it will look in
+ baseline. The most conservative approach would be to set the stack pointer to the max of
+ DFG and baseline.
+
+ When this code was written, a reckless assumption was made: that the stack usage in
+ baseline is always at least as large as the stack usage in DFG. Based on this incorrect
+ assumption, the code first adjusts the stack pointer to account for the baseline stack
+ usage. This sort of usually works, because usually baseline does happen to use more stack.
+ But that's not an invariant. Nobody guarantees this. We will never make any changes that
+ would make this be guaranteed, because that would be antithetical to how optimizing
+ compilers work. The DFG should be allowed to use however much stack it decides that it
+ should use in order to get good performance, and it shouldn't try to guarantee that it
+ always uses less stack than baseline.
+
+ As such, we must always assume that the frame size for DFG execution (i.e.
+ frameRegisterCount) and the frame size in baseline once we exit (i.e.
+ requiredRegisterCountForExit) are two independent quantities and they have no
+ relationship.
+
+ Fortunately, though, this code can be made correct by just moving the stack adjustment to
+ just before we do conversions. This is because we have since changed the OSR exit
+ algorithm to first lift up all state from the DFG state into a scratch buffer, and then to
+ drop it out of the scratch buffer and into the stack according to the baseline layout. The
+ point just before conversions is the point where we have finished reading the DFG frame
+ and will not read it anymore, and we haven't started writing the baseline frame. So, at
+ this point it is safe to set the stack pointer to account for the frame size at exit.
+
+ This is benign because baseline happens to create larger frames than DFG.
+
+ * dfg/DFGOSRExitCompiler32_64.cpp:
+ (JSC::DFG::OSRExitCompiler::compileExit):
+ * dfg/DFGOSRExitCompiler64.cpp:
+ (JSC::DFG::OSRExitCompiler::compileExit):
+ * dfg/DFGOSRExitCompilerCommon.cpp:
+ (JSC::DFG::adjustAndJumpToTarget):
+
2015-03-21 Andreas Kling <akl...@apple.com>
Make UnlinkedFunctionExecutable fit in a 128-byte cell.
Modified: releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler32_64.cpp (181947 => 181948)
--- releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler32_64.cpp 2015-03-25 11:19:12 UTC (rev 181947)
+++ releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler32_64.cpp 2015-03-25 11:23:54 UTC (rev 181948)
@@ -48,12 +48,6 @@
m_jit.debugCall(debugOperationPrintSpeculationFailure, debugInfo);
}
- // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit.
- m_jit.addPtr(
- CCallHelpers::TrustedImm32(
- -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
- CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
-
// 2) Perform speculation recovery. This only comes into play when an operation
// starts mutating state before verifying the speculation it has already made.
@@ -259,6 +253,14 @@
}
}
+ // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit. This
+ // could toast some stack that the DFG used. We need to do it before storing to stack offsets
+ // used by baseline.
+ m_jit.addPtr(
+ CCallHelpers::TrustedImm32(
+ -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
+ CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
+
// 7) Do all data format conversions and store the results into the stack.
bool haveArguments = false;
Modified: releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp (181947 => 181948)
--- releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp 2015-03-25 11:19:12 UTC (rev 181947)
+++ releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompiler64.cpp 2015-03-25 11:23:54 UTC (rev 181948)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2011, 2013, 2014 Apple Inc. All rights reserved.
+ * Copyright (C) 2011, 2013-2015 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -52,12 +52,6 @@
m_jit.debugCall(debugOperationPrintSpeculationFailure, debugInfo);
}
- // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit.
- m_jit.addPtr(
- CCallHelpers::TrustedImm32(
- -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
- CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
-
// 2) Perform speculation recovery. This only comes into play when an operation
// starts mutating state before verifying the speculation it has already made.
@@ -251,6 +245,14 @@
}
}
+ // Need to ensure that the stack pointer accounts for the worst-case stack usage at exit. This
+ // could toast some stack that the DFG used. We need to do it before storing to stack offsets
+ // used by baseline.
+ m_jit.addPtr(
+ CCallHelpers::TrustedImm32(
+ -m_jit.codeBlock()->jitCode()->dfgCommon()->requiredRegisterCountForExit * sizeof(Register)),
+ CCallHelpers::framePointerRegister, CCallHelpers::stackPointerRegister);
+
// 7) Do all data format conversions and store the results into the stack.
bool haveArguments = false;
Modified: releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp (181947 => 181948)
--- releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp 2015-03-25 11:19:12 UTC (rev 181947)
+++ releases/WebKitGTK/webkit-2.8/Source/_javascript_Core/dfg/DFGOSRExitCompilerCommon.cpp 2015-03-25 11:23:54 UTC (rev 181948)
@@ -265,7 +265,6 @@
void adjustAndJumpToTarget(CCallHelpers& jit, const OSRExitBase& exit)
{
#if ENABLE(GGC)
- // 11) Write barrier the owner executables because we're jumping into a different block.
jit.move(AssemblyHelpers::TrustedImmPtr(jit.codeBlock()->ownerExecutable()), GPRInfo::nonArgGPR0);
osrWriteBarrier(jit, GPRInfo::nonArgGPR0, GPRInfo::nonArgGPR1);
InlineCallFrameSet* inlineCallFrames = jit.codeBlock()->jitCode()->dfgCommon()->inlineCallFrames.get();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
