Skip to content

Commit

Permalink
8224509: Incorrect alignment in CDS related allocation code on 32-bit…
Browse files Browse the repository at this point in the history
… platforms

Reviewed-by: iklam, stuefe
  • Loading branch information
calvinccheung committed Oct 5, 2020
1 parent 4d29116 commit ea27a54
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/hotspot/share/classfile/systemDictionaryShared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2053,6 +2053,7 @@ InstanceKlass* SystemDictionaryShared::find_builtin_class(Symbol* name) {
const RunTimeSharedClassInfo* record = find_record(&_builtin_dictionary, &_dynamic_builtin_dictionary, name);
if (record != NULL) {
assert(!record->_klass->is_hidden(), "hidden class cannot be looked up by name");
assert(check_alignment(record->_klass), "Address not aligned");
return record->_klass;
} else {
return NULL;
Expand Down
5 changes: 2 additions & 3 deletions src/hotspot/share/memory/archiveBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,6 @@ void ArchiveBuilder::make_shallow_copy(DumpRegion *dump_region, SourceObjInfo* s
address src = ref->obj();
int bytes = src_info->size_in_bytes();
char* dest;
size_t alignment = BytesPerWord;
char* oldtop;
char* newtop;

Expand All @@ -473,10 +472,10 @@ void ArchiveBuilder::make_shallow_copy(DumpRegion *dump_region, SourceObjInfo* s
Klass* klass = (Klass*)src;
if (klass->is_instance_klass()) {
SystemDictionaryShared::validate_before_archiving(InstanceKlass::cast(klass));
dump_region->allocate(sizeof(address), BytesPerWord);
dump_region->allocate(sizeof(address));
}
}
dest = dump_region->allocate(bytes, alignment);
dest = dump_region->allocate(bytes);
newtop = dump_region->top();

memcpy(dest, src, bytes);
Expand Down
10 changes: 7 additions & 3 deletions src/hotspot/share/memory/archiveUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ address* ArchivePtrMarker::_ptr_base;
address* ArchivePtrMarker::_ptr_end;
bool ArchivePtrMarker::_compacted;

// Metaspace::allocate() requires that all blocks must be aligned with KlassAlignmentInBytes.
// We enforce the same alignment rule in blocks allocated from the shared space.
const int SharedSpaceObjectAlignment = KlassAlignmentInBytes;

void ArchivePtrMarker::initialize(CHeapBitMap* ptrmap, address* ptr_base, address* ptr_end) {
assert(_ptrmap == NULL, "initialize only once");
_ptr_base = ptr_base;
Expand Down Expand Up @@ -165,9 +169,9 @@ char* DumpRegion::expand_top_to(char* newtop) {
return _top;
}

char* DumpRegion::allocate(size_t num_bytes, size_t alignment) {
char* p = (char*)align_up(_top, alignment);
char* newtop = p + align_up(num_bytes, alignment);
char* DumpRegion::allocate(size_t num_bytes) {
char* p = (char*)align_up(_top, (size_t)SharedSpaceObjectAlignment);
char* newtop = p + align_up(num_bytes, (size_t)SharedSpaceObjectAlignment);
expand_top_to(newtop);
memset(p, 0, newtop - p);
return p;
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/share/memory/archiveUtils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ class DumpRegion {
DumpRegion(const char* name) : _name(name), _base(NULL), _top(NULL), _end(NULL), _is_packed(false) {}

char* expand_top_to(char* newtop);
char* allocate(size_t num_bytes, size_t alignment=BytesPerWord);
char* allocate(size_t num_bytes);

void append_intptr_t(intptr_t n, bool need_to_mark = false);

Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/memory/cppVtables.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ template <class T>
intptr_t* CppVtableCloner<T>::allocate(const char* name) {
assert(is_aligned(mc_region()->top(), sizeof(intptr_t)), "bad alignment");
int n = get_vtable_length(name);
_info = (CppVtableInfo*)mc_region()->allocate(CppVtableInfo::byte_size(n), sizeof(intptr_t));
_info = (CppVtableInfo*)mc_region()->allocate(CppVtableInfo::byte_size(n));
_info->set_vtable_size(n);

intptr_t* p = clone_vtable(name, _info);
Expand Down Expand Up @@ -242,7 +242,7 @@ static intptr_t** _cloned_cpp_vtptrs = NULL;
void CppVtables::allocate_cloned_cpp_vtptrs() {
assert(DumpSharedSpaces, "must");
size_t vtptrs_bytes = _num_cloned_vtable_kinds * sizeof(intptr_t*);
_cloned_cpp_vtptrs = (intptr_t**)mc_region()->allocate(vtptrs_bytes, sizeof(intptr_t*));
_cloned_cpp_vtptrs = (intptr_t**)mc_region()->allocate(vtptrs_bytes);
}

void CppVtables::serialize_cloned_cpp_vtptrs(SerializeClosure* soc) {
Expand Down

1 comment on commit ea27a54

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented on ea27a54 Oct 5, 2020

Choose a reason for hiding this comment

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

Please sign in to comment.