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

[Thomas Stuefe]

On Thu, 21 Nov 2024 13:34:54 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 incrementally with one additional commit since the last revision:
> 
>   Merge with incoming change for PR https://github.com/openjdk/jdk/pull/21932

Hi Coleen,

sorry for the late response.

I like your solution, but wonder whether we can simplify it. AFAIU, for the CDS=on path, you (1) test first in `MetaspaceShared::reserve_address_space_for_archives`, and switch off use_archive_base_addr on error. Then you check again after initialization in (2) `CompressedKlassPointers::initialize_for_given_encoding`, fatal'ing on error.

(1) means you need a pre-initialization variant of the check where you funnel through base, shift and range.

But (1) can only cause an error in two cases:
- User specified a wrong SharedBaseAddress
- the SharedBaseAddress we hardcoded as default is wrong.

The former is a user error and should result in an initialization error, exactly like we do for a wrong `CompressedClassSpaceBaseAddress`. The latter should never happen.

How about:

- you remove the check in (1). 
- you extend the error message in (2) to check if user specified SharedBaseAddress - if he did, do an initialization error, if it was our hard-coded SharedBaseAddress that was wrong, just assert.
- Then you can remove all variants of check_klass_decode_mode that take explicit arguments.

Furthermore, since we now check at initialization time, how about simplifying check_klass_decode_mode to:


bool MacroAssembler::check_klass_decode_mode() {
  return klass_decode_mode() != KlassDecodeNone;
}


and change `MacroAssembler::klass_decode_mode()` to not assert but to return KlassDecodeNone ? Then the relevant checking parts are not duplicated. We don't need the asserts in `MacroAssembler::klass_decode_mode()` anymore since we check at VM initialization now, and CompressedKlassPointers base and shift never change.

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

PR Comment: https://git.openjdk.org/jdk/pull/21695#issuecomment-2507452065