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

[Lindenmaier, Goetz]

Hi, 

Vladimir, thanks for sponsoring it!
And all the people handling JBS, thanks too!

Best regards,
  Goetz.


-----Original Message-----
From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Vladimir Ivanov
Sent: Mittwoch, 23. Juli 2014 17:39
To: hotspot-dev at openjdk.java.net
Subject: Re: RFR(S): 8050978: Fix bad field access check in C1 and C2

Looks good. I'll sponsor the fix.

Best regards,
Vladimir Ivanov

On 7/22/14, 2:42 PM, Lindenmaier, Goetz wrote:
> Hi,
>
> I please need a second reviewer for this change, as well as a sponsor.
> It also needs to go to 8u20.
>
> Thanks and best regards,
>    Goetz.
>
> -----Original Message-----
> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
> Sent: Freitag, 18. Juli 2014 16:59
> To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net
> Subject: Re: RFR(S): 8050978: Fix bad field access check in C1 and C2
>
> 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
>>>
>>>