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

[David Holmes]

On 29/06/2016 7:09 AM, serguei.spitsyn at oracle.com wrote:
> 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.

Disagree. You would say that a pointer is NULL, not that it points to a 
NULL.

David


>
> Thanks,
> Serguei
>
>
>>
>>
>> Dan
>