stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8270308: Arena::Amalloc may return misaligned address on 32-bit

Browse Source 
Reviewed-by: coleenp, kbarrett
This commit is contained in:
Thomas Stuefe 2021-07-27 04:21:56 +00:00
parent fde183130b
commit 45d277feb0
3 changed files with 115 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

48
src/hotspot/share/memory/arena.cpp
View File

@ -30,8 +30,17 @@
#include "runtime/task.hpp"
#include "runtime/threadCritical.hpp"
#include "services/memTracker.hpp"
#include "utilities/align.hpp"
#include "utilities/debug.hpp"
#include "utilities/ostream.hpp"
// Pre-defined default chunk sizes must be arena-aligned, see Chunk::operator new()
STATIC_ASSERT(is_aligned((int)Chunk::tiny_size, ARENA_AMALLOC_ALIGNMENT));
STATIC_ASSERT(is_aligned((int)Chunk::init_size, ARENA_AMALLOC_ALIGNMENT));
STATIC_ASSERT(is_aligned((int)Chunk::medium_size, ARENA_AMALLOC_ALIGNMENT));
STATIC_ASSERT(is_aligned((int)Chunk::size, ARENA_AMALLOC_ALIGNMENT));
STATIC_ASSERT(is_aligned((int)Chunk::non_pool_size, ARENA_AMALLOC_ALIGNMENT));
//--------------------------------------------------------------------------------------
// ChunkPool implementation
@ -171,13 +180,30 @@ class ChunkPoolCleaner : public PeriodicTask {
//--------------------------------------------------------------------------------------
// Chunk implementation
void* Chunk::operator new (size_t requested_size, AllocFailType alloc_failmode, size_t length) throw() {
// requested_size is equal to sizeof(Chunk) but in order for the arena
// allocations to come out aligned as expected the size must be aligned
// to expected arena alignment.
// expect requested_size but if sizeof(Chunk) doesn't match isn't proper size we must align it.
assert(ARENA_ALIGN(requested_size) == aligned_overhead_size(), "Bad alignment");
size_t bytes = ARENA_ALIGN(requested_size) + length;
void* Chunk::operator new (size_t sizeofChunk, AllocFailType alloc_failmode, size_t length) throw() {
// - requested_size = sizeof(Chunk)
// - length = payload size
// We must ensure that the boundaries of the payload (C and D) are aligned to 64-bit:
//
// +-----------+--+--------------------------------------------+
// | |g | |
// | Chunk |a | Payload |
// | |p | |
// +-----------+--+--------------------------------------------+
// A B C D
//
// - The Chunk is allocated from C-heap, therefore its start address (A) should be
// 64-bit aligned on all our platforms, including 32-bit.
// - sizeof(Chunk) (B) may not be aligned to 64-bit, and we have to take that into
// account when calculating the Payload bottom (C) (see Chunk::bottom())
// - the payload size (length) must be aligned to 64-bit, which takes care of 64-bit
// aligning (D)
assert(sizeofChunk == sizeof(Chunk), "weird request size");
assert(is_aligned(length, ARENA_AMALLOC_ALIGNMENT), "chunk payload length misaligned: "
SIZE_FORMAT ".", length);
size_t bytes = ARENA_ALIGN(sizeofChunk) + length;
switch (length) {
case Chunk::size: return ChunkPool::large_pool()->allocate(bytes, alloc_failmode);
case Chunk::medium_size: return ChunkPool::medium_pool()->allocate(bytes, alloc_failmode);
@ -188,6 +214,8 @@ void* Chunk::operator new (size_t requested_size, AllocFailType alloc_failmode,
if (p == NULL && alloc_failmode == AllocFailStrategy::EXIT_OOM) {
vm_exit_out_of_memory(bytes, OOM_MALLOC_ERROR, "Chunk::new");
}
// We rely on arena alignment <= malloc alignment.
assert(is_aligned(p, ARENA_AMALLOC_ALIGNMENT), "Chunk start address misaligned.");
return p;
}
}
@ -239,8 +267,7 @@ void Chunk::start_chunk_pool_cleaner_task() {
//------------------------------Arena------------------------------------------
Arena::Arena(MEMFLAGS flag, size_t init_size) : _flags(flag), _size_in_bytes(0) {
size_t round_size = (sizeof (char *)) - 1;
init_size = (init_size+round_size) & ~round_size;
init_size = ARENA_ALIGN(init_size);
_first = _chunk = new (AllocFailStrategy::EXIT_OOM, init_size) Chunk(init_size);
_hwm = _chunk->bottom(); // Save the cached hwm, max
_max = _chunk->top();
@ -340,7 +367,8 @@ size_t Arena::used() const {
// Grow a new Chunk
void* Arena::grow(size_t x, AllocFailType alloc_failmode) {
// Get minimal required size. Either real big, or even bigger for giant objs
size_t len = MAX2(x, (size_t) Chunk::size);
// (Note: all chunk sizes have to be 64-bit aligned)
size_t len = MAX2(ARENA_ALIGN(x), (size_t) Chunk::size);
Chunk *k = _chunk; // Get filled-up chunk address
_chunk = new (alloc_failmode, len) Chunk(len);

8
src/hotspot/share/memory/arena.hpp
View File

@ -52,11 +52,12 @@ class Chunk: CHeapObj<mtChunk> {
enum {
// default sizes; make them slightly smaller than 2**k to guard against
// buddy-system style malloc implementations
// Note: please keep these constants 64-bit aligned.
#ifdef _LP64
slack = 40, // [RGV] Not sure if this is right, but make it
// a multiple of 8.
#else
slack = 20, // suspected sizeof(Chunk) + internal malloc headers
slack = 24, // suspected sizeof(Chunk) + internal malloc headers
#endif
tiny_size = 256 - slack, // Size of first chunk (tiny)
@ -134,6 +135,11 @@ protected:
void* Amalloc(size_t x, AllocFailType alloc_failmode = AllocFailStrategy::EXIT_OOM) {
x = ARENA_ALIGN(x); // note for 32 bits this should align _hwm as well.
debug_only(if (UseMallocOnly) return malloc(x);)
// Amalloc guarantees 64-bit alignment and we need to ensure that in case the preceding
// allocation was AmallocWords. Only needed on 32-bit - on 64-bit Amalloc and AmallocWords are
// identical.
assert(is_aligned(_max, ARENA_AMALLOC_ALIGNMENT), "chunk end unaligned?");
NOT_LP64(_hwm = ARENA_ALIGN(_hwm));
return internal_amalloc(x, alloc_failmode);
}

70
test/hotspot/gtest/memory/test_arena.cpp Normal file
View File

@ -0,0 +1,70 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021 SAP SE. All rights reserved.
*
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
#include "precompiled.hpp"
#include "memory/arena.hpp"
#include "utilities/align.hpp"
#include "utilities/globalDefinitions.hpp"
#include "unittest.hpp"
#ifndef LP64
// These tests below are about alignment issues when mixing Amalloc and AmallocWords.
// Since on 64-bit these APIs offer the same alignment, they only matter for 32-bit.
TEST_VM(Arena, mixed_alignment_allocation) {
// Test that mixed alignment allocations work and provide allocations with the correct
// alignment
Arena ar(mtTest);
void* p1 = ar.AmallocWords(BytesPerWord);
void* p2 = ar.Amalloc(BytesPerLong);
ASSERT_TRUE(is_aligned(p1, BytesPerWord));
ASSERT_TRUE(is_aligned(p2, ARENA_AMALLOC_ALIGNMENT));
}
TEST_VM(Arena, Arena_with_crooked_initial_size) {
// Test that an arena with a crooked, not 64-bit aligned initial size
// works
Arena ar(mtTest, 4097);
void* p1 = ar.AmallocWords(BytesPerWord);
void* p2 = ar.Amalloc(BytesPerLong);
ASSERT_TRUE(is_aligned(p1, BytesPerWord));
ASSERT_TRUE(is_aligned(p2, ARENA_AMALLOC_ALIGNMENT));
}
TEST_VM(Arena, Arena_grows_large_unaligned) {
// Test that if the arena grows with a large unaligned value, nothing bad happens.
// We trigger allocation of a new, large, unaligned chunk with a non-standard size
// (only possible on 32-bit when allocating with word alignment).
// Then we alloc some more. If Arena::grow() does not correctly align, on 32-bit
// something should assert at some point.
Arena ar(mtTest, 100); // first chunk is small
void* p = ar.AmallocWords(Chunk::size + BytesPerWord); // if Arena::grow() misaligns, this asserts
// some more allocations for good measure
for (int i = 0; i < 100; i ++) {
ar.Amalloc(1);
}
}
#endif // LP64