RFR(S): 8050978: Fix bad field access check in C1 and C2

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 7/18/14 12:15 AM, Lindenmaier, Goetz wrote:
> Hi Vladimir,
>
> we updated the changeset with the new comment.
> http://cr.openjdk.java.net/~goetz/webrevs/8050978-fieldCheck/webrev-01/
>
> Best regards,
>    Goetz.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Donnerstag, 17. Juli 2014 18:02
> To: hotspot-dev at openjdk.java.net
> Subject: Re: RFR(S): 8050978: Fix bad field access check in C1 and C2
>
> Please, don't put next part of comment into sources:
>
> +  // This will make the jck8 test
> +  // vm/constantpool/accessControl/accessControl004/accessControl00402m3/accessControl00402m3.html
> +  // pass with -Xbatch -Xcomp
>
> instead add something like "canonical_holder should not be use to check access becasue it can erroneously succeeds".
>
> Thanks,
> Vladimir
>
> On 7/17/14 3:47 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> This fixes an error doing field access checks in C1 and C2.
>> Please review and test the change.  We please need a sponsor.
>> http://cr.openjdk.java.net/~goetz/webrevs/8050978-fieldCheck/webrev-01/
>>
>> This should be included in 8u20, too.
>>
>> JCK8 test vm/constantpool/accessControl/accessControl004/accessControl00402m3/accessControl00402m3.html fails with -Xbatch -Xcomp due to bad field access check in C1 and C2
>>
>> Precondition:
>> -------------
>>
>> Consider the following class hierarchy:
>>
>>       A
>>     / \
>>    B1 B2
>>
>> A declares a field "aa" which both B1 and B2 inherit.
>>
>> Despite aa is declared in a super class of B1, methods in B1 might not access the field aa of an object of class B2:
>>
>> class B1 extends A {
>>     m(B2 b2) {
>>       ...
>>       x = b2.aa; // !!! Access not allowed
>>     }
>> }
>>
>> This is checked by the test mentioned above.
>>
>> Problem:
>> --------
>>
>> ciField::will_link() used by C1 and C2 does the access check using the canonical_holder (which is A in this case) and thus the access erroneously succeeds.
>>
>> Fix:
>> ----
>>
>> In ciField::ciField(), just before the canonical holder is stored into the _holder variable (and which is used by ciField::will_link()) perform an additional access check with the holder declared in the class file. If this check fails, store the declared holder instead and ciField::will_link() will bail out compilation for this field later on. Then, the interpreter will throw an PrivilegedAccessException at runtime.
>>
>> Ways to reproduce:
>> ------------------
>>
>> Run the above JCK test with
>>
>> C2 only: -XX:-TieredCompilation -Xbatch -Xcomp
>>
>> or
>>
>> with C1: -Xbatch -Xcomp -XX:-Inline
>>
>> Best regards,
>>     Andreas and Goetz
>>
>>