On Mon, 19 Jul 2021 23:50:07 GMT, Tom Rodriguez <ne...@openjdk.org> wrote:
>> The clhsdb jstack command crashes when the debugged program is run with
>> `-Xcomp -XX:+StressGCM -XX:StressSeed=2` on AArch64:
>>
>>
>> sun.jvm.hotspot.utilities.AssertionFailure: sanity check
>> at jdk.hotspot.agent/sun.jvm.hotspot.utilities.Assert.that(Assert.java:32)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.RegisterMap.setLocation(RegisterMap.java:160)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.compiler.ImmutableOopMapSet.updateRegisterMap(ImmutableOopMapSet.java:303)
>> at
>> jdk.hotspot.agent/sun.jvm.hotspot.runtime.aarch64.AARCH64Frame.senderForCompiledFrame(AARCH64Frame.java:407)
>>
>>
>> The assertion failure here is:
>>
>>
>> Assert.that(0 <= i && i < regCount, "sanity check");
>>
>>
>> I.e. there's an invalid register in the oop map.
>>
>> The problem seems to be caused by the changes in JDK-8231586 which changed
>> `OopMapValue::oop_types` from a bit mask to normal integer enum. However the
>> changes in the C++ code weren't mirrored in SA's OopMapStream which still
>> treats OopTypes as a bit mask.
>>
>> The particular oop map this is crashing on looks like this:
>>
>>
>> ImmutableOopMap {[24]=Oop [32]=Oop [40]=Derived_oop_[24] } pc offsets: 324
>>
>>
>> The code is looking for callee saved values (type=2) by AND-ing with each
>> oop value type in the set, so it wrongly interprets the derived oop [40]
>> (type=3) as a callee saved register.
>>
>> This patch just mirrors the changes to the C++ code into the corresponding
>> SA classes. The C++ OopMapStream constructor no longer takes a type filter
>> argument and callers are expected filter themselves, so I've made the same
>> change to the Java code.
>>
>> This bug can also be seen using the clhsdb "disassemble" command. For
>> example the above oop map is currently printed incorrectly as:
>>
>>
>> OopMap:
>> NarrowOops:[40]
>> Callee saved:[40] = [24]
>> Derived oops:[40] = [24]
>>
>>
>> With this patch it becomes:
>>
>>
>> OopMap:
>> Oops:[24] [32]
>> Derived oops:[40] = [24]
>>
>>
>> This bug was reported on AArch64 but it seems to be just luck that we don't
>> see it on other platforms.
>>
>> Tested tier1 and hotspot_serviceability on AArch64 and x86.
>
> Yes that looks like the right fix to me. Sorry for missing this part.
Thanks @tkrodriguez . Could I get a second review for this please?
-------------
PR: https://git.openjdk.java.net/jdk/pull/4807
