RFR (M): JDK-8004967 - Default method cause java.lang.VerifyError: Illegal use of nonvirtual function call
coleen.phillimore at oracle.com
Fri Jan 18 14:07:57 PST 2013
On 1/18/2013 4:52 PM, Bharadwaj Yadavalli wrote:
> Hi Coleen,
> Thanks for the review.
> On 1/18/2013 2:28 PM, Coleen Phillimore wrote:
>> I think the extra check in the old verifier that the ACC flags are
>> set is going to cause a headache if we change the VM to change which
>> special ACC flags are set. I think the check should be in
>> JVM_IsVMGeneratedMethod() instead, and not check in the old verifier.
> The reason I kept the access flag check separate is to maintain a
> consistent check for overpass method in old and new verifiers.
> A method is considered overpass if the method type is
> ConstMethod::OVERPASS (see Method::is_overpass()) - a property not
> dependent on its access flags. The new verifier checks for overpass
> methods to decide whether to trust the code or not; additional access
> flag check is not done in the new verifier.
> I intend JVM_IsVMGeneratedMethodIx() to be functionally equivalent to
> Method::is_overpass(). To achieve this, my implementation of
> JVM_IsVMGeneratedMethodIx() basically returns (method->method_type()
> == ConstMethod::OVERPASS) - which is what Method::is_overpass() also
> does. To minimize any divergence, I now updated the implementation of
> JVM_IsVMGeneratedMethodIx() to call is_overpass(). This will ensure
> ultimately the same query is used to test for overpass methods by the
> old and new verifiers.
> Regarding checking of access flags: JVM_IsVMGeneratedMethodIx()
> pertains to the method type and has nothing to do with access flags. A
> generated method could have more than one legal set of access flags.
> Separately, JVM_GetMethodIxModifiersIx() provides the API to query
> method access flags and is not related to method type. I think, the
> functionality of these two APIs should not be mixed. If a decision
> needs to be made based on method type and access flags, I believe,
> separate queries should be used (as is done in the code currently
> under review).
This makes sense to keep these concerns separate.
> As I noted in the code comments, I am just performing an additional
> access flag check in the old verifier (which can also be potentially
> added in new verifier).
I wouldn't add the additional access flag check in the old verifier.
It doesn't add any extra security, and relies on an implementation
detail in the VM that is easily changed. It probably will be changed.
I would omit this access flag check (still check that it's a generated
method of course).
>> I think you have to file a CCC request to add this function.
> What are the criteria to file for a CCC? What are the circumstances
> under which one needs to be filed?
Changing an API is probably good enough to warrant a CCC request. I'll
send you how to do it (just found it).
More information about the hotspot-runtime-dev