Request for review: 8015385 Remove RelaxAccessControlCheck for JDK 8 bytecodes

[harold seigel]

Hi David,

Thanks for the review.  Please see inline comments.

Thanks, Harold

On 5/31/2013 3:31 AM, David Holmes wrote:
> Hi Harold,
>
> I'm not sure what the actual rules for this should be in terms of the 
> accessor and accessee having different versions. But isn't the main 
> issue the version of the accessee?
I'm also not sure what the actual rules should be.  I chose the stricter 
path of requiring the access check if either class is >= 52 in part 
because the existing checks for pre 49 class files require both accessor 
and accessee to be < 49.  If people feel strongly I can change it.
>
> Further your change will now require RelaxAccessControlCheck  be true 
> for pre 49 class files, but I thought this old code was supposed to 
> continue working?
Here's the code change using numbers instead of enums to make it easier 
to see the parentheses.  I think it shows that RelaxAccessControlCheck 
is not needed for pre 49 class files.

   if (*(*RelaxAccessControlCheck && accessor_ik->major_version() < 52 
&& accessee_ik->major_version() < 52*)* *||*
***(*accessor_ik->major_version() < 49 && accessee_ik->major_version() < 
49*)*) {
     return classloader_only &&
       Verifier::relax_verify_for(accessor_ik->class_loader()) &&
       accessor_ik->protection_domain() == 
accessee_ik->protection_domain() &&
       accessor_ik->class_loader() == accessee_ik->class_loader();
   } else {
     return false;
   }
}

>
> Thanks,
> David
>
> On 29/05/2013 11:25 PM, harold seigel wrote:
>> Hi,
>>
>> Please review this fix for bug 8015385.
>>
>> This fix removes the use of the RelaxAccessControlCheck option for
>> bytecode versions >= 52 and so requires the stricter access checking
>> that previously could be avoided by using this option. Note that the
>> original purpose of this option was for use with pre-Java_1_5 bytecodes
>> that did not contain the necessary accessors needed to properly access
>> private fields.
>>
>> This change was tested with JCK lang and vm tests, UTE vm.quick.testlist
>> and vm.mlvm.testlist tests, JPRT, and jtreg tests. Tests were run on
>> Linux and Solaris.
>>
>> Open webrev at http://cr.openjdk.java.net/~hseigel/bug_8015385/
>> <http://cr.openjdk.java.net/%7Ehseigel/bug_8015385/>
>>
>> Bug link at http://bugs.sun.com/view_bug.do?bug_id=8015385
>>
>> Thanks, Harold

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20130531/862758d9/attachment.html