Title: [279903] trunk/Source/_javascript_Core
- Revision
- 279903
- Author
- [email protected]
- Date
- 2021-07-13 20:21:03 -0700 (Tue, 13 Jul 2021)
Log Message
Invalid machine code emitted by SpeculativeJIT::emitObjectOrOtherBranch
https://bugs.webkit.org/show_bug.cgi?id=227869
<rdar://problem/80457566>
Reviewed by Mark Lam.
SpeculativeJIT::emitObjectOrOtherBranch used to check the validity of the masqueradesAsUndefined watchpoint twice, and assumed that it could not change in between.
That is clearly incorrect as the main thread is running concurrently with it, and so the watchpoint could fire at any time.
The fix is trivial: just check the validity once, and store the result in a boolean.
If the watchpoint triggers later that is fine: we'll notice and cancel the compilation (see WatchpointCollectionPhase, Plan::isStillValid() and Plan::finalize()).
The change only protects us from rare and hard-to-reproduce crashes on debug builds caused by an ASSERT firing.
I did not add a testcase, as I can only reproduce the bug by adding an extra wait in the middle of emitObjectOrOtherBranch.
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::emitObjectOrOtherBranch):
Modified Paths
Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (279902 => 279903)
--- trunk/Source/_javascript_Core/ChangeLog 2021-07-14 02:42:10 UTC (rev 279902)
+++ trunk/Source/_javascript_Core/ChangeLog 2021-07-14 03:21:03 UTC (rev 279903)
@@ -1,3 +1,22 @@
+2021-07-13 Robin Morisset <[email protected]>
+
+ Invalid machine code emitted by SpeculativeJIT::emitObjectOrOtherBranch
+ https://bugs.webkit.org/show_bug.cgi?id=227869
+ <rdar://problem/80457566>
+
+ Reviewed by Mark Lam.
+
+ SpeculativeJIT::emitObjectOrOtherBranch used to check the validity of the masqueradesAsUndefined watchpoint twice, and assumed that it could not change in between.
+ That is clearly incorrect as the main thread is running concurrently with it, and so the watchpoint could fire at any time.
+ The fix is trivial: just check the validity once, and store the result in a boolean.
+ If the watchpoint triggers later that is fine: we'll notice and cancel the compilation (see WatchpointCollectionPhase, Plan::isStillValid() and Plan::finalize()).
+ The change only protects us from rare and hard-to-reproduce crashes on debug builds caused by an ASSERT firing.
+
+ I did not add a testcase, as I can only reproduce the bug by adding an extra wait in the middle of emitObjectOrOtherBranch.
+
+ * dfg/DFGSpeculativeJIT64.cpp:
+ (JSC::DFG::SpeculativeJIT::emitObjectOrOtherBranch):
+
2021-07-13 Yijia Huang <[email protected]>
Add a new Air::Arg kind ZeroReg to let AIR recognise the new instructions/forms accepting zero register in ARM64
Modified: trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp (279902 => 279903)
--- trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2021-07-14 02:42:10 UTC (rev 279902)
+++ trunk/Source/_javascript_Core/dfg/DFGSpeculativeJIT64.cpp 2021-07-14 03:21:03 UTC (rev 279903)
@@ -1986,7 +1986,8 @@
GPRReg scratchGPR = scratch.gpr();
GPRReg structureGPR = InvalidGPRReg;
- if (!masqueradesAsUndefinedWatchpointIsStillValid()) {
+ bool objectMayMasqueradeAsUndefined = !masqueradesAsUndefinedWatchpointIsStillValid();
+ if (objectMayMasqueradeAsUndefined) {
GPRTemporary realStructure(this);
structure.adopt(realStructure);
structureGPR = structure.gpr();
@@ -1993,7 +1994,7 @@
}
MacroAssembler::Jump notCell = m_jit.branchIfNotCell(JSValueRegs(valueGPR));
- if (masqueradesAsUndefinedWatchpointIsStillValid()) {
+ if (!objectMayMasqueradeAsUndefined) {
DFG_TYPE_CHECK(
JSValueRegs(valueGPR), nodeUse, (~SpecCellCheck) | SpecObject, m_jit.branchIfNotObject(valueGPR));
} else {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
