- Revision
- 240564
- Author
- [email protected]
- Date
- 2019-01-28 01:40:47 -0800 (Mon, 28 Jan 2019)
Log Message
Cherry-pick r240449. rdar://problem/47586886
stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
https://bugs.webkit.org/show_bug.cgi?id=190693
Reviewed by Michael Saboff.
JSTests:
* stress/regress-190693.js: Added.
(truth):
(assert):
(shouldThrowInvalidConstAssignment):
(taz):
Source/_javascript_Core:
JITStubRoutine's fields are marked only when JITStubRoutine::m_mayBeExecuting is true.
This becomes true when we find the executable address in our conservative roots, which
means that we could be executing it right now. This means that object liveness in
JITStubRoutine depends on the information gathered in ConservativeRoots. However, our
constraints are separated, "Conservative Scan" and "JIT Stub Routines". They can even
be executed concurrently, so that "JIT Stub Routines" may miss to mark the actually
executing JITStubRoutine because "Conservative Scan" finds it later.
When finalizing the GC, we delete the dead JITStubRoutines. At that time, since
"Conservative Scan" already finishes, we do not delete some JITStubRoutines which do not
mark the depending objects. Then, in the next cycle, we find JITStubRoutines still live,
attempt to mark the depending objects, and encounter the dead objects which are collected
in the previous cycles.
This patch removes "JIT Stub Routines" and merge it to "Conservative Scan". Since
"Conservative Scan" and "JIT Stub Routines" need to be executed only when the execution
happens (ensured by GreyedByExecution and CollectionPhase check), this change is OK for
GC stop time.
* heap/ConservativeRoots.h:
(JSC::ConservativeRoots::roots const):
(JSC::ConservativeRoots::roots): Deleted.
* heap/Heap.cpp:
(JSC::Heap::addCoreConstraints):
* heap/SlotVisitor.cpp:
(JSC::SlotVisitor::append):
* heap/SlotVisitor.h:
* jit/GCAwareJITStubRoutine.cpp:
(JSC::GCAwareJITStubRoutine::GCAwareJITStubRoutine):
* jit/GCAwareJITStubRoutine.h:
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240449 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-607-branch/JSTests/ChangeLog (240563 => 240564)
--- branches/safari-607-branch/JSTests/ChangeLog 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/JSTests/ChangeLog 2019-01-28 09:40:47 UTC (rev 240564)
@@ -1,5 +1,69 @@
2019-01-28 Babak Shafiei <[email protected]>
+ Cherry-pick r240449. rdar://problem/47586886
+
+ stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
+ https://bugs.webkit.org/show_bug.cgi?id=190693
+
+ Reviewed by Michael Saboff.
+
+ JSTests:
+
+ * stress/regress-190693.js: Added.
+ (truth):
+ (assert):
+ (shouldThrowInvalidConstAssignment):
+ (taz):
+
+ Source/_javascript_Core:
+
+ JITStubRoutine's fields are marked only when JITStubRoutine::m_mayBeExecuting is true.
+ This becomes true when we find the executable address in our conservative roots, which
+ means that we could be executing it right now. This means that object liveness in
+ JITStubRoutine depends on the information gathered in ConservativeRoots. However, our
+ constraints are separated, "Conservative Scan" and "JIT Stub Routines". They can even
+ be executed concurrently, so that "JIT Stub Routines" may miss to mark the actually
+ executing JITStubRoutine because "Conservative Scan" finds it later.
+ When finalizing the GC, we delete the dead JITStubRoutines. At that time, since
+ "Conservative Scan" already finishes, we do not delete some JITStubRoutines which do not
+ mark the depending objects. Then, in the next cycle, we find JITStubRoutines still live,
+ attempt to mark the depending objects, and encounter the dead objects which are collected
+ in the previous cycles.
+
+ This patch removes "JIT Stub Routines" and merge it to "Conservative Scan". Since
+ "Conservative Scan" and "JIT Stub Routines" need to be executed only when the execution
+ happens (ensured by GreyedByExecution and CollectionPhase check), this change is OK for
+ GC stop time.
+
+ * heap/ConservativeRoots.h:
+ (JSC::ConservativeRoots::roots const):
+ (JSC::ConservativeRoots::roots): Deleted.
+ * heap/Heap.cpp:
+ (JSC::Heap::addCoreConstraints):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::append):
+ * heap/SlotVisitor.h:
+ * jit/GCAwareJITStubRoutine.cpp:
+ (JSC::GCAwareJITStubRoutine::GCAwareJITStubRoutine):
+ * jit/GCAwareJITStubRoutine.h:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240449 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-24 Yusuke Suzuki <[email protected]>
+
+ stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
+ https://bugs.webkit.org/show_bug.cgi?id=190693
+
+ Reviewed by Michael Saboff.
+
+ * stress/regress-190693.js: Added.
+ (truth):
+ (assert):
+ (shouldThrowInvalidConstAssignment):
+ (taz):
+
+2019-01-28 Babak Shafiei <[email protected]>
+
Cherry-pick r240447. rdar://problem/47586899
Object Allocation Sinking phase can move a node that walks the stack into a place where the InlineCallFrame is no longer valid
Added: branches/safari-607-branch/JSTests/stress/regress-190693.js (0 => 240564)
--- branches/safari-607-branch/JSTests/stress/regress-190693.js (rev 0)
+++ branches/safari-607-branch/JSTests/stress/regress-190693.js 2019-01-28 09:40:47 UTC (rev 240564)
@@ -0,0 +1,63 @@
+// Reduced and tweaked code from const-semantics.js to reproduce https://bugs.webkit.org/show_bug.cgi?id=190693 easily.
+"use strict";
+function truth() {
+ return true;
+}
+noInline(truth);
+
+function assert(cond) {
+ if (!cond)
+ throw new Error("broke assertion");
+}
+noInline(assert);
+function shouldThrowInvalidConstAssignment(f) {
+ var threw = false;
+ try {
+ f();
+ } catch(e) {
+ if (e.name.indexOf("TypeError") !== -1 && e.message.indexOf("readonly") !== -1)
+ threw = true;
+ }
+ assert(threw);
+}
+noInline(shouldThrowInvalidConstAssignment);
+
+
+// ========== tests below ===========
+
+const NUM_LOOPS = 6000;
+
+;(function() {
+ function taz() {
+ const x = 20;
+ shouldThrowInvalidConstAssignment(function() { x = 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x += 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x -= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x *= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x /= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x >>= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x <<= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x ^= 20; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x++; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { x--; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { ++x; });
+ assert(x === 20);
+ shouldThrowInvalidConstAssignment(function() { --x; });
+ assert(x === 20);
+ }
+ for (var i = 0; i < NUM_LOOPS; i++) {
+ taz();
+ }
+})();
+
+for(var i = 0; i < 1e6; ++i);
Modified: branches/safari-607-branch/Source/_javascript_Core/ChangeLog (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/ChangeLog 2019-01-28 09:40:47 UTC (rev 240564)
@@ -1,5 +1,93 @@
2019-01-28 Babak Shafiei <[email protected]>
+ Cherry-pick r240449. rdar://problem/47586886
+
+ stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
+ https://bugs.webkit.org/show_bug.cgi?id=190693
+
+ Reviewed by Michael Saboff.
+
+ JSTests:
+
+ * stress/regress-190693.js: Added.
+ (truth):
+ (assert):
+ (shouldThrowInvalidConstAssignment):
+ (taz):
+
+ Source/_javascript_Core:
+
+ JITStubRoutine's fields are marked only when JITStubRoutine::m_mayBeExecuting is true.
+ This becomes true when we find the executable address in our conservative roots, which
+ means that we could be executing it right now. This means that object liveness in
+ JITStubRoutine depends on the information gathered in ConservativeRoots. However, our
+ constraints are separated, "Conservative Scan" and "JIT Stub Routines". They can even
+ be executed concurrently, so that "JIT Stub Routines" may miss to mark the actually
+ executing JITStubRoutine because "Conservative Scan" finds it later.
+ When finalizing the GC, we delete the dead JITStubRoutines. At that time, since
+ "Conservative Scan" already finishes, we do not delete some JITStubRoutines which do not
+ mark the depending objects. Then, in the next cycle, we find JITStubRoutines still live,
+ attempt to mark the depending objects, and encounter the dead objects which are collected
+ in the previous cycles.
+
+ This patch removes "JIT Stub Routines" and merge it to "Conservative Scan". Since
+ "Conservative Scan" and "JIT Stub Routines" need to be executed only when the execution
+ happens (ensured by GreyedByExecution and CollectionPhase check), this change is OK for
+ GC stop time.
+
+ * heap/ConservativeRoots.h:
+ (JSC::ConservativeRoots::roots const):
+ (JSC::ConservativeRoots::roots): Deleted.
+ * heap/Heap.cpp:
+ (JSC::Heap::addCoreConstraints):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::append):
+ * heap/SlotVisitor.h:
+ * jit/GCAwareJITStubRoutine.cpp:
+ (JSC::GCAwareJITStubRoutine::GCAwareJITStubRoutine):
+ * jit/GCAwareJITStubRoutine.h:
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240449 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2019-01-24 Yusuke Suzuki <[email protected]>
+
+ stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
+ https://bugs.webkit.org/show_bug.cgi?id=190693
+
+ Reviewed by Michael Saboff.
+
+ JITStubRoutine's fields are marked only when JITStubRoutine::m_mayBeExecuting is true.
+ This becomes true when we find the executable address in our conservative roots, which
+ means that we could be executing it right now. This means that object liveness in
+ JITStubRoutine depends on the information gathered in ConservativeRoots. However, our
+ constraints are separated, "Conservative Scan" and "JIT Stub Routines". They can even
+ be executed concurrently, so that "JIT Stub Routines" may miss to mark the actually
+ executing JITStubRoutine because "Conservative Scan" finds it later.
+ When finalizing the GC, we delete the dead JITStubRoutines. At that time, since
+ "Conservative Scan" already finishes, we do not delete some JITStubRoutines which do not
+ mark the depending objects. Then, in the next cycle, we find JITStubRoutines still live,
+ attempt to mark the depending objects, and encounter the dead objects which are collected
+ in the previous cycles.
+
+ This patch removes "JIT Stub Routines" and merge it to "Conservative Scan". Since
+ "Conservative Scan" and "JIT Stub Routines" need to be executed only when the execution
+ happens (ensured by GreyedByExecution and CollectionPhase check), this change is OK for
+ GC stop time.
+
+ * heap/ConservativeRoots.h:
+ (JSC::ConservativeRoots::roots const):
+ (JSC::ConservativeRoots::roots): Deleted.
+ * heap/Heap.cpp:
+ (JSC::Heap::addCoreConstraints):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::append):
+ * heap/SlotVisitor.h:
+ * jit/GCAwareJITStubRoutine.cpp:
+ (JSC::GCAwareJITStubRoutine::GCAwareJITStubRoutine):
+ * jit/GCAwareJITStubRoutine.h:
+
+2019-01-28 Babak Shafiei <[email protected]>
+
Cherry-pick r240447. rdar://problem/47586899
Object Allocation Sinking phase can move a node that walks the stack into a place where the InlineCallFrame is no longer valid
Modified: branches/safari-607-branch/Source/_javascript_Core/heap/ConservativeRoots.h (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/heap/ConservativeRoots.h 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/heap/ConservativeRoots.h 2019-01-28 09:40:47 UTC (rev 240564)
@@ -42,7 +42,7 @@
void add(void* begin, void* end, JITStubRoutineSet&, CodeBlockSet&);
size_t size() const;
- HeapCell** roots();
+ HeapCell** roots() const;
private:
static const size_t inlineCapacity = 128;
@@ -68,7 +68,7 @@
return m_size;
}
-inline HeapCell** ConservativeRoots::roots()
+inline HeapCell** ConservativeRoots::roots() const
{
return m_roots;
}
Modified: branches/safari-607-branch/Source/_javascript_Core/heap/Heap.cpp (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/heap/Heap.cpp 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/heap/Heap.cpp 2019-01-28 09:40:47 UTC (rev 240564)
@@ -2639,15 +2639,22 @@
TimingScope preConvergenceTimingScope(*this, "Constraint: conservative scan");
m_objectSpace.prepareForConservativeScan();
- ConservativeRoots conservativeRoots(*this);
- SuperSamplerScope superSamplerScope(false);
+ {
+ ConservativeRoots conservativeRoots(*this);
+ SuperSamplerScope superSamplerScope(false);
- gatherStackRoots(conservativeRoots);
- gatherJSStackRoots(conservativeRoots);
- gatherScratchBufferRoots(conservativeRoots);
+ gatherStackRoots(conservativeRoots);
+ gatherJSStackRoots(conservativeRoots);
+ gatherScratchBufferRoots(conservativeRoots);
- SetRootMarkReasonScope rootScope(slotVisitor, SlotVisitor::RootMarkReason::ConservativeScan);
- slotVisitor.append(conservativeRoots);
+ SetRootMarkReasonScope rootScope(slotVisitor, SlotVisitor::RootMarkReason::ConservativeScan);
+ slotVisitor.append(conservativeRoots);
+ }
+ {
+ // JITStubRoutines must be visited after scanning ConservativeRoots since JITStubRoutines depend on the hook executed during gathering ConservativeRoots.
+ SetRootMarkReasonScope rootScope(slotVisitor, SlotVisitor::RootMarkReason::JITStubRoutines);
+ m_jitStubRoutines->traceMarkedStubRoutines(slotVisitor);
+ }
lastVersion = m_phaseVersion;
},
@@ -2715,14 +2722,6 @@
ConstraintVolatility::GreyedByExecution);
m_constraintSet->add(
- "Jsr", "JIT Stub Routines",
- [this] (SlotVisitor& slotVisitor) {
- SetRootMarkReasonScope rootScope(slotVisitor, SlotVisitor::RootMarkReason::JITStubRoutines);
- m_jitStubRoutines->traceMarkedStubRoutines(slotVisitor);
- },
- ConstraintVolatility::GreyedByExecution);
-
- m_constraintSet->add(
"Ws", "Weak Sets",
[this] (SlotVisitor& slotVisitor) {
SetRootMarkReasonScope rootScope(slotVisitor, SlotVisitor::RootMarkReason::WeakSets);
Modified: branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.cpp (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.cpp 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.cpp 2019-01-28 09:40:47 UTC (rev 240564)
@@ -134,7 +134,7 @@
});
}
-void SlotVisitor::append(ConservativeRoots& conservativeRoots)
+void SlotVisitor::append(const ConservativeRoots& conservativeRoots)
{
HeapCell** roots = conservativeRoots.roots();
size_t size = conservativeRoots.size();
Modified: branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.h (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.h 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/heap/SlotVisitor.h 2019-01-28 09:40:47 UTC (rev 240564)
@@ -86,7 +86,7 @@
const VM& vm() const;
Heap* heap() const;
- void append(ConservativeRoots&);
+ void append(const ConservativeRoots&);
template<typename T, typename Traits> void append(const WriteBarrierBase<T, Traits>&);
template<typename T, typename Traits> void appendHidden(const WriteBarrierBase<T, Traits>&);
Modified: branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp 2019-01-28 09:40:47 UTC (rev 240564)
@@ -43,8 +43,6 @@
GCAwareJITStubRoutine::GCAwareJITStubRoutine(
const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm)
: JITStubRoutine(code)
- , m_mayBeExecuting(false)
- , m_isJettisoned(false)
{
vm.heap.m_jitStubRoutines->add(this);
}
Modified: branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h (240563 => 240564)
--- branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h 2019-01-28 09:40:43 UTC (rev 240563)
+++ branches/safari-607-branch/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h 2019-01-28 09:40:47 UTC (rev 240564)
@@ -67,8 +67,8 @@
private:
friend class JITStubRoutineSet;
- bool m_mayBeExecuting;
- bool m_isJettisoned;
+ bool m_mayBeExecuting { false };
+ bool m_isJettisoned { false };
};
// Use this if you want to mark one additional object during GC if your stub
