Bug 1551275 - Refactor jsapi weak map tests r=sfink

This removes a bunch of repeated code and hopefully makes it easier to see what we're testing.  When marking two things the same color this now checks both orders (e.g. key before map, map before key).  I removed individual test cases and generate all possiblities with for loops.  The expected marking state is determined by functions factored out from the verifier.

The tests for JS WeakMap and internal weakmaps are slightly different because I wanted to cover all existing test cases without making things too complicated.  This means we don't test marking the key and delegate different colors for the former.

Differential Revision: https://phabricator.services.mozilla.com/D30948
This commit is contained in:
Jon Coppeard 2019-05-13 19:08:10 +01:00
parent 0a101d1747
commit 6832b3f753
3 changed files with 236 additions and 282 deletions

View File

@ -4,6 +4,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "gc/Verifier.h"
#include "mozilla/DebugOnly.h"
#include "mozilla/IntegerPrintfMacros.h"
#include "mozilla/Sprintf.h"
@ -447,23 +449,6 @@ void js::gc::GCRuntime::finishVerifier() {
#if defined(JS_GC_ZEAL) || defined(DEBUG)
// Like gc::MarkColor but allows the possibility of the cell being
// unmarked. Order is important here, with white being 'least marked'
// and black being 'most marked'.
enum class CellColor : uint8_t { White = 0, Gray = 1, Black = 2 };
static CellColor GetCellColor(Cell* cell) {
if (cell->isMarkedBlack()) {
return CellColor::Black;
}
if (cell->isMarkedGray()) {
return CellColor::Gray;
}
return CellColor::White;
}
static const char* CellColorName(CellColor color) {
switch (color) {
case CellColor::White:
@ -798,7 +783,7 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
valueColor = GetCellColor(value);
}
if (valueColor < Min(mapColor, keyColor)) {
if (valueColor < ExpectedWeakMapValueColor(mapColor, keyColor)) {
fprintf(stderr, "WeakMap value is less marked than map and key\n");
fprintf(stderr, "(map %p is %s, key %p is %s, value %p is %s)\n", map,
CellColorName(mapColor), key, CellColorName(keyColor), value,
@ -827,7 +812,7 @@ bool js::gc::CheckWeakMapEntryMarking(const WeakMapBase* map, Cell* key,
delegateColor = CellColor::Black;
}
if (keyColor < Min(mapColor, delegateColor)) {
if (keyColor < ExpectedWeakMapKeyColor(mapColor, delegateColor)) {
fprintf(stderr, "WeakMap key is less marked than map and delegate\n");
fprintf(stderr, "(map %p is %s, delegate %p is %s, key %p is %s)\n", map,
CellColorName(mapColor), delegate, CellColorName(delegateColor),

62
js/src/gc/Verifier.h Normal file
View File

@ -0,0 +1,62 @@
/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*-
* vim: set ts=8 sts=2 et sw=2 tw=80:
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
/*
* Internal header for definitions shared between the verifier and jsapi tests.
*/
#ifndef gc_Verifier_h
#define gc_Verifier_h
#include "gc/Cell.h"
namespace js {
namespace gc {
// Like gc::MarkColor but allows the possibility of the cell being
// unmarked. Order is important here, with white being 'least marked'
// and black being 'most marked'.
enum class CellColor : uint8_t { White = 0, Gray = 1, Black = 2 };
static constexpr CellColor AllCellColors[] = {
CellColor::White, CellColor::Gray, CellColor::Black
};
static constexpr CellColor MarkedCellColors[] = {
CellColor::Gray, CellColor::Black
};
inline CellColor GetCellColor(Cell* cell) {
if (cell->isMarkedBlack()) {
return CellColor::Black;
}
if (cell->isMarkedGray()) {
return CellColor::Gray;
}
return CellColor::White;
}
inline CellColor ExpectedWeakMapValueColor(CellColor keyColor,
CellColor mapColor) {
return Min(keyColor, mapColor);
}
inline CellColor ExpectedWeakMapKeyColor(CellColor mapColor,
CellColor delegateColor) {
return Min(mapColor, delegateColor);
}
inline CellColor ExpectedKeyAndDelegateColor(CellColor keyColor,
CellColor delegateColor) {
return Max(keyColor, delegateColor);
}
} // namespace gc
} // namespace js
#endif /* gc_Verifier_h */

View File

@ -6,6 +6,7 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
#include "gc/Heap.h"
#include "gc/Verifier.h"
#include "gc/WeakMap.h"
#include "gc/Zone.h"
#include "js/Proxy.h"
@ -56,7 +57,7 @@ BEGIN_TEST(testGCGrayMarking) {
InitGrayRootTracer();
bool ok = TestMarking() && TestWeakMaps() && TestUnassociatedWeakMaps() &&
bool ok = TestMarking() && TestJSWeakMaps() && TestInternalWeakMaps() &&
TestCCWs() && TestGrayUnmarking();
global1 = nullptr;
@ -149,280 +150,183 @@ bool TestMarking() {
return true;
}
bool TestWeakMaps() {
JSObject* weakMap = JS::NewWeakMapObject(cx);
CHECK(weakMap);
static const CellColor DontMark = CellColor::White;
JSObject* key;
JSObject* value;
{
JS::RootedObject rootedMap(cx, weakMap);
enum MarkKeyOrDelegate : bool { MarkKey = true, MarkDelegate = false };
key = AllocWeakmapKeyObject();
CHECK(key);
value = AllocPlainObject();
CHECK(value);
weakMap = rootedMap;
bool TestJSWeakMaps() {
for (auto keyOrDelegateColor : MarkedCellColors) {
for (auto mapColor : MarkedCellColors) {
for (auto markKeyOrDelegate : { MarkKey, MarkDelegate }) {
CellColor expected = ExpectedWeakMapValueColor(keyOrDelegateColor,
mapColor);
CHECK(TestJSWeakMap(markKeyOrDelegate, keyOrDelegateColor, mapColor,
expected));
}
}
}
{
JS::RootedObject rootedMap(cx, weakMap);
JS::RootedObject rootedKey(cx, key);
JS::RootedValue rootedValue(cx, ObjectValue(*value));
CHECK(SetWeakMapEntry(cx, rootedMap, rootedKey, rootedValue));
}
// Test the value of a weakmap entry is marked gray by GC if both the
// weakmap and key are marked gray.
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = key;
JS_GC(cx);
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
// Test the value of a weakmap entry is marked gray by GC if one of the
// weakmap and the key is marked gray and the other black.
JS::RootedObject blackRoot1(cx);
blackRoot1 = weakMap;
grayRoots.grayRoot1 = key;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
blackRoot1 = key;
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedGray(value));
// Test the value of a weakmap entry is marked black by GC if both the
// weakmap and the key are marked black.
JS::RootedObject blackRoot2(cx);
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
blackRoot1 = weakMap;
blackRoot2 = key;
JS_GC(cx);
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
blackRoot1 = key;
blackRoot2 = weakMap;
JS_GC(cx);
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
// Test that a weakmap key is marked gray if it has a gray delegate and the
// map is either gray or black.
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
blackRoot1 = weakMap;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = delegate;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedGray(delegate));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedGray(value));
blackRoot1 = nullptr;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = delegate;
JS_GC(cx);
CHECK(IsMarkedGray(delegate));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedGray(value));
// Test that a weakmap key is marked gray if it has a black delegate but
// the map is gray.
blackRoot1 = delegate;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedGray(value));
blackRoot1 = delegate;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = key;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedGray(value));
// Test that a weakmap key is marked black if it has a black delegate and
// the map is black.
blackRoot1 = delegate;
blackRoot2 = weakMap;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedBlack(value));
blackRoot1 = delegate;
blackRoot2 = weakMap;
grayRoots.grayRoot1 = key;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedBlack(value));
// Test what happens if there is a delegate but it is not marked for both
// black and gray cases.
delegate = nullptr;
blackRoot1 = key;
blackRoot2 = weakMap;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(weakMap));
CHECK(IsMarkedBlack(value));
blackRoot1 = nullptr;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = weakMap;
grayRoots.grayRoot2 = key;
JS_GC(cx);
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(weakMap));
CHECK(IsMarkedGray(value));
blackRoot1 = nullptr;
blackRoot2 = nullptr;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
return true;
}
bool TestUnassociatedWeakMaps() {
// Make a weakmap that's not associated with a JSObject.
bool TestInternalWeakMaps() {
for (auto keyMarkColor : AllCellColors) {
for (auto delegateMarkColor : AllCellColors) {
if (keyMarkColor == CellColor::White &&
delegateMarkColor == CellColor::White) {
continue;
}
CellColor keyOrDelegateColor =
ExpectedKeyAndDelegateColor(keyMarkColor, delegateMarkColor);
CellColor expected = ExpectedWeakMapValueColor(keyOrDelegateColor,
CellColor::Black);
CHECK(TestInternalWeakMap(keyMarkColor, delegateMarkColor, expected));
}
}
return true;
}
bool TestJSWeakMap(MarkKeyOrDelegate markKey, CellColor weakMapMarkColor,
CellColor keyOrDelegateMarkColor,
CellColor expectedValueColor) {
// Test marking a JS WeakMap object.
//
// This marks the map and one of the key or delegate. The key/delegate and the
// value can end up different colors depending on the color of the map.
JSObject* weakMap;
JSObject* key;
JSObject* value;
// If both map and key are marked the same color, test both possible
// orderings.
unsigned markOrderings = weakMapMarkColor == keyOrDelegateMarkColor ? 2 : 1;
for (unsigned markOrder = 0; markOrder < markOrderings; markOrder++) {
CHECK(CreateJSWeakMapObjects(&weakMap, &key, &value));
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
JSObject* keyOrDelegate = markKey ? key : delegate;
RootedObject blackRoot1(cx);
RootedObject blackRoot2(cx);
RootObject(weakMap, weakMapMarkColor, blackRoot1, grayRoots.grayRoot1);
RootObject(keyOrDelegate, keyOrDelegateMarkColor, blackRoot2,
grayRoots.grayRoot2);
if (markOrder != 0) {
mozilla::Swap(blackRoot1.get(), blackRoot2.get());
mozilla::Swap(grayRoots.grayRoot1, grayRoots.grayRoot2);
}
JS_GC(cx);
ClearGrayRoots();
CHECK(GetCellColor(weakMap) == weakMapMarkColor);
CHECK(GetCellColor(keyOrDelegate) == keyOrDelegateMarkColor);
CHECK(GetCellColor(value) == expectedValueColor);
}
return true;
}
bool CreateJSWeakMapObjects(JSObject** weakMapOut, JSObject** keyOut,
JSObject** valueOut) {
RootedObject key(cx, AllocWeakmapKeyObject());
CHECK(key);
RootedObject value(cx, AllocPlainObject());
CHECK(value);
RootedObject weakMap(cx, JS::NewWeakMapObject(cx));
CHECK(weakMap);
JS::RootedValue valueValue(cx, ObjectValue(*value));
CHECK(SetWeakMapEntry(cx, weakMap, key, valueValue));
*weakMapOut = weakMap;
*keyOut = key;
*valueOut = value;
return true;
}
bool TestInternalWeakMap(CellColor keyMarkColor, CellColor delegateMarkColor,
CellColor expectedColor) {
// Test marking for internal weakmaps (without an owning JSObject).
//
// All of the key, delegate and value are expected to end up the same color.
UniquePtr<GCManagedObjectWeakMap> weakMap;
JSObject* key;
JSObject* value;
// If both key and delegate are marked the same color, test both possible
// orderings.
unsigned markOrderings = keyMarkColor == delegateMarkColor ? 2 : 1;
for (unsigned markOrder = 0; markOrder < markOrderings; markOrder++) {
CHECK(CreateInternalWeakMapObjects(&weakMap, &key, &value));
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
RootedObject blackRoot1(cx);
RootedObject blackRoot2(cx);
Rooted<GCManagedObjectWeakMap*> rootMap(cx, weakMap.get());
RootObject(key, keyMarkColor, blackRoot1, grayRoots.grayRoot1);
RootObject(delegate, delegateMarkColor, blackRoot2, grayRoots.grayRoot2);
if (markOrder != 0) {
mozilla::Swap(blackRoot1.get(), blackRoot2.get());
mozilla::Swap(grayRoots.grayRoot1, grayRoots.grayRoot2);
}
JS_GC(cx);
ClearGrayRoots();
CHECK(GetCellColor(key) == expectedColor);
CHECK(GetCellColor(delegate) == expectedColor);
CHECK(GetCellColor(value) == expectedColor);
}
return true;
}
bool CreateInternalWeakMapObjects(UniquePtr<GCManagedObjectWeakMap>* weakMapOut,
JSObject** keyOut, JSObject** valueOut) {
RootedObject key(cx, AllocWeakmapKeyObject());
CHECK(key);
RootedObject value(cx, AllocPlainObject());
CHECK(value);
auto weakMap = cx->make_unique<GCManagedObjectWeakMap>(cx);
CHECK(weakMap);
// Make sure this gets traced during GC.
Rooted<GCManagedObjectWeakMap*> rootMap(cx, weakMap.get());
JSObject* key = AllocWeakmapKeyObject();
CHECK(key);
JSObject* value = AllocPlainObject();
CHECK(value);
CHECK(weakMap->add(cx, key, value));
// Test the value of a weakmap entry is marked gray by GC if the
// key is marked gray.
grayRoots.grayRoot1 = key;
JS_GC(cx);
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
// Test the value of a weakmap entry is marked gray by GC if the key is marked
// gray.
grayRoots.grayRoot1 = key;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
// Test the value of a weakmap entry is marked black by GC if the key is
// marked black.
JS::RootedObject blackRoot(cx);
blackRoot = key;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
// Test that a weakmap key is marked gray if it has a gray delegate.
JSObject* delegate = UncheckedUnwrapWithoutExpose(key);
blackRoot = nullptr;
grayRoots.grayRoot1 = delegate;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedGray(delegate));
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
// Test that a weakmap key is marked black if it has a black delegate.
blackRoot = delegate;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
blackRoot = delegate;
grayRoots.grayRoot1 = key;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(delegate));
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
// Test what happens if there is a delegate but it is not marked for both
// black and gray cases.
delegate = nullptr;
blackRoot = key;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedBlack(key));
CHECK(IsMarkedBlack(value));
blackRoot = nullptr;
grayRoots.grayRoot1 = key;
grayRoots.grayRoot2 = nullptr;
JS_GC(cx);
CHECK(IsMarkedGray(key));
CHECK(IsMarkedGray(value));
blackRoot = nullptr;
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
*weakMapOut = std::move(weakMap);
*keyOut = key;
*valueOut = value;
return true;
}
void RootObject(JSObject* object, CellColor color, RootedObject& blackRoot,
JS::Heap<JSObject*>& grayRoot) {
if (color == CellColor::Black) {
blackRoot = object;
} else if (color == CellColor::Gray) {
grayRoot = object;
} else {
MOZ_RELEASE_ASSERT(color == CellColor::White);
}
}
bool TestCCWs() {
JSObject* target = AllocPlainObject();
CHECK(target);
@ -594,15 +498,18 @@ bool InitGlobals() {
return global2 != nullptr;
}
void InitGrayRootTracer() {
void ClearGrayRoots() {
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
}
void InitGrayRootTracer() {
ClearGrayRoots();
JS_SetGrayGCRootsTracer(cx, TraceGrayRoots, &grayRoots);
}
void RemoveGrayRootTracer() {
grayRoots.grayRoot1 = nullptr;
grayRoots.grayRoot2 = nullptr;
ClearGrayRoots();
JS_SetGrayGCRootsTracer(cx, nullptr, nullptr);
}