RFR: 8261236: C2: ClhsdbJstackXcompStress test fails when StressGCM is enabled

[Nick Gasson]

On Mon, 19 Jul 2021 23:50:07 GMT, Tom Rodriguez <never at 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