- Revision
- 240449
- Author
- [email protected]
- Date
- 2019-01-24 14:37:01 -0800 (Thu, 24 Jan 2019)
Log Message
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:
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (240448 => 240449)
--- trunk/JSTests/ChangeLog 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/JSTests/ChangeLog 2019-01-24 22:37:01 UTC (rev 240449)
@@ -1,3 +1,16 @@
+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-24 Saam Barati <[email protected]>
Object Allocation Sinking phase can move a node that walks the stack into a place where the InlineCallFrame is no longer valid
Added: trunk/JSTests/stress/regress-190693.js (0 => 240449)
--- trunk/JSTests/stress/regress-190693.js (rev 0)
+++ trunk/JSTests/stress/regress-190693.js 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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: trunk/Source/_javascript_Core/ChangeLog (240448 => 240449)
--- trunk/Source/_javascript_Core/ChangeLog 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-01-24 22:37:01 UTC (rev 240449)
@@ -1,3 +1,40 @@
+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-24 Saam Barati <[email protected]>
Update ARM64EHash
Modified: trunk/Source/_javascript_Core/heap/ConservativeRoots.h (240448 => 240449)
--- trunk/Source/_javascript_Core/heap/ConservativeRoots.h 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/heap/ConservativeRoots.h 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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: trunk/Source/_javascript_Core/heap/Heap.cpp (240448 => 240449)
--- trunk/Source/_javascript_Core/heap/Heap.cpp 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/heap/Heap.cpp 2019-01-24 22:37:01 UTC (rev 240449)
@@ -2621,15 +2621,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;
},
@@ -2697,14 +2704,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: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (240448 => 240449)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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: trunk/Source/_javascript_Core/heap/SlotVisitor.h (240448 => 240449)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.h 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.h 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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: trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp (240448 => 240449)
--- trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.cpp 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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: trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h (240448 => 240449)
--- trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h 2019-01-24 22:07:33 UTC (rev 240448)
+++ trunk/Source/_javascript_Core/jit/GCAwareJITStubRoutine.h 2019-01-24 22:37:01 UTC (rev 240449)
@@ -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
