mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 844755 - Make mark bitmaps atomic r=sfink 

Currently there is a "benign" race between background sweeping and gray
unmarking. The latter does not affect the result of the former, but any data
race is undefined behaviour.

This patch changes the marking bitsmaps to be relaxed atomic to avoid this.
Marking bitmaps were already excluded from TSAN checks.

Differential Revision: https://phabricator.services.mozilla.com/D90239
This commit is contained in:
Jon Coppeard 2020-09-15 17:04:12 +00:00
Родитель f4b4008f5e
Коммит 9dd11c07bc
8 изменённых файлов: 87 добавлений и 61 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

23
js/public/HeapAPI.h
Просмотреть файл

@ -7,6 +7,8 @@
#ifndef js_HeapAPI_h
#define js_HeapAPI_h
#include "mozilla/Atomics.h"
#include <limits.h>
#include <type_traits>
@ -332,19 +334,26 @@ inline bool operator!=(const JS::GCCellPtr& ptr1, const JS::GCCellPtr& ptr2) {
namespace js {
namespace gc {
// Mark bitmaps are atomic because they can be written by gray unmarking on the
// main thread while read by sweeping on a background thread. The former does
// not affect the result of the latter.
using MarkBitmapWord = mozilla::Atomic<uintptr_t, mozilla::Relaxed>;
namespace detail {
static MOZ_ALWAYS_INLINE uintptr_t* GetGCThingMarkBitmap(const uintptr_t addr) {
static MOZ_ALWAYS_INLINE MarkBitmapWord* GetGCThingMarkBitmap(
const uintptr_t addr) {
// Note: the JIT pre-barrier trampolines inline this code. Update that
// code too when making changes here!
MOZ_ASSERT(addr);
const uintptr_t bmap_addr = (addr & ~ChunkMask) | ChunkMarkBitmapOffset;
return reinterpret_cast<uintptr_t*>(bmap_addr);
return reinterpret_cast<MarkBitmapWord*>(bmap_addr);
}
static MOZ_ALWAYS_INLINE void GetGCThingMarkWordAndMask(const uintptr_t addr,
ColorBit colorBit,
uintptr_t** wordp,
MarkBitmapWord** wordp,
uintptr_t* maskp) {
// Note: the JIT pre-barrier trampolines inline this code. Update that
// code too when making changes here!
@ -352,7 +361,7 @@ static MOZ_ALWAYS_INLINE void GetGCThingMarkWordAndMask(const uintptr_t addr,
const size_t bit = (addr & js::gc::ChunkMask) / js::gc::CellBytesPerMarkBit +
static_cast<uint32_t>(colorBit);
MOZ_ASSERT(bit < js::gc::ChunkMarkBitmapBits);
uintptr_t* bitmap = GetGCThingMarkBitmap(addr);
MarkBitmapWord* bitmap = GetGCThingMarkBitmap(addr);
const uintptr_t nbits = sizeof(*bitmap) * CHAR_BIT;
*maskp = uintptr_t(1) << (bit % nbits);
*wordp = &bitmap[bit / nbits];
@ -369,14 +378,16 @@ static MOZ_ALWAYS_INLINE bool TenuredCellIsMarkedGray(const Cell* cell) {
MOZ_ASSERT(cell);
MOZ_ASSERT(!js::gc::IsInsideNursery(cell));
uintptr_t *grayWord, grayMask;
MarkBitmapWord* grayWord;
uintptr_t grayMask;
js::gc::detail::GetGCThingMarkWordAndMask(
uintptr_t(cell), js::gc::ColorBit::GrayOrBlackBit, &grayWord, &grayMask);
if (!(*grayWord & grayMask)) {
return false;
}
uintptr_t *blackWord, blackMask;
MarkBitmapWord* blackWord;
uintptr_t blackMask;
js::gc::detail::GetGCThingMarkWordAndMask(
uintptr_t(cell), js::gc::ColorBit::BlackBit, &blackWord, &blackMask);
return !(*blackWord & blackMask);

16
js/src/ds/Bitmap.cpp
Просмотреть файл

@ -101,19 +101,3 @@ void SparseBitmap::bitwiseOrInto(DenseBitmap& other) const {
}
}
}
void SparseBitmap::bitwiseOrRangeInto(size_t wordStart, size_t numWords,
uintptr_t* target) const {
size_t blockWord = blockStartWord(wordStart);
// We only support using a single bit block in this API.
MOZ_ASSERT(numWords &&
(blockWord == blockStartWord(wordStart + numWords - 1)));
BitBlock* block = getBlock(blockWord / WordsInBlock);
if (block) {
for (size_t i = 0; i < numWords; i++) {
target[i] |= (*block)[wordStart - blockWord + i];
}
}
}

34
js/src/ds/Bitmap.h
Просмотреть файл

@ -18,6 +18,7 @@
#include "js/AllocPolicy.h"
#include "js/HashTable.h"
#include "js/HeapAPI.h"
#include "js/Vector.h"
// This file provides two classes for representing bitmaps.
@ -50,15 +51,18 @@ class DenseBitmap {
uintptr_t word(size_t i) const { return data[i]; }
uintptr_t& word(size_t i) { return data[i]; }
void copyBitsFrom(size_t wordStart, size_t numWords, uintptr_t* source) {
template <typename T>
typename std::enable_if_t<std::is_convertible_v<T, uintptr_t>, void>
copyBitsFrom(size_t wordStart, size_t numWords, T* source) {
MOZ_ASSERT(wordStart + numWords <= data.length());
// Use std::copy and not std::copy_n because the former requires no
// overlap and so provides extra opportunity to optimize.
std::copy(source, source + numWords, &data[wordStart]);
for (size_t i = 0; i < numWords; i++) {
data[wordStart + i] = source[i];
}
}
void bitwiseOrRangeInto(size_t wordStart, size_t numWords,
uintptr_t* target) const {
template <typename T>
typename std::enable_if_t<std::is_convertible_v<T, uintptr_t>, void>
bitwiseOrRangeInto(size_t wordStart, size_t numWords, T* target) const {
for (size_t i = 0; i < numWords; i++) {
target[i] |= data[wordStart + i];
}
@ -149,8 +153,22 @@ class SparseBitmap {
// Currently, this API only supports a range of words that is in a single bit
// block.
void bitwiseOrRangeInto(size_t wordStart, size_t numWords,
uintptr_t* target) const;
template <typename T>
typename std::enable_if_t<std::is_convertible_v<T, uintptr_t>, void>
bitwiseOrRangeInto(size_t wordStart, size_t numWords, T* target) const {
size_t blockWord = blockStartWord(wordStart);
// We only support using a single bit block in this API.
MOZ_ASSERT(numWords &&
(blockWord == blockStartWord(wordStart + numWords - 1)));
BitBlock* block = getBlock(blockWord / WordsInBlock);
if (block) {
for (size_t i = 0; i < numWords; i++) {
target[i] |= (*block)[wordStart - blockWord + i];
}
}
}
};
} // namespace js

4
js/src/gc/AtomMarking.cpp
Просмотреть файл

@ -86,7 +86,7 @@ bool AtomMarkingRuntime::computeBitmapFromChunkMarkBits(JSRuntime* runtime,
for (auto thingKind : AllAllocKinds()) {
for (ArenaIter aiter(atomsZone, thingKind); !aiter.done(); aiter.next()) {
Arena* arena = aiter.get();
uintptr_t* chunkWords = arena->chunk()->bitmap.arenaBits(arena);
MarkBitmapWord* chunkWords = arena->chunk()->bitmap.arenaBits(arena);
bitmap.copyBitsFrom(arena->atomBitmapStart(), ArenaBitmapWords,
chunkWords);
}
@ -121,7 +121,7 @@ static void BitwiseOrIntoChunkMarkBits(JSRuntime* runtime, Bitmap& bitmap) {
for (auto thingKind : AllAllocKinds()) {
for (ArenaIter aiter(atomsZone, thingKind); !aiter.done(); aiter.next()) {
Arena* arena = aiter.get();
uintptr_t* chunkWords = arena->chunk()->bitmap.arenaBits(arena);
MarkBitmapWord* chunkWords = arena->chunk()->bitmap.arenaBits(arena);
bitmap.bitwiseOrRangeInto(arena->atomBitmapStart(), ArenaBitmapWords,
chunkWords);
}

6
js/src/gc/GC.cpp
Просмотреть файл

@ -397,8 +397,10 @@ static constexpr FinalizePhase BackgroundFinalizePhases[] = {
AllocKind::OBJECT_GROUP}}};
void Arena::unmarkAll() {
uintptr_t* word = chunk()->bitmap.arenaBits(this);
memset(word, 0, ArenaBitmapWords * sizeof(uintptr_t));
MarkBitmapWord* arenaBits = chunk()->bitmap.arenaBits(this);
for (size_t i = 0; i < ArenaBitmapWords; i++) {
arenaBits[i] = 0;
}
}
void Arena::unmarkPreMarkedFreeCells() {

53
js/src/gc/Heap.h
Просмотреть файл

@ -7,7 +7,6 @@
#ifndef gc_Heap_h
#define gc_Heap_h
#include "mozilla/Atomics.h"
#include "mozilla/DebugOnly.h"
#include "ds/BitArray.h"
@ -610,39 +609,37 @@ static_assert(ArenasPerChunk == 252,
/* A chunk bitmap contains enough mark bits for all the cells in a chunk. */
struct ChunkBitmap {
volatile uintptr_t bitmap[ArenaBitmapWords * ArenasPerChunk];
public:
ChunkBitmap() = default;
static constexpr size_t WordCount = ArenaBitmapWords * ArenasPerChunk;
MarkBitmapWord bitmap[WordCount];
MOZ_ALWAYS_INLINE void getMarkWordAndMask(const TenuredCell* cell,
ColorBit colorBit,
uintptr_t** wordp,
MarkBitmapWord** wordp,
uintptr_t* maskp) {
MOZ_ASSERT(size_t(colorBit) < MarkBitsPerCell);
detail::GetGCThingMarkWordAndMask(uintptr_t(cell), colorBit, wordp, maskp);
}
MOZ_ALWAYS_INLINE MOZ_TSAN_BLACKLIST bool markBit(const TenuredCell* cell,
ColorBit colorBit) {
uintptr_t *word, mask;
public:
ChunkBitmap() = default;
MOZ_ALWAYS_INLINE bool markBit(const TenuredCell* cell, ColorBit colorBit) {
MarkBitmapWord* word;
uintptr_t mask;
getMarkWordAndMask(cell, colorBit, &word, &mask);
return *word & mask;
}
MOZ_ALWAYS_INLINE MOZ_TSAN_BLACKLIST bool isMarkedAny(
const TenuredCell* cell) {
MOZ_ALWAYS_INLINE bool isMarkedAny(const TenuredCell* cell) {
return markBit(cell, ColorBit::BlackBit) ||
markBit(cell, ColorBit::GrayOrBlackBit);
}
MOZ_ALWAYS_INLINE MOZ_TSAN_BLACKLIST bool isMarkedBlack(
const TenuredCell* cell) {
MOZ_ALWAYS_INLINE bool isMarkedBlack(const TenuredCell* cell) {
return markBit(cell, ColorBit::BlackBit);
}
MOZ_ALWAYS_INLINE MOZ_TSAN_BLACKLIST bool isMarkedGray(
const TenuredCell* cell) {
MOZ_ALWAYS_INLINE bool isMarkedGray(const TenuredCell* cell) {
return !markBit(cell, ColorBit::BlackBit) &&
markBit(cell, ColorBit::GrayOrBlackBit);
}
@ -650,7 +647,8 @@ struct ChunkBitmap {
// The return value indicates if the cell went from unmarked to marked.
MOZ_ALWAYS_INLINE bool markIfUnmarked(const TenuredCell* cell,
MarkColor color) {
uintptr_t *word, mask;
MarkBitmapWord* word;
uintptr_t mask;
getMarkWordAndMask(cell, ColorBit::BlackBit, &word, &mask);
if (*word & mask) {
return false;
@ -672,17 +670,20 @@ struct ChunkBitmap {
}
MOZ_ALWAYS_INLINE void markBlack(const TenuredCell* cell) {
uintptr_t *word, mask;
MarkBitmapWord* word;
uintptr_t mask;
getMarkWordAndMask(cell, ColorBit::BlackBit, &word, &mask);
*word |= mask;
}
MOZ_ALWAYS_INLINE void copyMarkBit(TenuredCell* dst, const TenuredCell* src,
ColorBit colorBit) {
uintptr_t *srcWord, srcMask;
MarkBitmapWord* srcWord;
uintptr_t srcMask;
getMarkWordAndMask(src, colorBit, &srcWord, &srcMask);
uintptr_t *dstWord, dstMask;
MarkBitmapWord* dstWord;
uintptr_t dstMask;
getMarkWordAndMask(dst, colorBit, &dstWord, &dstMask);
*dstWord &= ~dstMask;
@ -692,23 +693,29 @@ struct ChunkBitmap {
}
MOZ_ALWAYS_INLINE void unmark(const TenuredCell* cell) {
uintptr_t *word, mask;
MarkBitmapWord* word;
uintptr_t mask;
getMarkWordAndMask(cell, ColorBit::BlackBit, &word, &mask);
*word &= ~mask;
getMarkWordAndMask(cell, ColorBit::GrayOrBlackBit, &word, &mask);
*word &= ~mask;
}
void clear() { memset((void*)bitmap, 0, sizeof(bitmap)); }
void clear() {
for (size_t i = 0; i < WordCount; i++) {
bitmap[i] = 0;
}
}
uintptr_t* arenaBits(Arena* arena) {
MarkBitmapWord* arenaBits(Arena* arena) {
static_assert(
ArenaBitmapBits == ArenaBitmapWords * JS_BITS_PER_WORD,
"We assume that the part of the bitmap corresponding to the arena "
"has the exact number of words so we do not need to deal with a word "
"that covers bits from two arenas.");
uintptr_t *word, unused;
MarkBitmapWord* word;
uintptr_t unused;
getMarkWordAndMask(reinterpret_cast<TenuredCell*>(arena->address()),
ColorBit::BlackBit, &word, &unused);
return word;

6
js/src/gc/Marking.cpp
Просмотреть файл

@ -4209,11 +4209,11 @@ uintptr_t* GetMarkWordAddress(Cell* cell) {
return nullptr;
}
uintptr_t* wordp;
MarkBitmapWord* wordp;
uintptr_t mask;
js::gc::detail::GetGCThingMarkWordAndMask(uintptr_t(cell), ColorBit::BlackBit,
&wordp, &mask);
return wordp;
return reinterpret_cast<uintptr_t*>(wordp);
}
uintptr_t GetMarkMask(Cell* cell, uint32_t colorBit) {
@ -4224,7 +4224,7 @@ uintptr_t GetMarkMask(Cell* cell, uint32_t colorBit) {
}
ColorBit bit = colorBit == 0 ? ColorBit::BlackBit : ColorBit::GrayOrBlackBit;
uintptr_t* wordp;
MarkBitmapWord* wordp;
uintptr_t mask;
js::gc::detail::GetGCThingMarkWordAndMask(uintptr_t(cell), bit, &wordp,
&mask);

6
js/src/gc/Verifier.cpp
Просмотреть файл

@ -637,7 +637,11 @@ void js::gc::MarkingValidator::nonIncrementalMark(AutoGCSession& session) {
auto ptr = map.lookup(chunk);
MOZ_RELEASE_ASSERT(ptr, "Chunk not found in map");
ChunkBitmap* entry = ptr->value().get();
std::swap(*entry, *bitmap);
for (size_t i = 0; i < ChunkBitmap::WordCount; i++) {
uintptr_t v = entry->bitmap[i];
entry->bitmap[i] = uintptr_t(bitmap->bitmap[i]);
bitmap->bitmap[i] = v;
}
}
}