Title: [255295] branches/safari-610.1.1-branch/Source/_javascript_Core
- Revision
- 255295
- Author
- [email protected]
- Date
- 2020-01-28 13:49:37 -0800 (Tue, 28 Jan 2020)
Log Message
Cherry-pick r254962. rdar://problem/58811967
Restore nullification of DFG::Plan::m_vm when the plan is cancelled.
https://bugs.webkit.org/show_bug.cgi?id=206633
<rdar://problem/58811967>
Reviewed by Robin Morisset.
In r253243, I replaced the nullification of Plan::m_vm in Plan::cancel() with
code to decorate the m_vm pointer with a nuke bit. The thinking is that keeping
the VM pointer in nuked form allows us to do certain assertions, as well as
implementing code in support of keeping Box<Identifier>s alive. It is only
correct to use the nuked VM pointer if and only if the VM is guaranteed to
outlive the Plan. r253243 guarantees this condition.
In r254464, I replaced the use of Box<Identifier> with CacheableIdentifier.
This obviated all the support code added above, and rolled out most of it.
However, I opted to keep the nuked VM pointer in the DFG::Plan to as a debugging
aid (it's nice to be able to know which VM the Plan came from).
However, r254464 also undid the guarantee that the VM will outlive the Plan.
As a result, a nuked VM pointer is no longer guaranteed to point to a valid VM.
Some worker layout tests, run on an ASAN build, detected that the pointer is
pointing to an already freed VM and failed with a crash.
This patch fixes this issue by completely reverting the nuked VM pointer code,
and restores nullification of the m_vm pointer in Plan::cancel().
* dfg/DFGPlan.cpp:
(JSC::DFG::Plan::computeCompileTimes const):
(JSC::DFG::Plan::cancel):
* dfg/DFGPlan.h:
(JSC::DFG::Plan::vm const):
(JSC::DFG::Plan::unnukedVM const): Deleted.
(JSC::DFG::Plan::nuke): Deleted.
(JSC::DFG::Plan::unnuke): Deleted.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254962 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-610.1.1-branch/Source/_javascript_Core/ChangeLog (255294 => 255295)
--- branches/safari-610.1.1-branch/Source/_javascript_Core/ChangeLog 2020-01-28 21:49:34 UTC (rev 255294)
+++ branches/safari-610.1.1-branch/Source/_javascript_Core/ChangeLog 2020-01-28 21:49:37 UTC (rev 255295)
@@ -1,5 +1,85 @@
2020-01-28 Russell Epstein <[email protected]>
+ Cherry-pick r254962. rdar://problem/58811967
+
+ Restore nullification of DFG::Plan::m_vm when the plan is cancelled.
+ https://bugs.webkit.org/show_bug.cgi?id=206633
+ <rdar://problem/58811967>
+
+ Reviewed by Robin Morisset.
+
+ In r253243, I replaced the nullification of Plan::m_vm in Plan::cancel() with
+ code to decorate the m_vm pointer with a nuke bit. The thinking is that keeping
+ the VM pointer in nuked form allows us to do certain assertions, as well as
+ implementing code in support of keeping Box<Identifier>s alive. It is only
+ correct to use the nuked VM pointer if and only if the VM is guaranteed to
+ outlive the Plan. r253243 guarantees this condition.
+
+ In r254464, I replaced the use of Box<Identifier> with CacheableIdentifier.
+ This obviated all the support code added above, and rolled out most of it.
+ However, I opted to keep the nuked VM pointer in the DFG::Plan to as a debugging
+ aid (it's nice to be able to know which VM the Plan came from).
+
+ However, r254464 also undid the guarantee that the VM will outlive the Plan.
+ As a result, a nuked VM pointer is no longer guaranteed to point to a valid VM.
+ Some worker layout tests, run on an ASAN build, detected that the pointer is
+ pointing to an already freed VM and failed with a crash.
+
+ This patch fixes this issue by completely reverting the nuked VM pointer code,
+ and restores nullification of the m_vm pointer in Plan::cancel().
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::computeCompileTimes const):
+ (JSC::DFG::Plan::cancel):
+ * dfg/DFGPlan.h:
+ (JSC::DFG::Plan::vm const):
+ (JSC::DFG::Plan::unnukedVM const): Deleted.
+ (JSC::DFG::Plan::nuke): Deleted.
+ (JSC::DFG::Plan::unnuke): Deleted.
+
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@254962 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-01-22 Mark Lam <[email protected]>
+
+ Restore nullification of DFG::Plan::m_vm when the plan is cancelled.
+ https://bugs.webkit.org/show_bug.cgi?id=206633
+ <rdar://problem/58811967>
+
+ Reviewed by Robin Morisset.
+
+ In r253243, I replaced the nullification of Plan::m_vm in Plan::cancel() with
+ code to decorate the m_vm pointer with a nuke bit. The thinking is that keeping
+ the VM pointer in nuked form allows us to do certain assertions, as well as
+ implementing code in support of keeping Box<Identifier>s alive. It is only
+ correct to use the nuked VM pointer if and only if the VM is guaranteed to
+ outlive the Plan. r253243 guarantees this condition.
+
+ In r254464, I replaced the use of Box<Identifier> with CacheableIdentifier.
+ This obviated all the support code added above, and rolled out most of it.
+ However, I opted to keep the nuked VM pointer in the DFG::Plan to as a debugging
+ aid (it's nice to be able to know which VM the Plan came from).
+
+ However, r254464 also undid the guarantee that the VM will outlive the Plan.
+ As a result, a nuked VM pointer is no longer guaranteed to point to a valid VM.
+ Some worker layout tests, run on an ASAN build, detected that the pointer is
+ pointing to an already freed VM and failed with a crash.
+
+ This patch fixes this issue by completely reverting the nuked VM pointer code,
+ and restores nullification of the m_vm pointer in Plan::cancel().
+
+ * dfg/DFGPlan.cpp:
+ (JSC::DFG::Plan::computeCompileTimes const):
+ (JSC::DFG::Plan::cancel):
+ * dfg/DFGPlan.h:
+ (JSC::DFG::Plan::vm const):
+ (JSC::DFG::Plan::unnukedVM const): Deleted.
+ (JSC::DFG::Plan::nuke): Deleted.
+ (JSC::DFG::Plan::unnuke): Deleted.
+
+2020-01-28 Russell Epstein <[email protected]>
+
Cherry-pick r254523. rdar://problem/58415623
Web Inspector: crash in DumpRenderTree at com.apple._javascript_Core: WTF::RefCountedBase::hasOneRef const
Modified: branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.cpp (255294 => 255295)
--- branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.cpp 2020-01-28 21:49:34 UTC (rev 255294)
+++ branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.cpp 2020-01-28 21:49:37 UTC (rev 255295)
@@ -162,7 +162,7 @@
{
return reportCompileTimes()
|| Options::reportTotalCompileTimes()
- || unnukedVM()->m_perBytecodeProfiler;
+ || (m_vm && m_vm->m_perBytecodeProfiler);
}
bool Plan::reportCompileTimes() const
@@ -713,12 +713,8 @@
// mutator know which VM it belongs to.
// Ref: See Worklist::deleteCancelledPlansForVM().
ASSERT(m_vm);
+ m_vm = nullptr;
- // Nuke the VM pointer so that pointer comparisons against it will fail.
- // We rely on VM pointer comparison in many places to filter out Cancelled
- // plans.
- m_vm = nuke(m_vm);
-
m_codeBlock = nullptr;
m_profiledDFGCodeBlock = nullptr;
m_mustHandleValues.clear();
Modified: branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.h (255294 => 255295)
--- branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.h 2020-01-28 21:49:34 UTC (rev 255294)
+++ branches/safari-610.1.1-branch/Source/_javascript_Core/dfg/DFGPlan.h 2020-01-28 21:49:37 UTC (rev 255295)
@@ -83,7 +83,6 @@
void cleanMustHandleValuesIfNecessary();
VM* vm() const { return m_vm; }
- VM* unnukedVM() const { return unnuke(m_vm); }
CodeBlock* codeBlock() { return m_codeBlock; }
@@ -137,14 +136,6 @@
// Warning: pretty much all of the pointer fields in this object get nulled by cancel(). So, if
// you're writing code that is callable on the cancel path, be sure to null check everything!
-#if CPU(ADDRESS64)
- static constexpr uintptr_t s_nukeBit = 1ull << 63;
-#else
- static constexpr uintptr_t s_nukeBit = 1ull;
-#endif
- static VM* nuke(VM* vm) { return bitwise_cast<VM*>(bitwise_cast<uintptr_t>(vm) | s_nukeBit); }
- static VM* unnuke(VM* vm) { return bitwise_cast<VM*>(bitwise_cast<uintptr_t>(vm) & ~s_nukeBit); }
-
CompilationMode m_mode;
VM* m_vm;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
