Skip to content

Commit

Permalink
8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x4000100000…
Browse files Browse the repository at this point in the history
…0 crashes on macos-aarch64

Reviewed-by: iklam
Backport-of: a6277bb521e07e569cd75a4641b2a05a26f47b0a
  • Loading branch information
coleenp committed Dec 11, 2024
1 parent 03bdee0 commit 950c8ad
Show file tree
Hide file tree
Showing 8 changed files with 123 additions and 24 deletions.
14 changes: 13 additions & 1 deletion src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@
*/

#include "precompiled.hpp"
#include "asm/assembler.hpp"
#include "asm/macroAssembler.hpp"
#include "logging/log.hpp"
#include "oops/compressedKlass.hpp"
#include "memory/metaspace.hpp"
#include "runtime/java.hpp"
#include "runtime/os.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/formatBuffer.hpp"

// Helper function; reserve at an address that is compatible with EOR
static char* reserve_at_eor_compatible_address(size_t size, bool aslr) {
Expand Down Expand Up @@ -79,6 +81,7 @@ static char* reserve_at_eor_compatible_address(size_t size, bool aslr) {
}
return result;
}

char* CompressedKlassPointers::reserve_address_space_for_compressed_classes(size_t size, bool aslr, bool optimize_for_zero_base) {

char* result = nullptr;
Expand Down Expand Up @@ -117,3 +120,12 @@ char* CompressedKlassPointers::reserve_address_space_for_compressed_classes(size

return result;
}

bool CompressedKlassPointers::check_klass_decode_mode(address base, int shift, const size_t range) {
return MacroAssembler::check_klass_decode_mode(base, shift, range);
}

bool CompressedKlassPointers::set_klass_decode_mode() {
const size_t range = klass_range_end() - base();
return MacroAssembler::set_klass_decode_mode(_base, _shift, range);
}
43 changes: 29 additions & 14 deletions src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5291,32 +5291,47 @@ void MacroAssembler::decode_heap_oop_not_null(Register dst, Register src) {
MacroAssembler::KlassDecodeMode MacroAssembler::_klass_decode_mode(KlassDecodeNone);

MacroAssembler::KlassDecodeMode MacroAssembler::klass_decode_mode() {
assert(UseCompressedClassPointers, "not using compressed class pointers");
assert(Metaspace::initialized(), "metaspace not initialized yet");
assert(_klass_decode_mode != KlassDecodeNone, "should be initialized");
return _klass_decode_mode;
}

if (_klass_decode_mode != KlassDecodeNone) {
return _klass_decode_mode;
}
MacroAssembler::KlassDecodeMode MacroAssembler::klass_decode_mode(address base, int shift, const size_t range) {
assert(UseCompressedClassPointers, "not using compressed class pointers");

// KlassDecodeMode shouldn't be set already.
assert(_klass_decode_mode == KlassDecodeNone, "set once");

if (CompressedKlassPointers::base() == nullptr) {
return (_klass_decode_mode = KlassDecodeZero);
if (base == nullptr) {
return KlassDecodeZero;
}

if (operand_valid_for_logical_immediate(
/*is32*/false, (uint64_t)CompressedKlassPointers::base())) {
const size_t range = CompressedKlassPointers::klass_range_end() - CompressedKlassPointers::base();
/*is32*/false, (uint64_t)base)) {
const uint64_t range_mask = right_n_bits(log2i_ceil(range));
if (((uint64_t)CompressedKlassPointers::base() & range_mask) == 0) {
return (_klass_decode_mode = KlassDecodeXor);
if (((uint64_t)base & range_mask) == 0) {
return KlassDecodeXor;
}
}

const uint64_t shifted_base =
(uint64_t)CompressedKlassPointers::base() >> CompressedKlassPointers::shift();
guarantee((shifted_base & 0xffff0000ffffffff) == 0,
"compressed class base bad alignment");
(uint64_t)base >> shift;
if ((shifted_base & 0xffff0000ffffffff) == 0) {
return KlassDecodeMovk;
}

// No valid encoding.
return KlassDecodeNone;
}

// Check if one of the above decoding modes will work for given base, shift and range.
bool MacroAssembler::check_klass_decode_mode(address base, int shift, const size_t range) {
return klass_decode_mode(base, shift, range) != KlassDecodeNone;
}

return (_klass_decode_mode = KlassDecodeMovk);
bool MacroAssembler::set_klass_decode_mode(address base, int shift, const size_t range) {
_klass_decode_mode = klass_decode_mode(base, shift, range);
return _klass_decode_mode != KlassDecodeNone;
}

void MacroAssembler::encode_klass_not_null(Register dst, Register src) {
Expand Down
13 changes: 12 additions & 1 deletion src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,22 @@ class MacroAssembler: public Assembler {
KlassDecodeMovk
};

KlassDecodeMode klass_decode_mode();
// Calculate decoding mode based on given parameters, used for checking then ultimately setting.
static KlassDecodeMode klass_decode_mode(address base, int shift, const size_t range);

private:
static KlassDecodeMode _klass_decode_mode;

// Returns above setting with asserts
static KlassDecodeMode klass_decode_mode();

public:
// Checks the decode mode and returns false if not compatible with preferred decoding mode.
static bool check_klass_decode_mode(address base, int shift, const size_t range);

// Sets the decode mode and returns false if cannot be set.
static bool set_klass_decode_mode(address base, int shift, const size_t range);

public:
MacroAssembler(CodeBuffer* code) : Assembler(code) {}

Expand Down
18 changes: 14 additions & 4 deletions src/hotspot/share/cds/metaspaceShared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,15 +145,16 @@ size_t MetaspaceShared::core_region_alignment() {
}

static bool shared_base_valid(char* shared_base) {
// We check user input for SharedBaseAddress at dump time. We must weed out values
// we already know to be invalid later.
// We check user input for SharedBaseAddress at dump time.

// At CDS runtime, "shared_base" will be the (attempted) mapping start. It will also
// be the encoding base, since the the headers of archived base objects (and with Lilliput,
// the prototype mark words) carry pre-computed narrow Klass IDs that refer to the mapping
// start as base.
//
// Therefore, "shared_base" must be later usable as encoding base.
// On AARCH64, The "shared_base" may not be later usable as encoding base, depending on the
// total size of the reserved area and the precomputed_narrow_klass_shift. This is checked
// before reserving memory. Here we weed out values already known to be invalid later.
return AARCH64_ONLY(is_aligned(shared_base, 4 * G)) NOT_AARCH64(true);
}

Expand Down Expand Up @@ -1486,6 +1487,15 @@ char* MetaspaceShared::reserve_address_space_for_archives(FileMapInfo* static_ma
const size_t total_range_size =
archive_space_size + gap_size + class_space_size;

// Test that class space base address plus shift can be decoded by aarch64, when restored.
const int precomputed_narrow_klass_shift = ArchiveBuilder::precomputed_narrow_klass_shift();
if (!CompressedKlassPointers::check_klass_decode_mode(base_address, precomputed_narrow_klass_shift,
total_range_size)) {
log_info(cds)("CDS initialization: Cannot use SharedBaseAddress " PTR_FORMAT " with precomputed shift %d.",
p2i(base_address), precomputed_narrow_klass_shift);
use_archive_base_addr = false;
}

assert(total_range_size > ccs_begin_offset, "must be");
if (use_windows_memory_mapping() && use_archive_base_addr) {
if (base_address != nullptr) {
Expand Down Expand Up @@ -1525,7 +1535,7 @@ char* MetaspaceShared::reserve_address_space_for_archives(FileMapInfo* static_ma
}

// Paranoid checks:
assert(base_address == nullptr || (address)total_space_rs.base() == base_address,
assert(!use_archive_base_addr || (address)total_space_rs.base() == base_address,
"Sanity (" PTR_FORMAT " vs " PTR_FORMAT ")", p2i(base_address), p2i(total_space_rs.base()));
assert(is_aligned(total_space_rs.base(), base_address_alignment), "Sanity");
assert(total_space_rs.size() == total_range_size, "Sanity");
Expand Down
21 changes: 21 additions & 0 deletions src/hotspot/share/oops/compressedKlass.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "runtime/java.hpp"
#include "runtime/os.hpp"
#include "utilities/debug.hpp"
#include "utilities/formatBuffer.hpp"
#include "utilities/globalDefinitions.hpp"
#include "utilities/ostream.hpp"

Expand Down Expand Up @@ -176,6 +177,12 @@ void CompressedKlassPointers::initialize_for_given_encoding(address addr, size_t

calc_lowest_highest_narrow_klass_id();

// This has already been checked for SharedBaseAddress and if this fails, it's a bug in the allocation code.
if (!set_klass_decode_mode()) {
fatal("base=" PTR_FORMAT " given with shift %d, cannot be used to encode class pointers",
p2i(_base), _shift);
}

DEBUG_ONLY(sanity_check_after_initialization();)
}

Expand Down Expand Up @@ -267,6 +274,20 @@ void CompressedKlassPointers::initialize(address addr, size_t len) {

calc_lowest_highest_narrow_klass_id();

// Initialize klass decode mode and check compability with decode instructions
if (!set_klass_decode_mode()) {

// Give fatal error if this is a specified address
if ((address)CompressedClassSpaceBaseAddress == _base) {
vm_exit_during_initialization(
err_msg("CompressedClassSpaceBaseAddress=" PTR_FORMAT " given with shift %d, cannot be used to encode class pointers",
CompressedClassSpaceBaseAddress, _shift));
} else {
// If this fails, it's a bug in the allocation code.
fatal("CompressedClassSpaceBaseAddress=" PTR_FORMAT " given with shift %d, cannot be used to encode class pointers",
p2i(_base), _shift);
}
}
#ifdef ASSERT
sanity_check_after_initialization();
#endif
Expand Down
3 changes: 3 additions & 0 deletions src/hotspot/share/oops/compressedKlass.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ class CompressedKlassPointers : public AllStatic {
is_aligned(addr, klass_alignment_in_bytes());
}

// Check that with the given base, shift and range, aarch64 an encode and decode the klass pointer.
static bool check_klass_decode_mode(address base, int shift, const size_t range) NOT_AARCH64({ return true;});
static bool set_klass_decode_mode() NOT_AARCH64({ return true;}); // can be called after initialization
};

#endif // SHARE_OOPS_COMPRESSEDKLASS_HPP
4 changes: 0 additions & 4 deletions test/hotspot/jtreg/ProblemList.txt
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ runtime/ErrorHandling/MachCodeFramesInErrorFile.java 8313315 linux-ppc64le
runtime/cds/appcds/customLoader/HelloCustom_JFR.java 8241075 linux-all,windows-x64
runtime/NMT/VirtualAllocCommitMerge.java 8309698 linux-s390x

# Fails with +UseCompactObjectHeaders on aarch64
runtime/cds/appcds/SharedBaseAddress.java 8340212 linux-aarch64,macosx-aarch64
runtime/cds/SharedBaseAddress.java 8340212 linux-aarch64,macosx-aarch64

applications/jcstress/copy.java 8229852 linux-all

containers/docker/TestJcmd.java 8278102 linux-all
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,33 @@ private static void test(long forceAddress, boolean COH, long classSpaceSize, lo
output.shouldContain("Narrow klass base: " + expectedEncodingBaseString + ", Narrow klass shift: " + expectedEncodingShift);
}

private static void testFailure(String forceAddressString) throws IOException {
ProcessBuilder pb = ProcessTools.createLimitedTestJavaProcessBuilder(
"-Xshare:off", // to make CompressedClassSpaceBaseAddress work
"-XX:+UnlockDiagnosticVMOptions",
"-XX:CompressedClassSpaceBaseAddress=" + forceAddressString,
"-Xmx128m",
"-Xlog:metaspace*",
"-version");
OutputAnalyzer output = new OutputAnalyzer(pb.start());

output.reportDiagnosticSummary();

// We ignore cases where we were not able to map at the force address
if (!output.contains("Successfully forced class space address to " + forceAddressString)) {
throw new SkippedException("Skipping because we cannot force ccs to " + forceAddressString);
}

if (Platform.isAArch64()) {
output.shouldHaveExitValue(1);
output.shouldContain("Error occurred during initialization of VM");
output.shouldContain("CompressedClassSpaceBaseAddress=" + forceAddressString +
" given with shift 0, cannot be used to encode class pointers");
} else {
output.shouldHaveExitValue(0);
}
}

final static long K = 1024;
final static long M = K * 1024;
final static long G = M * 1024;
Expand Down Expand Up @@ -123,5 +150,9 @@ public static void main(String[] args) throws Exception {
expectedShift = 10;
test(forceAddress, true, ccsSize, forceAddress, expectedShift);
}

// Test failure for -XX:CompressedClassBaseAddress and -Xshare:off
testFailure("0x0000040001000000");

}
}

0 comments on commit 950c8ad

Please sign in to comment.