RFR: 8343699: [aarch64] Bug in MacroAssembler::klass_decode_mode() [v2]
Thomas Stuefe
stuefe at openjdk.org
Thu Dec 5 14:11:30 UTC 2024
> In `MacroAssembler::klass_decode_mode(),` there is a subtle bug in the xor part.
>
>
> if (operand_valid_for_logical_immediate(
> /*is32*/false, (uint64_t)CompressedKlassPointers::base())) {
> (1) const size_t range = CompressedKlassPointers::klass_range_end() - CompressedKlassPointers::base();
> (2) const uint64_t range_mask = (1ULL << log2i(range)) - 1;
> (3) if (((uint64_t)CompressedKlassPointers::base() & range_mask) == 0) {
> log_debug(metaspace)("MacroAssembler::klass_decode_mode xor");
> return (_klass_decode_mode = KlassDecodeXor);
> }
> }
>
>
> We first determine if the encoding base is encodable as immediate. If it is, then we check if it intersects with the value range of a narrow Klass ID.
>
> (Note: the code ignores encoding shift since the XOR will be applied to the pre-shifted narrow Klass value later.)
>
> The test is done by
> 1) calculating the range the encoding needs to cover
> 2) calculating a corresponding bit mask
> 3) checking that this mask does not intersect with the encoding base.
>
> (2) contains a wrongness: `range_mask = (1ULL << log2i(range)) - 1` . `log2i` returns the largest log2 value *smaller* or equal to input. So, for `range` values that are not a pow2 value, the resulting mask will be one bit too short. As an effect, the code may chose XOR for cases where the lowest encoding base bit can intersect the highest narrow Klass ID bit, thus making the xor lossy.
>
> ----
>
> Example:
>
> Let range be 80MB (`-XX:CompressedClassSpaceSize=80m -Xshare:off`).
>
> Then, range_mask = `1 << log2i(80m) - 1` => `(1 << 26) - 1` => `0x3ff_ffff`
> The largest possible nKlass value, however, sits just below 80MB => `0x500_0000`. As we see, the mask does not cover the full extent of the narrow Klass ID value range (bit 26 is not covered).
> Hence, if we have a base with bit 26 set, its bit 26 intersects a possible bit 26 in high-value narrow Klass ID. The xor would not be lossless.
>
> ----
>
> The error is very unlikely because
> - we try to reserve the klass range at addresses that are guaranteed not to intersect with the narrow Klass range. Only if that fails - very unlikely - we use whatever address the OS gives us. Only then could we end up with such an address.
> - The class space is rarely filled so high that the highest bit of a narrowKlass ID is `1`.
>
> ----
>
> Reproduce:
>
>
> java -XX:CompressedClassSpaceBaseAddress=0x1Fc000000 -XX:CompressedClassSpaceSize=80m -Xlog:metaspace* -Xshare:off
>
>
> it does not reproduce an error, but causes the JVM to start in XOR mode with an enc...
Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
- Merge branch 'openjdk:master' into JDK-8343699-aarch64-Bug-in-MacroAssembler-klass_decode_mode
- Merge branch 'openjdk:master' into JDK-8343699-aarch64-Bug-in-MacroAssembler-klass_decode_mode
- Merge branch 'openjdk:master' into JDK-8343699-aarch64-Bug-in-MacroAssembler-klass_decode_mode
- JDK-8343699-aarch64-Bug-in-MacroAssembler-klass_decode_mode
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/21932/files
- new: https://git.openjdk.org/jdk/pull/21932/files/2e9cccab..1803a934
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=21932&range=01
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=21932&range=00-01
Stats: 301913 lines in 5184 files changed: 142473 ins; 137515 del; 21925 mod
Patch: https://git.openjdk.org/jdk/pull/21932.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/21932/head:pull/21932
PR: https://git.openjdk.org/jdk/pull/21932
More information about the hotspot-dev
mailing list