Skip to content

Jdk 8340297 metaspace api for checking if address is in use #25891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/hotspot/share/gc/g1/g1HeapRegion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ static bool is_oop_safe(oop obj) {
return false;
}

if (!Metaspace::contains(klass)) {
if (!Metaspace::klass_is_live(klass, true)) {
log_error(gc, verify)("klass " PTR_FORMAT " of object " PTR_FORMAT " "
"is not in metaspace", p2i(klass), p2i(obj));
return false;
Expand Down
7 changes: 5 additions & 2 deletions src/hotspot/share/gc/shared/collectedHeap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ bool CollectedHeap::supports_concurrent_gc_breakpoints() const {
}

static bool klass_is_sane(oop object) {
const Klass* k = nullptr;
if (UseCompactObjectHeaders) {
// With compact headers, we can't safely access the Klass* when
// the object has been forwarded, because non-full-GC-forwarding
Expand All @@ -248,10 +249,12 @@ static bool klass_is_sane(oop object) {
return true;
}

return Metaspace::contains(mark.klass_without_asserts());
k =mark.klass_without_asserts();
} else {
k = object->klass_without_asserts();
}

return Metaspace::contains(object->klass_without_asserts());
return Metaspace::klass_is_live(k, true);
}

bool CollectedHeap::is_oop(oop object) const {
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/gc/shenandoah/shenandoahAsserts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*
file,line);
}

if (!Metaspace::contains(obj_klass)) {
if (!Metaspace::klass_is_live(obj_klass, true)) {
print_failure(_safe_unknown, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Object klass pointer must go to metaspace",
file,line);
Expand Down Expand Up @@ -270,14 +270,14 @@ void ShenandoahAsserts::assert_correct(void* interior_loc, oop obj, const char*

if (Universe::is_fully_initialized() && (obj_klass == vmClasses::Class_klass())) {
Metadata* klass = obj->metadata_field(java_lang_Class::klass_offset());
if (klass != nullptr && !Metaspace::contains(klass)) {
if (klass != nullptr && !Metaspace::metadata_is_live(klass)) {
print_failure(_safe_all, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Instance class mirror should point to Metaspace",
file, line);
}

Metadata* array_klass = obj->metadata_field(java_lang_Class::array_klass_offset());
if (array_klass != nullptr && !Metaspace::contains(array_klass)) {
if (array_klass != nullptr && !Metaspace::metadata_is_live(array_klass)) {
print_failure(_safe_all, obj, interior_loc, nullptr, "Shenandoah assert_correct failed",
"Array class mirror should point to Metaspace",
file, line);
Expand Down
8 changes: 4 additions & 4 deletions src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
// Do this before touching obj->size()
check(ShenandoahAsserts::_safe_unknown, obj, obj_klass != nullptr,
"Object klass pointer should not be null");
check(ShenandoahAsserts::_safe_unknown, obj, Metaspace::contains(obj_klass),
check(ShenandoahAsserts::_safe_unknown, obj, Metaspace::klass_is_live(obj_klass, true),
"Object klass pointer must go to metaspace");

HeapWord *obj_addr = cast_from_oop<HeapWord*>(obj);
Expand Down Expand Up @@ -216,7 +216,7 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
Klass* fwd_klass = fwd->klass_or_null();
check(ShenandoahAsserts::_safe_oop, obj, fwd_klass != nullptr,
"Forwardee klass pointer should not be null");
check(ShenandoahAsserts::_safe_oop, obj, Metaspace::contains(fwd_klass),
check(ShenandoahAsserts::_safe_oop, obj, Metaspace::klass_is_live(fwd_klass, true),
"Forwardee klass pointer must go to metaspace");
check(ShenandoahAsserts::_safe_oop, obj, obj_klass == fwd_klass,
"Forwardee klass pointer must go to metaspace");
Expand Down Expand Up @@ -249,12 +249,12 @@ class ShenandoahVerifyOopClosure : public BasicOopIterateClosure {
if (obj_klass == vmClasses::Class_klass()) {
Metadata* klass = obj->metadata_field(java_lang_Class::klass_offset());
check(ShenandoahAsserts::_safe_oop, obj,
klass == nullptr || Metaspace::contains(klass),
klass == nullptr || Metaspace::metadata_is_live(klass),
"Instance class mirror should point to Metaspace");

Metadata* array_klass = obj->metadata_field(java_lang_Class::array_klass_offset());
check(ShenandoahAsserts::_safe_oop, obj,
array_klass == nullptr || Metaspace::contains(array_klass),
array_klass == nullptr || Metaspace::metadata_is_live(array_klass),
"Array class mirror should point to Metaspace");
}

Expand Down
11 changes: 11 additions & 0 deletions src/hotspot/share/memory/metaspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1040,3 +1040,14 @@ bool Metaspace::is_in_shared_metaspace(const void* ptr) {
bool Metaspace::is_in_nonclass_metaspace(const void* ptr) {
return VirtualSpaceList::vslist_nonclass()->contains((MetaWord*)ptr);
}

#ifdef ASSERT
bool Metaspace::metadata_is_live(const Metadata* md) {
return md->metadata_token_is_valid() && contains(md); // flesh out later
}

bool Metaspace::klass_is_live(const Klass* k, bool must_have_narrow_klass_id) {
return ((const Metadata*)k)->metadata_token_is_valid_klass() && contains(k); // flesh out later
}

#endif // ASSERT
12 changes: 12 additions & 0 deletions src/hotspot/share/memory/metaspace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "utilities/globalDefinitions.hpp"

class ClassLoaderData;
class Metadata;
class MetaspaceShared;
class MetaspaceTracer;
class Mutex;
Expand Down Expand Up @@ -154,6 +155,17 @@ class Metaspace : public AllStatic {
return ptr < _class_space_end && ptr >= _class_space_start;
}

#ifdef ASSERT
// xxx_is_live functions returns true if the pointer given points to
// valid metadata. In detail, it checks that:
// - the specified metadata is inside class-space or metaspace in committed, readable memory
// - not marked as dead space (i.e. not returned prematurely via Metaspace::deallocate)
// - correctly aligned for the type (esp. Klass)
// - Metadata token is valid specific to this type
static bool metadata_is_live(const Metadata* md);
static bool klass_is_live(const Klass* k, bool must_have_narrow_klass_id);
#endif

// Free empty virtualspaces
static void purge(bool classes_unloaded);

Expand Down
24 changes: 15 additions & 9 deletions src/hotspot/share/memory/metaspace/binList.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "memory/metaspace/counters.hpp"
#include "memory/metaspace/metablock.hpp"
#include "memory/metaspace/metaspaceCommon.hpp"
#include "memory/metaspace/metaspaceZapper.hpp"
#include "utilities/align.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"
Expand Down Expand Up @@ -76,6 +77,8 @@ class BinListImpl {
struct Block {
Block* const _next;
Block(Block* next) : _next(next) {}
MetaWord* base() { return (MetaWord*) this; }
const MetaWord* base() const { return (MetaWord*) this; }
};

#define BLOCK_FORMAT "Block @" PTR_FORMAT ": size: %zu, next: " PTR_FORMAT
Expand All @@ -93,6 +96,9 @@ class BinListImpl {
// Maximal (incl) word size a block can have to be manageable by this structure.
const static size_t MaxWordSize = num_lists;

// for tests
static constexpr size_t header_wordsize = sizeof(Block) / BytesPerWord;

private:

Block* _blocks[num_lists];
Expand Down Expand Up @@ -123,15 +129,15 @@ class BinListImpl {
}

#ifdef ASSERT
static const uintptr_t canary = 0xFFEEFFEE;
static void write_canary(MetaWord* p, size_t word_size) {
static void zap_block(Block* b, size_t word_size) {
if (word_size > 1) { // 1-word-sized blocks have no space for a canary
((uintptr_t*)p)[word_size - 1] = canary;
Zapper::zap_payload(b, word_size);
}
}
static bool check_canary(const Block* b, size_t word_size) {
return word_size == 1 || // 1-word-sized blocks have no space for a canary
((const uintptr_t*)b)[word_size - 1] == canary;
static bool check_block_zap(const Block* b, size_t word_size) {
return word_size <= header_wordsize || // 1-word-sized blocks have no space for a canary
( Zapper::is_zapped_location(b->base() + header_wordsize) &&
Zapper::is_zapped_location(b->base() + word_size - header_wordsize) );
}
#endif

Expand All @@ -149,12 +155,12 @@ class BinListImpl {
MetaWord* const p = mb.base();
assert(word_size >= MinWordSize &&
word_size <= MaxWordSize, "bad block size");
DEBUG_ONLY(write_canary(p, word_size);)
const int index = index_for_word_size(word_size);
Block* old_head = _blocks[index];
Block* new_head = new (p) Block(old_head);
_blocks[index] = new_head;
_counter.add(word_size);
DEBUG_ONLY(zap_block(new_head, word_size);)
}

// Given a word_size, searches and returns a block of at least that size.
Expand All @@ -169,7 +175,7 @@ class BinListImpl {
Block* b = _blocks[index];
const size_t real_word_size = word_size_for_index(index);
assert(b != nullptr, "Sanity");
assert(check_canary(b, real_word_size),
assert(check_block_zap(b, real_word_size),
"bad block in list[%d] (" BLOCK_FORMAT ")", index, BLOCK_FORMAT_ARGS(b, real_word_size));
_blocks[index] = b->_next;
_counter.sub(real_word_size);
Expand All @@ -194,7 +200,7 @@ class BinListImpl {
int pos = 0;
Block* b_last = nullptr; // catch simple circularities
for (Block* b = _blocks[i]; b != nullptr; b = b->_next, pos++) {
assert(check_canary(b, s), "");
assert(check_block_zap(b, s), "");
assert(b != b_last, "Circle");
local_counter.add(s);
b_last = b;
Expand Down
17 changes: 10 additions & 7 deletions src/hotspot/share/memory/metaspace/blockTree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const size_t BlockTree::MinWordSize;

#define NODE_FORMAT \
"@" PTR_FORMAT \
": canary " INTPTR_FORMAT \
": canary " UINTX_FORMAT_X_0 \
", parent " PTR_FORMAT \
", left " PTR_FORMAT \
", right " PTR_FORMAT \
Expand All @@ -47,7 +47,7 @@ const size_t BlockTree::MinWordSize;

#define NODE_FORMAT_ARGS(n) \
p2i(n), \
(n)->_canary, \
(uintptr_t)((n)->_canary), \
p2i((n)->_parent), \
p2i((n)->_left), \
p2i((n)->_right), \
Expand Down Expand Up @@ -90,7 +90,7 @@ void BlockTree::verify_node_pointer(const Node* n) const {
// 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) {
if (!Zapper::is_zapped_location(&(n->_canary))) {
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));
}
Expand Down Expand Up @@ -136,6 +136,13 @@ void BlockTree::verify() const {
tree_assert_invalid_node(n->_word_size > info.lim1, n);
tree_assert_invalid_node(n->_word_size < info.lim2, n);

// Check if part of the payload was overwritten (the canary was checked
// before in verify_node_pointer)
if (!Zapper::is_zapped_location(n->end() - 1)) {
os::print_hex_dump(tty, (address)n, (address)n + sizeof(Node), 1);
tree_assert(false, "Invalid node: @" PTR_FORMAT " node end overwritten?", p2i(n));
}

// Check children
if (n->_left != nullptr) {
tree_assert_invalid_node(n->_left != n, n);
Expand Down Expand Up @@ -179,10 +186,6 @@ void BlockTree::verify() const {
_counter.check(counter);
}

void BlockTree::zap_block(MetaBlock bl) {
memset(bl.base(), 0xF3, bl.word_size() * sizeof(MetaWord));
}

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

// Note: we do not print the tree indented, since I found that printing it
Expand Down
49 changes: 30 additions & 19 deletions src/hotspot/share/memory/metaspace/blockTree.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "memory/metaspace/chunklevel.hpp"
#include "memory/metaspace/counters.hpp"
#include "memory/metaspace/metablock.hpp"
#include "memory/metaspace/metaspaceZapper.hpp"
#include "utilities/debug.hpp"
#include "utilities/globalDefinitions.hpp"

Expand Down Expand Up @@ -77,14 +78,9 @@ 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).
// It is initialized in debug and release, but only automatically tested
// in debug.
const intptr_t _canary;
// A leading canary section that is zapped with the same metaspace zap pattern
// as the rest of the payload following the header (and all of dead/unsused metaspace).
MetaWord _canary;

// Normal tree node stuff...
// (Note: all null if this is a stacked node)
Expand All @@ -100,8 +96,12 @@ class BlockTree: public CHeapObj<mtMetaspace> {
// tree and need additional space for weighting information).
const size_t _word_size;

const MetaWord* base() const { return (MetaWord*) this; }
MetaWord* base() { return (MetaWord*) this; }
const MetaWord* end() const { return base() + _word_size; }

Node(size_t word_size) :
_canary(_canary_value),
_canary(0),
_parent(nullptr),
_left(nullptr),
_right(nullptr),
Expand All @@ -110,14 +110,25 @@ class BlockTree: public CHeapObj<mtMetaspace> {
{}

#ifdef ASSERT
void zap() {
// Zap first word of header and payload that follows
Zapper::zap_location(&_canary);
Zapper::zap_payload(this, _word_size);
}
bool check_zap() const {
return Zapper::is_zapped_location(&_canary) &&
Zapper::is_zapped_location(end() - 1);
}
bool valid() const {
return _canary == _canary_value &&
_word_size >= sizeof(Node) &&
_word_size < chunklevel::MAX_CHUNK_WORD_SIZE;
return _word_size > sizeof(Node) && // at least one word larger
_word_size < chunklevel::MAX_CHUNK_WORD_SIZE // but not larger than a root chunk
DEBUG_ONLY(&& check_zap());
}
#endif
};

STATIC_ASSERT(is_aligned(sizeof(Node), sizeof(MetaWord)));

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

Expand All @@ -128,9 +139,12 @@ class BlockTree: public CHeapObj<mtMetaspace> {

public:

// Minimum word size a block has to be to be added to this structure (note ceil division).
const static size_t MinWordSize =
(sizeof(Node) + sizeof(MetaWord) - 1) / sizeof(MetaWord);
// Public only for tests
static constexpr size_t header_wordsize = sizeof(Node) / sizeof(MetaWord);

// Minimum word size a block has to be to be added to this structure
// (Node size + at least one word; smaller blocks -> BinList)
constexpr static size_t MinWordSize = header_wordsize + 1;

private:

Expand Down Expand Up @@ -335,7 +349,6 @@ class BlockTree: public CHeapObj<mtMetaspace> {
}

#ifdef ASSERT
void zap_block(MetaBlock block);
// Helper for verify()
void verify_node_pointer(const Node* n) const;
#endif // ASSERT
Expand All @@ -346,7 +359,6 @@ class BlockTree: public CHeapObj<mtMetaspace> {

// Add a memory block to the tree. Its content will be overwritten.
void add_block(MetaBlock block) {
DEBUG_ONLY(zap_block(block);)
const size_t word_size = block.word_size();
assert(word_size >= MinWordSize, "invalid block size %zu", word_size);
Node* n = new(block.base()) Node(word_size);
Expand All @@ -355,6 +367,7 @@ class BlockTree: public CHeapObj<mtMetaspace> {
} else {
insert(_root, n);
}
DEBUG_ONLY(n->zap();)
_counter.add(word_size);
}

Expand Down Expand Up @@ -383,8 +396,6 @@ class BlockTree: public CHeapObj<mtMetaspace> {
result = MetaBlock((MetaWord*)n, n->_word_size);

_counter.sub(n->_word_size);

DEBUG_ONLY(zap_block(result);)
}
return result;
}
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/memory/metaspace/chunkManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,7 @@ Metachunk* ChunkManager::get_chunk_locked(chunklevel_t preferred_level, chunklev
// !! Note: this may invalidate the chunk. Do not access the chunk after
// this function returns !!
void ChunkManager::return_chunk(Metachunk* c) {
DEBUG_ONLY(c->zap();)
// It is valid to poison the chunk payload area at this point since its physically separated from
// the chunk meta info.
ASAN_POISON_MEMORY_REGION(c->base(), c->word_size() * BytesPerWord);
Expand Down
Loading