Jigsaw Enhancement RFR round #3: 8159145 Add JVMTI function GetNamedModule

[serguei.spitsyn at oracle.com]

On 6/28/16 14:02, Daniel D. Daugherty wrote:
> On 6/28/16 2:11 PM, serguei.spitsyn at oracle.com wrote:
>> On 6/28/16 11:19, Daniel D. Daugherty wrote:
>>
>>>
>>>         I'll have to check the upper layers of this API, but if an
>>>         agent can pass a bad 'class_loader' parameter and get this
>>>         assert() to fire, then that's not good. Hopefully a bad
>>>         'class_loader' parameter is caught at a higher layer.
>>
>> Not sure, what do you mean.
>> Do you mean the generated JVMTI upper layer or the 
>> JvmtiEnv::GetNamedModule?
>> Probably, the generated code.
>
> I did mean the generated layer.

Ok, thanks.

>
>
>>
>>>
>>>         Update: Yes, passing a non-NULL jobject as the class_loader 
>>> parameter
>>>             when the jobject does not refer to a "class loader" is 
>>> caught
>>>             at the upper layer.
>>
>> The upper layer does not check that it is a class loader, just for 
>> non-NULL.
>> I think, it is good to have an assert here to double-checks the 
>> pre-conditions as other caller can be added later.
>> But I'm Ok to get rid of it if you suggest.
>
> Please keep the asserts. Basically I was mumbling to myself to
> make sure that the asserts could not be reached by user code
> and the "Update:" was to indicate that I did do.

Ok, thanks.


>
>
>>
>>>
>>> src/share/vm/prims/jvmti.xml
>>>     L6550:         <param id="module_ptr">
>>>     L6551: <outptr><jobject/></outptr>
>>>     L6552:           <description>
>>>     L6553:             On return, points to a 
>>> <code>java.lang.reflect.Module</code> object.
>>>     L6554:           </description>
>>>     L6555:         </param>
>>>
>>>         The above wording doesn't allow for module_ptr to be NULL which
>>>         is a mismatch with the description.
>>
>> I disagree (or maybe I got it incorrectly).
>> Pointing to NULL and be NULL is different.
>> It is not allowed for the module_ptr to be NULL but Ok to pint to 
>> NULL on return.
>
> I think the description needs to be:
>
>     On return, points to a <code>java.lang.reflect.Module</code> object
>     or points to a <code>NULL</code>.

Agreed, fixed.


Thanks,
Serguei


>
>
> Dan