WebKit Bugzilla
Attachment 360035 Details for
Bug 190693
: stress/const-semantics.js fails a dfg-eager / ftl-eager run with an ASAN release build.
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-190693-20190124135757.patch (text/plain), 8.71 KB, created by
Yusuke Suzuki
on 2019-01-24 13:57:58 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Yusuke Suzuki
Created:
2019-01-24 13:57:58 PST
Size:
8.71 KB
patch
obsolete
>Subversion Revision: 240444 >diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog >index bdf15f7981394565e54f5cef9ceb8ee1c5d3ceff..cabf1476e72ae9a70c194c661041d7ba8f539278 100644 >--- a/Source/JavaScriptCore/ChangeLog >+++ b/Source/JavaScriptCore/ChangeLog >@@ -1,3 +1,40 @@ >+2019-01-24 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ 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 Guillaume Emont <guijemont@igalia.com> > > [JSC] Reenable baseline JIT on mips >diff --git a/Source/JavaScriptCore/heap/ConservativeRoots.h b/Source/JavaScriptCore/heap/ConservativeRoots.h >index 83e5ce4694267c3143f08b44b0a861df450ce1a6..3c064a9b777b06d049f4fa07042764f89f6bde6a 100644 >--- a/Source/JavaScriptCore/heap/ConservativeRoots.h >+++ b/Source/JavaScriptCore/heap/ConservativeRoots.h >@@ -42,7 +42,7 @@ class ConservativeRoots { > 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 @@ inline size_t ConservativeRoots::size() const > return m_size; > } > >-inline HeapCell** ConservativeRoots::roots() >+inline HeapCell** ConservativeRoots::roots() const > { > return m_roots; > } >diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp >index d1667037ac5cdc9ab128613822e87dee3020f307..8da70dd04411a36242389b3f2a83a87bd94694de 100644 >--- a/Source/JavaScriptCore/heap/Heap.cpp >+++ b/Source/JavaScriptCore/heap/Heap.cpp >@@ -2621,15 +2621,22 @@ void Heap::addCoreConstraints() > 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; > }, >@@ -2696,14 +2703,6 @@ void Heap::addCoreConstraints() > }, > 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) { >diff --git a/Source/JavaScriptCore/heap/SlotVisitor.cpp b/Source/JavaScriptCore/heap/SlotVisitor.cpp >index 1d0bd10a9faa848e60254d7985335a33006e5365..59082966bf078db1b745b757e5cd33a17b977d85 100644 >--- a/Source/JavaScriptCore/heap/SlotVisitor.cpp >+++ b/Source/JavaScriptCore/heap/SlotVisitor.cpp >@@ -134,7 +134,7 @@ void SlotVisitor::clearMarkStacks() > }); > } > >-void SlotVisitor::append(ConservativeRoots& conservativeRoots) >+void SlotVisitor::append(const ConservativeRoots& conservativeRoots) > { > HeapCell** roots = conservativeRoots.roots(); > size_t size = conservativeRoots.size(); >diff --git a/Source/JavaScriptCore/heap/SlotVisitor.h b/Source/JavaScriptCore/heap/SlotVisitor.h >index 0d2e6e0aa6fef9a96616dfcc8e511c91810b5476..77118d184ca8167c04666cc207802599ec4d384a 100644 >--- a/Source/JavaScriptCore/heap/SlotVisitor.h >+++ b/Source/JavaScriptCore/heap/SlotVisitor.h >@@ -86,7 +86,7 @@ class SlotVisitor { > 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>&); >diff --git a/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp b/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp >index 2aa17f4173ea2f2b2ed483c6f2501248eea605ae..195ff02f397cc020701c5a10b6c30a63c70269d3 100644 >--- a/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp >+++ b/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.cpp >@@ -43,8 +43,6 @@ namespace JSC { > GCAwareJITStubRoutine::GCAwareJITStubRoutine( > const MacroAssemblerCodeRef<JITStubRoutinePtrTag>& code, VM& vm) > : JITStubRoutine(code) >- , m_mayBeExecuting(false) >- , m_isJettisoned(false) > { > vm.heap.m_jitStubRoutines->add(this); > } >diff --git a/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h b/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h >index f5d0f616369a40866604cf2585e8abe961ac5586..5dedc6ab8c9cf9886bb32f2195a735fc0774cbf4 100644 >--- a/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h >+++ b/Source/JavaScriptCore/jit/GCAwareJITStubRoutine.h >@@ -67,8 +67,8 @@ class GCAwareJITStubRoutine : public JITStubRoutine { > 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 >diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog >index e4819f84e0e288f4ee8fc649fd7764d8969d27b2..e2838988aad4a51d34d1dd844c07801eb622cb7d 100644 >--- a/JSTests/ChangeLog >+++ b/JSTests/ChangeLog >@@ -1,3 +1,16 @@ >+2019-01-24 Yusuke Suzuki <ysuzuki@apple.com> >+ >+ 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 NOBODY (OOPS!). >+ >+ * stress/regress-190693.js: Added. >+ (truth): >+ (assert): >+ (shouldThrowInvalidConstAssignment): >+ (taz): >+ > 2019-01-24 Guillaume Emont <guijemont@igalia.com> > > [JSC] Reenable baseline JIT on mips
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
msaboff
:
review+
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 190693
: 360035