Skip to content

Commit

Permalink
8256725: Metaspace: better blocktree and binlist asserts
Browse files Browse the repository at this point in the history 
Reviewed-by: shade, rrich, lkorinth
  • Loading branch information
@tstuefe
tstuefe committed Nov 23, 2020
1 parent 9de5d09 commit fa75ad6
Show file tree
Hide file tree
Showing 5 changed files with 160 additions and 44 deletions.
27 changes: 15 additions & 12 deletions src/hotspot/share/memory/metaspace/binList.hpp
View file
Expand Up @@ -85,6 +85,9 @@ class BinListImpl {
{}
};

#define BLOCK_FORMAT "Block @" PTR_FORMAT ": size: " SIZE_FORMAT ", next: " PTR_FORMAT
#define BLOCK_FORMAT_ARGS(b) p2i(b), (b)->_word_size, p2i((b)->_next)

// Smallest block size must be large enough to hold a Block structure.
STATIC_ASSERT(smallest_word_size * sizeof(MetaWord) >= sizeof(Block));
STATIC_ASSERT(num_lists > 0);
Expand Down Expand Up @@ -150,19 +153,16 @@ class BinListImpl {
int index = index_for_word_size(word_size);
index = index_for_next_non_empty_list(index);
if (index != -1) {
assert(_blocks[index] != NULL &&
_blocks[index]->_word_size >= word_size, "sanity");

MetaWord* const p = (MetaWord*)_blocks[index];
Block* b = _blocks[index];
const size_t real_word_size = word_size_for_index(index);

_blocks[index] = _blocks[index]->_next;

assert(b != NULL, "Sanity");
assert(b->_word_size >= word_size &&
b->_word_size == real_word_size,
"bad block size in list[%d] (" BLOCK_FORMAT ")", index, BLOCK_FORMAT_ARGS(b));
_blocks[index] = b->_next;
_counter.sub(real_word_size);
*p_real_word_size = real_word_size;

return p;

return (MetaWord*)b;
} else {
*p_real_word_size = 0;
return NULL;
Expand All @@ -182,8 +182,11 @@ class BinListImpl {
MemRangeCounter local_counter;
for (int i = 0; i < num_lists; i++) {
const size_t s = MinWordSize + i;
for (Block* b = _blocks[i]; b != NULL; b = b->_next) {
assert(b->_word_size == s, "bad block size");
int pos = 0;
for (Block* b = _blocks[i]; b != NULL; b = b->_next, pos++) {
assert(b->_word_size == s,
"bad block size in list[%d] at pos %d (" BLOCK_FORMAT ")",
i, pos, BLOCK_FORMAT_ARGS(b));
local_counter.add(s);
}
}
Expand Down
114 changes: 84 additions & 30 deletions src/hotspot/share/memory/metaspace/blockTree.cpp
View file
Expand Up @@ -24,6 +24,7 @@
*/

#include "precompiled.hpp"
#include "memory/metaspace/chunklevel.hpp"
#include "memory/metaspace/blockTree.hpp"
#include "memory/resourceArea.hpp"
#include "utilities/debug.hpp"
Expand All @@ -36,27 +37,43 @@ namespace metaspace {
// Needed to prevent linker errors on MacOS and AIX
const size_t BlockTree::MinWordSize;

#define NODE_FORMAT \
"@" PTR_FORMAT \
": canary " INTPTR_FORMAT \
", parent " PTR_FORMAT \
", left " PTR_FORMAT \
", right " PTR_FORMAT \
", next " PTR_FORMAT \
", size " SIZE_FORMAT

#define NODE_FORMAT_ARGS(n) \
p2i(n), \
(n)->_canary, \
p2i((n)->_parent), \
p2i((n)->_left), \
p2i((n)->_right), \
p2i((n)->_next), \
(n)->_word_size

#ifdef ASSERT

// Tree verification

// These asserts prints the tree, then asserts
#define assrt(cond, format, ...) \
// This assert prints the tree too
#define tree_assert(cond, format, ...) \
do { \
if (!(cond)) { \
tty->print("Error in tree @" PTR_FORMAT ": ", p2i(this)); \
tty->print_cr(format, __VA_ARGS__); \
tty->print_cr("Tree:"); \
print_tree(tty); \
assert(cond, format, __VA_ARGS__); \
} \
} while (0)

// This assert prints the tree, then stops (generic message)
#define assrt0(cond) \
do { \
if (!(cond)) { \
print_tree(tty); \
assert(cond, "sanity"); \
} \
} while (0)
// Assert, prints tree and specific given node
#define tree_assert_invalid_node(cond, failure_node) \
tree_assert(cond, "Invalid node: " NODE_FORMAT, NODE_FORMAT_ARGS(failure_node))

// walkinfo keeps a node plus the size corridor it and its children
// are supposed to be in.
Expand All @@ -67,12 +84,24 @@ struct BlockTree::walkinfo {
size_t lim2; // )
};

// Helper for verify()
void BlockTree::verify_node_pointer(const Node* n) const {
tree_assert(os::is_readable_pointer(n),
"Invalid node: @" PTR_FORMAT " is unreadable.", p2i(n));
// If the canary is broken, this is either an invalid node pointer or
// the node has been overwritten. Either way, print a hex dump, then
// assert away.
if (n->_canary != Node::_canary_value) {
os::print_hex_dump(tty, (address)n, (address)n + sizeof(Node), 1);
tree_assert(false, "Invalid node: @" PTR_FORMAT " canary broken or pointer invalid", p2i(n));
}
}

void BlockTree::verify() const {
// Traverse the tree and test that all nodes are in the correct order.

MemRangeCounter counter;
int longest_edge = 0;

if (_root != NULL) {

ResourceMark rm;
Expand All @@ -90,27 +119,29 @@ void BlockTree::verify() const {
info = stack.pop();
const Node* n = info.n;

verify_node_pointer(n);

// Assume a (ridiculously large) edge limit to catch cases
// of badly degenerated or circular trees.
assrt0(info.depth < 10000);
tree_assert(info.depth < 10000, "too deep (%d)", info.depth);
counter.add(n->_word_size);

// Verify node.
if (n == _root) {
assrt0(n->_parent == NULL);
tree_assert_invalid_node(n->_parent == NULL, n);
} else {
assrt0(n->_parent != NULL);
tree_assert_invalid_node(n->_parent != NULL, n);
}

// check size and ordering
assrt(n->_word_size >= MinWordSize, "bad node size " SIZE_FORMAT, n->_word_size);
assrt0(n->_word_size > info.lim1);
assrt0(n->_word_size < info.lim2);
tree_assert_invalid_node(n->_word_size >= MinWordSize, n);
tree_assert_invalid_node(n->_word_size <= chunklevel::MAX_CHUNK_WORD_SIZE, n);
tree_assert_invalid_node(n->_word_size > info.lim1, n);
tree_assert_invalid_node(n->_word_size < info.lim2, n);

// Check children
if (n->_left != NULL) {
assrt0(n->_left != n);
assrt0(n->_left->_parent == n);
tree_assert_invalid_node(n->_left != n, n);
tree_assert_invalid_node(n->_left->_parent == n, n);

walkinfo info2;
info2.n = n->_left;
Expand All @@ -121,8 +152,8 @@ void BlockTree::verify() const {
}

if (n->_right != NULL) {
assrt0(n->_right != n);
assrt0(n->_right->_parent == n);
tree_assert_invalid_node(n->_right != n, n);
tree_assert_invalid_node(n->_right->_parent == n, n);

walkinfo info2;
info2.n = n->_right;
Expand All @@ -135,26 +166,33 @@ void BlockTree::verify() const {
// If node has same-sized siblings check those too.
const Node* n2 = n->_next;
while (n2 != NULL) {
assrt0(n2 != n);
assrt0(n2->_word_size == n->_word_size);
verify_node_pointer(n2);
tree_assert_invalid_node(n2 != n, n2); // catch simple circles
tree_assert_invalid_node(n2->_word_size == n->_word_size, n2);
counter.add(n2->_word_size);
n2 = n2->_next;
}
}
}

// At the end, check that counters match
// (which also verifies that we visited every node, or at least
// as many nodes as are in this tree)
_counter.check(counter);

#undef assrt0n
}

void BlockTree::zap_range(MetaWord* p, size_t word_size) {
memset(p, 0xF3, word_size * sizeof(MetaWord));
}

#undef assrt
#undef assrt0

void BlockTree::print_tree(outputStream* st) const {

// Note: we do not print the tree indented, since I found that printing it
// as a quasi list is much clearer to the eye.
// We print the tree depth-first, with stacked nodes below normal ones
// (normal "real" nodes are marked with a leading '+')
if (_root != NULL) {

ResourceMark rm;
Expand All @@ -168,11 +206,27 @@ void BlockTree::print_tree(outputStream* st) const {
while (stack.length() > 0) {
info = stack.pop();
const Node* n = info.n;

// Print node.
for (int i = 0; i < info.depth; i++) {
st->print("---");
st->print("%4d + ", info.depth);
if (os::is_readable_pointer(n)) {
st->print_cr(NODE_FORMAT, NODE_FORMAT_ARGS(n));
} else {
st->print_cr("@" PTR_FORMAT ": unreadable (skipping subtree)", p2i(n));
continue; // don't print this subtree
}

// Print same-sized-nodes stacked under this node
for (Node* n2 = n->_next; n2 != NULL; n2 = n2->_next) {
st->print_raw(" ");
if (os::is_readable_pointer(n2)) {
st->print_cr(NODE_FORMAT, NODE_FORMAT_ARGS(n2));
} else {
st->print_cr("@" PTR_FORMAT ": unreadable (skipping rest of chain).", p2i(n2));
break; // stop printing this chain.
}
}
st->print_cr("<" PTR_FORMAT " (size " SIZE_FORMAT ")", p2i(n), n->_word_size);

// Handle children.
if (n->_right != NULL) {
walkinfo info2;
Expand Down
33 changes: 31 additions & 2 deletions src/hotspot/share/memory/metaspace/blockTree.hpp
View file
Expand Up @@ -77,7 +77,18 @@ class BlockTree: public CHeapObj<mtMetaspace> {

struct Node {

static const intptr_t _canary_value =
NOT_LP64(0x4e4f4445) LP64_ONLY(0x4e4f44454e4f4445ULL); // "NODE" resp "NODENODE"

// Note: we afford us the luxury of an always-there canary value.
// The space for that is there (these nodes are only used to manage larger blocks,
// see FreeBlocks::MaxSmallBlocksWordSize).
// It is initialized in debug and release, but only automatically tested
// in debug.
const intptr_t _canary;

// Normal tree node stuff...
// (Note: all null if this is a stacked node)
Node* _parent;
Node* _left;
Node* _right;
Expand All @@ -91,18 +102,31 @@ class BlockTree: public CHeapObj<mtMetaspace> {
const size_t _word_size;

Node(size_t word_size) :
_canary(_canary_value),
_parent(NULL),
_left(NULL),
_right(NULL),
_next(NULL),
_word_size(word_size)
{}

#ifdef ASSERT
bool valid() const {
return _canary == _canary_value &&
_word_size >= sizeof(Node) &&
_word_size < chunklevel::MAX_CHUNK_WORD_SIZE;
}
#endif
};

// Needed for verify() and print_tree()
struct walkinfo;

#ifdef ASSERT
// Run a quick check on a node; upon suspicion dive into a full tree check.
void check_node(const Node* n) const { if (!n->valid()) verify(); }
#endif

public:

// Minimum word size a block has to be to be added to this structure (note ceil division).
Expand Down Expand Up @@ -196,9 +220,10 @@ class BlockTree: public CHeapObj<mtMetaspace> {
}

// Given a node n and an insertion point, insert n under insertion point.
static void insert(Node* insertion_point, Node* n) {
void insert(Node* insertion_point, Node* n) {
assert(n->_parent == NULL, "Sanity");
for (;;) {
DEBUG_ONLY(check_node(insertion_point);)
if (n->_word_size == insertion_point->_word_size) {
add_to_list(n, insertion_point); // parent stays NULL in this case.
break;
Expand All @@ -222,9 +247,10 @@ class BlockTree: public CHeapObj<mtMetaspace> {

// Given a node and a wish size, search this node and all children for
// the node closest (equal or larger sized) to the size s.
static Node* find_closest_fit(Node* n, size_t s) {
Node* find_closest_fit(Node* n, size_t s) {
Node* best_match = NULL;
while (n != NULL) {
DEBUG_ONLY(check_node(n);)
if (n->_word_size >= s) {
best_match = n;
if (n->_word_size == s) {
Expand Down Expand Up @@ -311,6 +337,8 @@ class BlockTree: public CHeapObj<mtMetaspace> {

#ifdef ASSERT
void zap_range(MetaWord* p, size_t word_size);
// Helper for verify()
void verify_node_pointer(const Node* n) const;
#endif // ASSERT

public:
Expand Down Expand Up @@ -339,6 +367,7 @@ class BlockTree: public CHeapObj<mtMetaspace> {
Node* n = find_closest_fit(word_size);

if (n != NULL) {
DEBUG_ONLY(check_node(n);)
assert(n->_word_size >= word_size, "sanity");

if (n->_next != NULL) {
Expand Down
4 changes: 4 additions & 0 deletions src/hotspot/share/memory/metaspace/freeBlocks.hpp
View file
Expand Up @@ -69,6 +69,10 @@ class FreeBlocks : public CHeapObj<mtMetaspace> {
// to fit into _smallblocks.
BlockTree _tree;

// This verifies that blocks too large to go into the binlist can be
// kept in the blocktree.
STATIC_ASSERT(BinList32::MaxWordSize >= BlockTree::MinWordSize);

// Cutoff point: blocks larger than this size are kept in the
// tree, blocks smaller than or equal to this size in the bin list.
const size_t MaxSmallBlocksWordSize = BinList32::MaxWordSize;
Expand Down
26 changes: 26 additions & 0 deletions test/hotspot/gtest/metaspace/test_blocktree.cpp
View file
Expand Up @@ -215,7 +215,33 @@ TEST_VM(metaspace, BlockTree_print_test) {
bt.print_tree(&ss);

LOG("%s", ss.as_string());
}

// Test that an overwritten node would result in an assert and a printed tree
TEST_VM_ASSERT_MSG(metaspace, BlockTree_overwriter_test, "Invalid node") {
static const size_t sizes1[] = { 30, 17, 0 };
static const size_t sizes2[] = { 12, 12, 0 };

BlockTree bt;
FeederBuffer fb(4 * K);

// some nodes...
create_nodes(sizes1, fb, bt);

// a node we will break...
MetaWord* p_broken = fb.get(12);
bt.add_block(p_broken, 12);

// some more nodes...
create_nodes(sizes2, fb, bt);

// overwrite node memory (only the very first byte), then verify tree.
// Verification should catch the broken canary, print the tree,
// then assert.
LOG("Will break node at " PTR_FORMAT ".", p2i(p_broken));
tty->print_cr("Death test, please ignore the following \"Invalid node\" printout.");
*((char*)p_broken) = '\0';
bt.verify();
}
#endif

Expand Down

1 comment on commit fa75ad6

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.