[9] RFR (S): 8160527: Check for final instance field updates can be omitted

[Zoltán Majó]

Hi Coleen,


On 06/29/2016 11:02 PM, Coleen Phillimore wrote:
>
> Since this is a runtime change, can you check it into hs?

Yes, I'll do so once we're finished with the review process.

Thank you!

Best regards,


Zoltan

>
> Thanks,
> Coleen
>
> On 6/29/16 11:00 AM, Zoltán Majó wrote:
>> Hi,
>>
>>
>> please review the patch for 8160527.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8160527
>>
>> Problem: 8157181 added a check that verifies that final instance 
>> fields can be updated only by object initializer methods (as required 
>> by JVMS >=7) [1].
>>
>> Unfortunately, the newly added check can be circumvented due to 
>> constant pool caching: If the instance field update is executed in an 
>> <init> method first, the instruction updating the field is cached in 
>> the constant pool cache. Subsequent updates use the constant pool 
>> cache (and do not call into the VM where the check is executed). As a 
>> result, any method can update a final instance field if that field 
>> was first updated in an instance initializer (the method must be 
>> declared in the same class as the field, though).
>>
>> John's comment in the bug description provides more detailed 
>> information on how the above can happen.
>>
>>
>> Solution: Do not cache putfield instructions to final instance fields 
>> if the verifier has detected that the field is updated in other 
>> methods than <init> (i.e., has_initialized_field_update() returns 
>> true for the field). Avoiding caching results in the field access 
>> being re-resolved until the offending access is attempted; then an 
>> exception is thrown. Avoiding caching must be done very infrequently, 
>> as offending code is rare (e.g., such code cannot be produced by javac).
>>
>> I've also corrected a small mistake in the fix for 8157181 (the class 
>> name is printed in the error message instead of the method's name).
>>
>> Webrev:
>> http://cr.openjdk.java.net/~zmajo/8160527/webrev.00/
>>
>> Testing:
>> - JPRT (testset hotspot, incl. newly added test);
>> - verified that the newly added test triggers the problem with an 
>> unmodified VM.
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>
>> [1] 
>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/rev/c558d46c1af2#l11.76
>>
>