From 950c8adfd7a19c1de3d0e36afdc17e5dad15bc30 Mon Sep 17 00:00:00 2001 From: Coleen Phillimore Date: Wed, 11 Dec 2024 17:16:13 +0000 Subject: [PATCH] 8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x40001000000 crashes on macos-aarch64 Reviewed-by: iklam Backport-of: a6277bb521e07e569cd75a4641b2a05a26f47b0a --- .../cpu/aarch64/compressedKlass_aarch64.cpp | 14 +++++- .../cpu/aarch64/macroAssembler_aarch64.cpp | 43 +++++++++++++------ .../cpu/aarch64/macroAssembler_aarch64.hpp | 13 +++++- src/hotspot/share/cds/metaspaceShared.cpp | 18 ++++++-- src/hotspot/share/oops/compressedKlass.cpp | 21 +++++++++ src/hotspot/share/oops/compressedKlass.hpp | 3 ++ test/hotspot/jtreg/ProblemList.txt | 4 -- ...CompressedClassPointersEncodingScheme.java | 31 +++++++++++++ 8 files changed, 123 insertions(+), 24 deletions(-) diff --git a/src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp b/src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp index b96241aab19..6ad6e3134bc 100644 --- a/src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/compressedKlass_aarch64.cpp @@ -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) { @@ -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; @@ -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); +} diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp index a836d71205e..d561fb912a3 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp @@ -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) { diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp index d9686aec3a9..244de10d0e2 100644 --- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp +++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp @@ -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) {} diff --git a/src/hotspot/share/cds/metaspaceShared.cpp b/src/hotspot/share/cds/metaspaceShared.cpp index f21b9c9060d..6348b04cbe1 100644 --- a/src/hotspot/share/cds/metaspaceShared.cpp +++ b/src/hotspot/share/cds/metaspaceShared.cpp @@ -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); } @@ -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) { @@ -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"); diff --git a/src/hotspot/share/oops/compressedKlass.cpp b/src/hotspot/share/oops/compressedKlass.cpp index f3c6fe92897..97415cf6e85 100644 --- a/src/hotspot/share/oops/compressedKlass.cpp +++ b/src/hotspot/share/oops/compressedKlass.cpp @@ -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" @@ -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();) } @@ -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 diff --git a/src/hotspot/share/oops/compressedKlass.hpp b/src/hotspot/share/oops/compressedKlass.hpp index 9e3a09d73b9..f0615283f33 100644 --- a/src/hotspot/share/oops/compressedKlass.hpp +++ b/src/hotspot/share/oops/compressedKlass.hpp @@ -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 diff --git a/test/hotspot/jtreg/ProblemList.txt b/test/hotspot/jtreg/ProblemList.txt index 08678af706d..af16f9b228a 100644 --- a/test/hotspot/jtreg/ProblemList.txt +++ b/test/hotspot/jtreg/ProblemList.txt @@ -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 diff --git a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java index 665c4cb8b9f..6a7ab088fac 100644 --- a/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java +++ b/test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java @@ -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; @@ -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"); + } }