Skip to content
This repository was archived by the owner on Jun 24, 2022. It is now read-only.

Commit 58ea35c

Browse files
author
mark.lam@apple.com
committed
Add crash diagnostics for debugging unexpected zapped cells.
https://bugs.webkit.org/show_bug.cgi?id=200149 <rdar://problem/53570112> Reviewed by Yusuke Suzuki. Source/JavaScriptCore: Add a check for zapped cells in SlotVisitor::appendToMarkStack() and SlotVisitor::visitChildren(). If a zapped cell is detected, we will crash with some diagnostic info. To facilitate this, we've made the following changes: 1. Changed FreeCell to preserve the 1st 8 bytes. This is fine to do because all cells are at least 16 bytes long. 2. Changed HeapCell::zap() to only zap the structureID. Leave the rest of the cell header info intact (including the cell JSType). 3. Changed HeapCell::zap() to record the reason for zapping the cell. We stash the reason immediately after the first 8 bytes. This is the same location as FreeCell::scrambledNext. However, since a cell is not expected to be zapped and on the free list at the same time, it is also fine to do this. 4. Added a few utility functions to MarkedBlock for checking if a cell points into the block. 5. Added VMInspector and JSDollarVM utilities to dump in-use subspace hashes. 6. Added some comments to document the hashes of known subspaces. 7. Added Options::dumpZappedCellCrashData() to make this check conditional. We use this option to disable this check for slower machines so that their PLT5 performance is not impacted. * assembler/CPU.cpp: (JSC::hwL3CacheSize): (JSC::hwPhysicalCPUMax): * assembler/CPU.h: (JSC::hwL3CacheSize): (JSC::hwPhysicalCPUMax): * heap/FreeList.h: (JSC::FreeCell::offsetOfScrambledNext): * heap/HeapCell.h: (JSC::HeapCell::zap): (JSC::HeapCell::isZapped const): * heap/MarkedBlock.cpp: (JSC::MarkedBlock::Handle::stopAllocating): * heap/MarkedBlock.h: (JSC::MarkedBlock::Handle::start const): (JSC::MarkedBlock::Handle::end const): (JSC::MarkedBlock::Handle::contains const): * heap/MarkedBlockInlines.h: (JSC::MarkedBlock::Handle::specializedSweep): * heap/MarkedSpace.h: (JSC::MarkedSpace::forEachSubspace): * heap/SlotVisitor.cpp: (JSC::SlotVisitor::appendToMarkStack): (JSC::SlotVisitor::visitChildren): (JSC::SlotVisitor::reportZappedCellAndCrash): * heap/SlotVisitor.h: * jit/AssemblyHelpers.cpp: (JSC::AssemblyHelpers::emitAllocateWithNonNullAllocator): * runtime/Options.cpp: (JSC::Options::initialize): * runtime/Options.h: * runtime/VM.cpp: (JSC::VM::VM): * tools/JSDollarVM.cpp: (JSC::functionDumpSubspaceHashes): (JSC::JSDollarVM::finishCreation): * tools/VMInspector.cpp: (JSC::VMInspector::dumpSubspaceHashes): * tools/VMInspector.h: Source/WebCore: No new tests because this is a feature for debugging crashes. It has been tested manually by modifying the code to force a crash at the point of interest. Added some comments to document the hashes of known subspaces. * bindings/js/WebCoreJSClientData.cpp: (WebCore::JSVMClientData::JSVMClientData): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@248143 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent a5600cd commit 58ea35c

File tree

20 files changed

+268
-51
lines changed

20 files changed

+268
-51
lines changed

‎Source/JavaScriptCore/ChangeLog‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,72 @@
1+
2019-08-01 Mark Lam <mark.lam@apple.com>
2+
3+
Add crash diagnostics for debugging unexpected zapped cells.
4+
https://bugs.webkit.org/show_bug.cgi?id=200149
5+
<rdar://problem/53570112>
6+
7+
Reviewed by Yusuke Suzuki.
8+
9+
Add a check for zapped cells in SlotVisitor::appendToMarkStack() and
10+
SlotVisitor::visitChildren(). If a zapped cell is detected, we will crash with
11+
some diagnostic info.
12+
13+
To facilitate this, we've made the following changes:
14+
1. Changed FreeCell to preserve the 1st 8 bytes. This is fine to do because all
15+
cells are at least 16 bytes long.
16+
2. Changed HeapCell::zap() to only zap the structureID. Leave the rest of the
17+
cell header info intact (including the cell JSType).
18+
3. Changed HeapCell::zap() to record the reason for zapping the cell. We stash
19+
the reason immediately after the first 8 bytes. This is the same location as
20+
FreeCell::scrambledNext. However, since a cell is not expected to be zapped
21+
and on the free list at the same time, it is also fine to do this.
22+
4. Added a few utility functions to MarkedBlock for checking if a cell points
23+
into the block.
24+
5. Added VMInspector and JSDollarVM utilities to dump in-use subspace hashes.
25+
6. Added some comments to document the hashes of known subspaces.
26+
7. Added Options::dumpZappedCellCrashData() to make this check conditional.
27+
We use this option to disable this check for slower machines so that their
28+
PLT5 performance is not impacted.
29+
30+
* assembler/CPU.cpp:
31+
(JSC::hwL3CacheSize):
32+
(JSC::hwPhysicalCPUMax):
33+
* assembler/CPU.h:
34+
(JSC::hwL3CacheSize):
35+
(JSC::hwPhysicalCPUMax):
36+
* heap/FreeList.h:
37+
(JSC::FreeCell::offsetOfScrambledNext):
38+
* heap/HeapCell.h:
39+
(JSC::HeapCell::zap):
40+
(JSC::HeapCell::isZapped const):
41+
* heap/MarkedBlock.cpp:
42+
(JSC::MarkedBlock::Handle::stopAllocating):
43+
* heap/MarkedBlock.h:
44+
(JSC::MarkedBlock::Handle::start const):
45+
(JSC::MarkedBlock::Handle::end const):
46+
(JSC::MarkedBlock::Handle::contains const):
47+
* heap/MarkedBlockInlines.h:
48+
(JSC::MarkedBlock::Handle::specializedSweep):
49+
* heap/MarkedSpace.h:
50+
(JSC::MarkedSpace::forEachSubspace):
51+
* heap/SlotVisitor.cpp:
52+
(JSC::SlotVisitor::appendToMarkStack):
53+
(JSC::SlotVisitor::visitChildren):
54+
(JSC::SlotVisitor::reportZappedCellAndCrash):
55+
* heap/SlotVisitor.h:
56+
* jit/AssemblyHelpers.cpp:
57+
(JSC::AssemblyHelpers::emitAllocateWithNonNullAllocator):
58+
* runtime/Options.cpp:
59+
(JSC::Options::initialize):
60+
* runtime/Options.h:
61+
* runtime/VM.cpp:
62+
(JSC::VM::VM):
63+
* tools/JSDollarVM.cpp:
64+
(JSC::functionDumpSubspaceHashes):
65+
(JSC::JSDollarVM::finishCreation):
66+
* tools/VMInspector.cpp:
67+
(JSC::VMInspector::dumpSubspaceHashes):
68+
* tools/VMInspector.h:
69+
170
2019-08-01 Keith Miller <keith_miller@apple.com>
271

372
Fix bug in testMulImm32SignExtend

‎Source/JavaScriptCore/assembler/CPU.cpp‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,27 @@ int kernTCSMAwareNumberOfProcessorCores()
6666
});
6767
return result;
6868
}
69+
70+
int64_t hwL3CacheSize()
71+
{
72+
int64_t val = 0;
73+
size_t valSize = sizeof(val);
74+
int rc = sysctlbyname("hw.l3cachesize", &val, &valSize, nullptr, 0);
75+
if (rc < 0)
76+
return 0;
77+
return val;
78+
}
79+
80+
int32_t hwPhysicalCPUMax()
81+
{
82+
int64_t val = 0;
83+
size_t valSize = sizeof(val);
84+
int rc = sysctlbyname("hw.physicalcpu_max", &val, &valSize, nullptr, 0);
85+
if (rc < 0)
86+
return 0;
87+
return val;
88+
}
89+
6990
#endif // #if (CPU(X86) || CPU(X86_64)) && OS(DARWIN)
7091

7192
} // namespace JSC

‎Source/JavaScriptCore/assembler/CPU.h‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2008-2017 Apple Inc. All rights reserved.
2+
* Copyright (C) 2008-2019 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -145,10 +145,14 @@ inline bool hasSensibleDoubleToInt()
145145
bool isKernTCSMAvailable();
146146
bool enableKernTCSM();
147147
int kernTCSMAwareNumberOfProcessorCores();
148+
int64_t hwL3CacheSize();
149+
int32_t hwPhysicalCPUMax();
148150
#else
149151
ALWAYS_INLINE bool isKernTCSMAvailable() { return false; }
150152
ALWAYS_INLINE bool enableKernTCSM() { return false; }
151153
ALWAYS_INLINE int kernTCSMAwareNumberOfProcessorCores() { return WTF::numberOfProcessorCores(); }
154+
ALWAYS_INLINE int64_t hwL3CacheSize() { return 0; }
155+
ALWAYS_INLINE int32_t hwPhysicalCPUMax() { return kernTCSMAwareNumberOfProcessorCores(); }
152156
#endif
153157

154158
} // namespace JSC

‎Source/JavaScriptCore/heap/FreeList.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2016-2017 Apple Inc. All rights reserved.
2+
* Copyright (C) 2016-2019 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -53,6 +53,9 @@ struct FreeCell {
5353
return descramble(scrambledNext, secret);
5454
}
5555

56+
static ptrdiff_t offsetOfScrambledNext() { return OBJECT_OFFSETOF(FreeCell, scrambledNext); }
57+
58+
uint64_t preservedBitsForCrashAnalysis;
5659
uintptr_t scrambledNext;
5760
};
5861

‎Source/JavaScriptCore/heap/HeapCell.h‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2016-2018 Apple Inc. All rights reserved.
2+
* Copyright (C) 2016-2019 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -47,8 +47,17 @@ class HeapCell {
4747

4848
HeapCell() { }
4949

50-
void zap() { *reinterpret_cast_ptr<uintptr_t**>(this) = 0; }
51-
bool isZapped() const { return !*reinterpret_cast_ptr<uintptr_t* const*>(this); }
50+
// We're intentionally only zapping the bits for the structureID and leaving
51+
// the rest of the cell header bits intact for crash analysis uses.
52+
enum ZapReason : int8_t { Unspecified, Destruction, StopAllocating };
53+
void zap(ZapReason reason)
54+
{
55+
uint32_t* cellWords = bitwise_cast<uint32_t*>(this);
56+
cellWords[0] = 0;
57+
// Leaving cellWords[1] alone for crash analysis if needed.
58+
cellWords[2] = reason;
59+
}
60+
bool isZapped() const { return !*bitwise_cast<const uint32_t*>(this); }
5261

5362
bool isLive();
5463

‎Source/JavaScriptCore/heap/MarkedBlock.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ void MarkedBlock::Handle::stopAllocating(const FreeList& freeList)
161161
if (MarkedBlockInternal::verbose)
162162
dataLog("Free cell: ", RawPointer(cell), "\n");
163163
if (m_attributes.destruction == NeedsDestruction)
164-
cell->zap();
164+
cell->zap(HeapCell::StopAllocating);
165165
block().clearNewlyAllocated(cell);
166166
});
167167

‎Source/JavaScriptCore/heap/MarkedBlock.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,10 @@ class MarkedBlock {
198198
void didAddToDirectory(BlockDirectory*, size_t index);
199199
void didRemoveFromDirectory();
200200

201+
void* start() const { return &m_block->atoms()[0]; }
202+
void* end() const { return &m_block->atoms()[m_endAtom]; }
203+
bool contains(void* p) const { return start() <= p && p < end(); }
204+
201205
void dumpState(PrintStream&);
202206

203207
private:

‎Source/JavaScriptCore/heap/MarkedBlockInlines.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (C) 2016-2018 Apple Inc. All rights reserved.
2+
* Copyright (C) 2016-2019 Apple Inc. All rights reserved.
33
*
44
* Redistribution and use in source and binary forms, with or without
55
* modification, are permitted provided that the following conditions
@@ -258,7 +258,7 @@ void MarkedBlock::Handle::specializedSweep(FreeList* freeList, MarkedBlock::Hand
258258
JSCell* jsCell = static_cast<JSCell*>(cell);
259259
if (!jsCell->isZapped()) {
260260
destroyFunc(vm, jsCell);
261-
jsCell->zap();
261+
jsCell->zap(HeapCell::Destruction);
262262
}
263263
};
264264

‎Source/JavaScriptCore/heap/MarkedSpace.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ class MarkedSpace {
124124
template<typename Functor> void forEachLiveCell(HeapIterationScope&, const Functor&);
125125
template<typename Functor> void forEachDeadCell(HeapIterationScope&, const Functor&);
126126
template<typename Functor> void forEachBlock(const Functor&);
127+
template<typename Functor> void forEachSubspace(const Functor&);
127128

128129
void shrink();
129130
void freeBlock(MarkedBlock::Handle*);
@@ -240,6 +241,16 @@ void MarkedSpace::forEachDirectory(const Functor& functor)
240241
}
241242
}
242243

244+
template<typename Functor>
245+
void MarkedSpace::forEachSubspace(const Functor& functor)
246+
{
247+
for (auto subspace : m_subspaces) {
248+
if (functor(*subspace) == IterationStatus::Done)
249+
return;
250+
}
251+
}
252+
253+
243254
ALWAYS_INLINE size_t MarkedSpace::optimalSizeFor(size_t bytes)
244255
{
245256
ASSERT(bytes);

‎Source/JavaScriptCore/heap/SlotVisitor.cpp‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -286,8 +286,14 @@ template<typename ContainerType>
286286
ALWAYS_INLINE void SlotVisitor::appendToMarkStack(ContainerType& container, JSCell* cell)
287287
{
288288
ASSERT(m_heap.isMarked(cell));
289+
#if CPU(X86_64)
290+
if (Options::dumpZappedCellCrashData()) {
291+
if (UNLIKELY(cell->isZapped()))
292+
reportZappedCellAndCrash(cell);
293+
}
294+
#endif
289295
ASSERT(!cell->isZapped());
290-
296+
291297
container.noteMarked();
292298

293299
m_visitCount++;
@@ -385,6 +391,17 @@ ALWAYS_INLINE void SlotVisitor::visitChildren(const JSCell* cell)
385391
default:
386392
// FIXME: This could be so much better.
387393
// https://bugs.webkit.org/show_bug.cgi?id=162462
394+
#if CPU(X86_64)
395+
if (Options::dumpZappedCellCrashData()) {
396+
Structure* structure = cell->structure(vm());
397+
if (LIKELY(structure)) {
398+
const MethodTable* methodTable = &structure->classInfo()->methodTable;
399+
methodTable->visitChildren(const_cast<JSCell*>(cell), *this);
400+
break;
401+
}
402+
reportZappedCellAndCrash(const_cast<JSCell*>(cell));
403+
}
404+
#endif
388405
cell->methodTable(vm())->visitChildren(const_cast<JSCell*>(cell), *this);
389406
break;
390407
}
@@ -804,4 +821,33 @@ void SlotVisitor::addParallelConstraintTask(RefPtr<SharedTask<void(SlotVisitor&)
804821
m_currentSolver->addParallelTask(task, *m_currentConstraint);
805822
}
806823

824+
#if CPU(X86_64)
825+
NEVER_INLINE NO_RETURN_DUE_TO_CRASH NOT_TAIL_CALLED void SlotVisitor::reportZappedCellAndCrash(JSCell* cell)
826+
{
827+
MarkedBlock::Handle* foundBlock = nullptr;
828+
uint32_t* cellWords = reinterpret_cast_ptr<uint32_t*>(this);
829+
830+
uintptr_t cellAddress = bitwise_cast<uintptr_t>(cell);
831+
uintptr_t headerWord = cellWords[1];
832+
uintptr_t zapReason = cellWords[2];
833+
unsigned subspaceHash = 0;
834+
size_t cellSize = 0;
835+
836+
m_heap.objectSpace().forEachBlock([&] (MarkedBlock::Handle* block) {
837+
if (block->contains(cell)) {
838+
foundBlock = block;
839+
return IterationStatus::Done;
840+
}
841+
return IterationStatus::Continue;
842+
});
843+
844+
if (foundBlock) {
845+
subspaceHash = StringHasher::computeHash(foundBlock->subspace()->name());
846+
cellSize = foundBlock->cellSize();
847+
}
848+
849+
CRASH_WITH_INFO(cellAddress, headerWord, zapReason, subspaceHash, cellSize);
850+
}
851+
#endif // PLATFORM(MAC)
852+
807853
} // namespace JSC

0 commit comments

Comments
 (0)