Diff
Modified: trunk/Source/_javascript_Core/ChangeLog (227717 => 227718)
--- trunk/Source/_javascript_Core/ChangeLog 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/ChangeLog 2018-01-28 19:08:08 UTC (rev 227718)
@@ -1,5 +1,57 @@
2018-01-27 Filip Pizlo <[email protected]>
+ Make MarkedBlock::Footer bigger
+ https://bugs.webkit.org/show_bug.cgi?id=182220
+
+ Reviewed by JF Bastien.
+
+ This makes the block footer larger by moving the newlyAllocated bits from the handle into
+ the footer.
+
+ It used to be profitable to put anything we could into the handle because that would free up
+ payload space inside the block. But now that we want to use the footer for padding, it's
+ profitable to put GC state information - especially data that is used by the GC itself and so
+ is not useful for a Spectre attack - into the footer to increase object distancing.
+
+ * heap/CellContainer.cpp:
+ (JSC::CellContainer::isNewlyAllocated const):
+ * heap/IsoCellSet.cpp:
+ (JSC::IsoCellSet::sweepToFreeList):
+ * heap/MarkedBlock.cpp:
+ (JSC::MarkedBlock::Handle::Handle):
+ (JSC::MarkedBlock::Footer::Footer):
+ (JSC::MarkedBlock::Handle::stopAllocating):
+ (JSC::MarkedBlock::Handle::lastChanceToFinalize):
+ (JSC::MarkedBlock::Handle::resumeAllocating):
+ (JSC::MarkedBlock::aboutToMarkSlow):
+ (JSC::MarkedBlock::resetAllocated):
+ (JSC::MarkedBlock::Handle::resetAllocated): Deleted.
+ * heap/MarkedBlock.h:
+ (JSC::MarkedBlock::newlyAllocatedVersion const):
+ (JSC::MarkedBlock::isNewlyAllocated):
+ (JSC::MarkedBlock::setNewlyAllocated):
+ (JSC::MarkedBlock::clearNewlyAllocated):
+ (JSC::MarkedBlock::newlyAllocated const):
+ (JSC::MarkedBlock::Handle::newlyAllocatedVersion const): Deleted.
+ (JSC::MarkedBlock::Handle::isNewlyAllocated): Deleted.
+ (JSC::MarkedBlock::Handle::setNewlyAllocated): Deleted.
+ (JSC::MarkedBlock::Handle::clearNewlyAllocated): Deleted.
+ (JSC::MarkedBlock::Handle::newlyAllocated const): Deleted.
+ * heap/MarkedBlockInlines.h:
+ (JSC::MarkedBlock::isNewlyAllocatedStale const):
+ (JSC::MarkedBlock::hasAnyNewlyAllocated):
+ (JSC::MarkedBlock::Handle::isLive):
+ (JSC::MarkedBlock::Handle::specializedSweep):
+ (JSC::MarkedBlock::Handle::newlyAllocatedMode):
+ (JSC::MarkedBlock::Handle::isNewlyAllocatedStale const): Deleted.
+ (JSC::MarkedBlock::Handle::hasAnyNewlyAllocated): Deleted.
+ * heap/MarkedSpace.cpp:
+ (JSC::MarkedSpace::endMarking):
+ * heap/SlotVisitor.cpp:
+ (JSC::SlotVisitor::appendJSCellOrAuxiliary):
+
+2018-01-27 Filip Pizlo <[email protected]>
+
MarkedBlock should have a footer instead of a header
https://bugs.webkit.org/show_bug.cgi?id=182217
Modified: trunk/Source/_javascript_Core/heap/CellContainer.cpp (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/CellContainer.cpp 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/CellContainer.cpp 2018-01-28 19:08:08 UTC (rev 227718)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2016 Apple Inc. All rights reserved.
+ * Copyright (C) 2016-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -34,9 +34,9 @@
{
if (isLargeAllocation())
return largeAllocation().isNewlyAllocated();
- MarkedBlock::Handle& handle = markedBlock().handle();
- return !handle.isNewlyAllocatedStale()
- && handle.isNewlyAllocated(cell);
+ MarkedBlock& block = markedBlock();
+ return !block.isNewlyAllocatedStale()
+ && block.isNewlyAllocated(cell);
}
} // namespace JSC
Modified: trunk/Source/_javascript_Core/heap/IsoCellSet.cpp (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/IsoCellSet.cpp 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/IsoCellSet.cpp 2018-01-28 19:08:08 UTC (rev 227718)
@@ -126,8 +126,8 @@
RELEASE_ASSERT_NOT_REACHED();
}
- if (block->hasAnyNewlyAllocated()) {
- m_bits[block->index()]->concurrentFilter(block->newlyAllocated());
+ if (block->block().hasAnyNewlyAllocated()) {
+ m_bits[block->index()]->concurrentFilter(block->block().newlyAllocated());
return;
}
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.cpp (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.cpp 2018-01-28 19:08:08 UTC (rev 227718)
@@ -62,7 +62,6 @@
MarkedBlock::Handle::Handle(Heap& heap, AlignedMemoryAllocator* alignedMemoryAllocator, void* blockSpace)
: m_alignedMemoryAllocator(alignedMemoryAllocator)
, m_weakSet(heap.vm(), CellContainer())
- , m_newlyAllocatedVersion(MarkedSpace::nullVersion)
{
m_block = new (NotNull, blockSpace) MarkedBlock(*heap.vm(), *this);
@@ -101,6 +100,7 @@
: m_handle(handle)
, m_vm(&vm)
, m_markingVersion(MarkedSpace::nullVersion)
+ , m_newlyAllocatedVersion(MarkedSpace::nullVersion)
{
}
@@ -144,12 +144,12 @@
// allocated from our free list are not currently marked, so we need another
// way to tell what's live vs dead.
- m_newlyAllocated.clearAll();
- m_newlyAllocatedVersion = heap()->objectSpace().newlyAllocatedVersion();
+ blockFooter().m_newlyAllocated.clearAll();
+ blockFooter().m_newlyAllocatedVersion = heap()->objectSpace().newlyAllocatedVersion();
forEachCell(
[&] (HeapCell* cell, HeapCell::Kind) -> IterationStatus {
- setNewlyAllocated(cell);
+ block().setNewlyAllocated(cell);
return IterationStatus::Continue;
});
@@ -159,7 +159,7 @@
dataLog("Free cell: ", RawPointer(cell), "\n");
if (m_attributes.destruction == NeedsDestruction)
cell->zap();
- clearNewlyAllocated(cell);
+ block().clearNewlyAllocated(cell);
});
m_isFreeListed = false;
@@ -173,8 +173,8 @@
block().clearHasAnyMarked();
blockFooter().m_markingVersion = heap()->objectSpace().markingVersion();
m_weakSet.lastChanceToFinalize();
- m_newlyAllocated.clearAll();
- m_newlyAllocatedVersion = heap()->objectSpace().newlyAllocatedVersion();
+ blockFooter().m_newlyAllocated.clearAll();
+ blockFooter().m_newlyAllocatedVersion = heap()->objectSpace().newlyAllocatedVersion();
sweep(nullptr);
}
@@ -188,7 +188,7 @@
ASSERT(!directory()->isAllocated(NoLockingNecessary, this));
ASSERT(!isFreeListed());
- if (!hasAnyNewlyAllocated()) {
+ if (!block().hasAnyNewlyAllocated()) {
if (false)
dataLog("There ain't no newly allocated.\n");
// This means we had already exhausted the block when we stopped allocation.
@@ -236,7 +236,7 @@
if (false)
dataLog(RawPointer(this), ": Doing things.\n");
HeapVersion newlyAllocatedVersion = space()->newlyAllocatedVersion();
- if (handle().m_newlyAllocatedVersion == newlyAllocatedVersion) {
+ if (footer().m_newlyAllocatedVersion == newlyAllocatedVersion) {
// When do we get here? The block could not have been filled up. The newlyAllocated bits would
// have had to be created since the end of the last collection. The only things that create
// them are aboutToMarkSlow, lastChanceToFinalize, and stopAllocating. If it had been
@@ -244,11 +244,11 @@
// cannot be lastChanceToFinalize. So it must be stopAllocating. That means that we just
// computed the newlyAllocated bits just before the start of an increment. When we are in that
// mode, it seems as if newlyAllocated should subsume marks.
- ASSERT(handle().m_newlyAllocated.subsumes(footer().m_marks));
+ ASSERT(footer().m_newlyAllocated.subsumes(footer().m_marks));
footer().m_marks.clearAll();
} else {
- handle().m_newlyAllocated.setAndClear(footer().m_marks);
- handle().m_newlyAllocatedVersion = newlyAllocatedVersion;
+ footer().m_newlyAllocated.setAndClear(footer().m_marks);
+ footer().m_newlyAllocatedVersion = newlyAllocatedVersion;
}
}
clearHasAnyMarked();
@@ -259,10 +259,10 @@
directory->setIsMarkingNotEmpty(holdLock(directory->bitvectorLock()), &handle(), true);
}
-void MarkedBlock::Handle::resetAllocated()
+void MarkedBlock::resetAllocated()
{
- m_newlyAllocated.clearAll();
- m_newlyAllocatedVersion = MarkedSpace::nullVersion;
+ footer().m_newlyAllocated.clearAll();
+ footer().m_newlyAllocatedVersion = MarkedSpace::nullVersion;
}
void MarkedBlock::resetMarks()
Modified: trunk/Source/_javascript_Core/heap/MarkedBlock.h (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/MarkedBlock.h 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/MarkedBlock.h 2018-01-28 19:08:08 UTC (rev 227718)
@@ -174,18 +174,6 @@
bool isFreeListedCell(const void* target) const;
- bool isNewlyAllocated(const void*);
- void setNewlyAllocated(const void*);
- void clearNewlyAllocated(const void*);
- const Bitmap<atomsPerBlock>& newlyAllocated() const;
-
- HeapVersion newlyAllocatedVersion() const { return m_newlyAllocatedVersion; }
-
- inline bool isNewlyAllocatedStale() const;
-
- inline bool hasAnyNewlyAllocated();
- void resetAllocated();
-
template <typename Functor> IterationStatus forEachCell(const Functor&);
template <typename Functor> inline IterationStatus forEachLiveCell(const Functor&);
template <typename Functor> inline IterationStatus forEachDeadCell(const Functor&);
@@ -232,8 +220,6 @@
size_t m_atomsPerCell { std::numeric_limits<size_t>::max() };
size_t m_endAtom { std::numeric_limits<size_t>::max() }; // This is a fuzzy end. Always test for < m_endAtom.
- Bitmap<atomsPerBlock> m_newlyAllocated;
-
CellAttributes m_attributes;
bool m_isFreeListed { false };
@@ -242,8 +228,6 @@
size_t m_index { std::numeric_limits<size_t>::max() };
WeakSet m_weakSet;
- HeapVersion m_newlyAllocatedVersion;
-
MarkedBlock* m_block { nullptr };
};
@@ -296,8 +280,10 @@
int16_t m_markCountBias;
HeapVersion m_markingVersion;
+ HeapVersion m_newlyAllocatedVersion;
Bitmap<atomsPerBlock> m_marks;
+ Bitmap<atomsPerBlock> m_newlyAllocated;
};
private:
@@ -330,6 +316,18 @@
bool isAtom(const void*);
void clearMarked(const void*);
+ bool isNewlyAllocated(const void*);
+ void setNewlyAllocated(const void*);
+ void clearNewlyAllocated(const void*);
+ const Bitmap<atomsPerBlock>& newlyAllocated() const;
+
+ HeapVersion newlyAllocatedVersion() const { return footer().m_newlyAllocatedVersion; }
+
+ inline bool isNewlyAllocatedStale() const;
+
+ inline bool hasAnyNewlyAllocated();
+ void resetAllocated();
+
size_t cellSize();
const CellAttributes& attributes() const;
@@ -584,24 +582,24 @@
return footer().m_marks;
}
-inline bool MarkedBlock::Handle::isNewlyAllocated(const void* p)
+inline bool MarkedBlock::isNewlyAllocated(const void* p)
{
- return m_newlyAllocated.get(m_block->atomNumber(p));
+ return footer().m_newlyAllocated.get(atomNumber(p));
}
-inline void MarkedBlock::Handle::setNewlyAllocated(const void* p)
+inline void MarkedBlock::setNewlyAllocated(const void* p)
{
- m_newlyAllocated.set(m_block->atomNumber(p));
+ footer().m_newlyAllocated.set(atomNumber(p));
}
-inline void MarkedBlock::Handle::clearNewlyAllocated(const void* p)
+inline void MarkedBlock::clearNewlyAllocated(const void* p)
{
- m_newlyAllocated.clear(m_block->atomNumber(p));
+ footer().m_newlyAllocated.clear(atomNumber(p));
}
-inline const Bitmap<MarkedBlock::atomsPerBlock>& MarkedBlock::Handle::newlyAllocated() const
+inline const Bitmap<MarkedBlock::atomsPerBlock>& MarkedBlock::newlyAllocated() const
{
- return m_newlyAllocated;
+ return footer().m_newlyAllocated;
}
inline bool MarkedBlock::isAtom(const void* p)
Modified: trunk/Source/_javascript_Core/heap/MarkedBlockInlines.h (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/MarkedBlockInlines.h 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/MarkedBlockInlines.h 2018-01-28 19:08:08 UTC (rev 227718)
@@ -40,12 +40,12 @@
return MarkedSpace::blockPayload / cellSize();
}
-inline bool MarkedBlock::Handle::isNewlyAllocatedStale() const
+inline bool MarkedBlock::isNewlyAllocatedStale() const
{
- return m_newlyAllocatedVersion != space()->newlyAllocatedVersion();
+ return footer().m_newlyAllocatedVersion != space()->newlyAllocatedVersion();
}
-inline bool MarkedBlock::Handle::hasAnyNewlyAllocated()
+inline bool MarkedBlock::hasAnyNewlyAllocated()
{
return !isNewlyAllocatedStale();
}
@@ -143,19 +143,18 @@
auto count = footer.m_lock.tryOptimisticFencelessRead();
if (count.value) {
Dependency fenceBefore = Dependency::fence(count.input);
+ MarkedBlock& fencedBlock = *fenceBefore.consume(&block);
+ MarkedBlock::Footer& fencedFooter = fencedBlock.footer();
MarkedBlock::Handle* fencedThis = fenceBefore.consume(this);
- ASSERT(!fencedThis->isFreeListed());
+ ASSERT_UNUSED(fencedThis, !fencedThis->isFreeListed());
- HeapVersion myNewlyAllocatedVersion = fencedThis->m_newlyAllocatedVersion;
+ HeapVersion myNewlyAllocatedVersion = fencedFooter.m_newlyAllocatedVersion;
if (myNewlyAllocatedVersion == newlyAllocatedVersion) {
- bool result = fencedThis->isNewlyAllocated(cell);
+ bool result = fencedBlock.isNewlyAllocated(cell);
if (footer.m_lock.fencelessValidate(count.value, Dependency::fence(result)))
return result;
} else {
- MarkedBlock& fencedBlock = *fenceBefore.consume(&block);
- MarkedBlock::Footer& fencedFooter = fencedBlock.footer();
-
HeapVersion myMarkingVersion = fencedFooter.m_markingVersion;
if (myMarkingVersion != markingVersion
&& (!isMarking || !fencedBlock.marksConveyLivenessDuringMarking(myMarkingVersion, markingVersion))) {
@@ -173,9 +172,9 @@
ASSERT(!isFreeListed());
- HeapVersion myNewlyAllocatedVersion = m_newlyAllocatedVersion;
+ HeapVersion myNewlyAllocatedVersion = footer.m_newlyAllocatedVersion;
if (myNewlyAllocatedVersion == newlyAllocatedVersion)
- return isNewlyAllocated(cell);
+ return block.isNewlyAllocated(cell);
if (block.areMarksStale(markingVersion)) {
if (!isMarking)
@@ -326,7 +325,7 @@
for (size_t i = 0; i < m_endAtom; i += m_atomsPerCell) {
if (emptyMode == NotEmpty
&& ((marksMode == MarksNotStale && footer.m_marks.get(i))
- || (newlyAllocatedMode == HasNewlyAllocated && m_newlyAllocated.get(i)))) {
+ || (newlyAllocatedMode == HasNewlyAllocated && footer.m_newlyAllocated.get(i)))) {
isEmpty = false;
continue;
}
@@ -340,7 +339,7 @@
// We only want to discard the newlyAllocated bits if we're creating a FreeList,
// otherwise we would lose information on what's currently alive.
if (sweepMode == SweepToFreeList && newlyAllocatedMode == HasNewlyAllocated)
- m_newlyAllocatedVersion = MarkedSpace::nullVersion;
+ footer.m_newlyAllocatedVersion = MarkedSpace::nullVersion;
if (space()->isMarking())
footer.m_lock.unlock();
@@ -464,7 +463,7 @@
inline MarkedBlock::Handle::NewlyAllocatedMode MarkedBlock::Handle::newlyAllocatedMode()
{
- return hasAnyNewlyAllocated() ? HasNewlyAllocated : DoesNotHaveNewlyAllocated;
+ return block().hasAnyNewlyAllocated() ? HasNewlyAllocated : DoesNotHaveNewlyAllocated;
}
inline MarkedBlock::Handle::MarksMode MarkedBlock::Handle::marksMode()
Modified: trunk/Source/_javascript_Core/heap/MarkedSpace.cpp (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/MarkedSpace.cpp 2018-01-28 19:08:08 UTC (rev 227718)
@@ -44,6 +44,11 @@
[] {
result = new Vector<size_t>();
+ if (Options::dumpSizeClasses()) {
+ dataLog("Block size: ", MarkedBlock::blockSize, "\n");
+ dataLog("Footer size: ", sizeof(MarkedBlock::Footer), "\n");
+ }
+
auto add = [&] (size_t sizeClass) {
sizeClass = WTF::roundUpToMultipleOf<MarkedBlock::atomSize>(sizeClass);
if (Options::dumpSizeClasses())
@@ -424,7 +429,7 @@
if (UNLIKELY(nextVersion(m_newlyAllocatedVersion) == initialVersion)) {
forEachBlock(
[&] (MarkedBlock::Handle* handle) {
- handle->resetAllocated();
+ handle->block().resetAllocated();
});
}
Modified: trunk/Source/_javascript_Core/heap/SlotVisitor.cpp (227717 => 227718)
--- trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2018-01-28 02:23:25 UTC (rev 227717)
+++ trunk/Source/_javascript_Core/heap/SlotVisitor.cpp 2018-01-28 19:08:08 UTC (rev 227718)
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2012-2017 Apple Inc. All rights reserved.
+ * Copyright (C) 2012-2018 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -170,8 +170,8 @@
out.print("Is marked raw: ", block.isMarkedRaw(jsCell), "\n");
out.print("Marking version: ", block.markingVersion(), "\n");
out.print("Heap marking version: ", heap()->objectSpace().markingVersion(), "\n");
- out.print("Is newly allocated raw: ", block.handle().isNewlyAllocated(jsCell), "\n");
- out.print("Newly allocated version: ", block.handle().newlyAllocatedVersion(), "\n");
+ out.print("Is newly allocated raw: ", block.isNewlyAllocated(jsCell), "\n");
+ out.print("Newly allocated version: ", block.newlyAllocatedVersion(), "\n");
out.print("Heap newly allocated version: ", heap()->objectSpace().newlyAllocatedVersion(), "\n");
}
UNREACHABLE_FOR_PLATFORM();
