RFR: 8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x40001000000 crashes on macos-aarch64 [v10]

[Ioi Lam]

On Thu, 5 Dec 2024 16:51:21 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> Added checks to verify that the given compressed class base or shared base address and shift given will be decodable for aarch64.  This code is moved in PR https://github.com/openjdk/jdk/pull/20677 so this would be moved to the new place once that's integrated.
>> 
>> Tested with tier1-7.
>
> Coleen Phillimore has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge branch 'master' into decode-mode
>  - Recalculate decode mode if OS picks a different encoding.
>  - Only calculate klass-decode-mode in one place.
>  - Missed one. Another good reason to not copy code.
>  - Rename ceil_log2 to log2i_ceil
>  - Merge branch 'master' into decode-mode
>  - Merge with incoming change for PR https://github.com/openjdk/jdk/pull/21932
>  - Merge branch 'master' into decode-mode
>  - include formatBuffer.hpp
>  - 8340212: -Xshare:off -XX:CompressedClassSpaceBaseAddress=0x40001000000 crashes on macos-aarch64

Changes requested by iklam (Reviewer).

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 5328:

> 5326: 
> 5327: // Check if one of the above decoding modes will work for given base, shift and range.
> 5328: bool MacroAssembler::check_klass_decode_mode(address base, int shift, const size_t range) {

Can we refactor the code so that `check_klass_decode_mode()` only does the checking and have no side effects?

This code is called from CDS, which sometimes will unmap the archive and retry at an os-assigned random address, so updating the actual base/shift here will be problematic.

src/hotspot/share/cds/metaspaceShared.cpp line 1496:

> 1494:                   p2i(base_address), precomputed_narrow_klass_shift);
> 1495:     use_archive_base_addr = false;
> 1496:   }

It seems there's no easy way of testing this code, as the `base_address` need to come from a CDS archive, but the CDS archive cannot contain invalid base addresses.

Maybe we should change this to `log_error()`, or even `fatal()`

test/hotspot/jtreg/runtime/CompressedOops/CompressedClassPointersEncodingScheme.java line 87:

> 85: 
> 86:         // We ignore cases where we were not able to map at the force address
> 87:         if (!output.contains("Successfully forced class space address to " + forceAddressString)) {

"Successfully forced class space address" seems misleading. Can we change the code in metaspace.cpp to say something like "reserved class space at ...."?

-------------

PR Review: https://git.openjdk.org/jdk/pull/21695#pullrequestreview-2489481553
PR Review Comment: https://git.openjdk.org/jdk/pull/21695#discussion_r1876384125
PR Review Comment: https://git.openjdk.org/jdk/pull/21695#discussion_r1876392335
PR Review Comment: https://git.openjdk.org/jdk/pull/21695#discussion_r1876389280