RFR: 8286490: JvmtiEventControllerPrivate::set_event_callbacks CLEARING_MASK computation is incorrect [v2]

[Alex Menkov]

On Tue, 24 May 2022 21:47:54 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> It was a typo when the original CLEARING_MASK was initially introduced:
>> 
>> 
>>  // Mask to clear normal event bits.
>>   const jlong CLEARING_MASK = (1L >> (TOTAL_MIN_EVENT_TYPE_VAL - TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;
>>   // Avoid cleaning extension event bits.
>>   jlong enabled_bits = CLEARING_MASK & env->env_event_enable()->_event_callback_enabled.get_bits();
>> 
>> ``` 
>> 
>> The first TOTAL_MIN_EVENT_TYPE_VAL has to be JVMTI_MIN_EVENT_TYPE_VAL as below:
>> `  const jlong CLEARING_MASK = (1L >> (JVMTI_MIN_EVENT_TYPE_VAL - TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;`
>> 
>> Let me provide some background to understand how this mask is used.
>> 
>> Normal JVMTI event types are numbered from 50 to 88. There are two constants:
>> 
>>   JVMTI_MIN_EVENT_TYPE_VAL = 50
>>   JVMTI_MAX_EVENT_TYPE_VAL = 88
>> 
>> The extension event types are numbered from 47 to 49. There are also two constants:
>> 
>>   EXT_MIN_EVENT_TYPE_VAL = 47
>>   EXT_MAX_EVENT_TYPE_VAL = 49
>> 
>> 
>> There are also two additional constants:
>> 
>>   TOTAL_MIN_EVENT_TYPE_VAL = 47
>>   TOTAL_MAX_EVENT_TYPE_VAL = 88
>> 
>> 
>> The `enabled_bits` are shifted left by 47, so that the 0-bit in `enabled_bits` is for first extension event type. And the first normal JVMTI event type is numbered 3 in the `enabled_bits` bit mask. The `CLEARING_MASK` is used to clear only normal JVMTI event types but keep the extension event bits (0-2) unchanged.
>> 
>> Testing:
>> I'll run all JVMTI tests including nsk.jvmti tests and serviceability/jvmti tests including those added in Loom for virtual thread coverage.
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   add extra test coverage for JVMTI extension events: VirtualThreadMount/VirtualThreadUnmount

src/hotspot/share/prims/jvmtiEventController.cpp line 775:

> 773:   env->set_event_callbacks(callbacks, size_of_callbacks);
> 774:   // Mask to clear normal event bits.
> 775:   const jlong CLEARING_MASK = (1L >> (JVMTI_MIN_EVENT_TYPE_VAL - TOTAL_MIN_EVENT_TYPE_VAL)) - 1L;

I don't follow how this works..
Should it be "<<" instead of ">>" ?

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

PR: https://git.openjdk.java.net/jdk/pull/8860