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

[Coleen Phillimore]

Zoltan,

http://cr.openjdk.java.net/~zmajo/8160527/webrev.00/src/share/vm/interpreter/interpreterRuntime.cpp.udiff.html

Yes, this matches the general strategy.   You don't have to have this line

*+ (bytecode == Bytecodes::_putfield || bytecode == 
Bytecodes::_nofast_putfield);*


Because the test below is:

*!if (_(_is_put_&& !final_instance_update)_|| 
!info.access_flags().is_final()) {*


Otherwise, looks fine.  I'm sorry I didn't realize this in the initial 
code review.

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
>