[9] RFR (S): 8157181: Compilers accept modification of final fields outside initializer methods

[David Holmes]

<adding back hotspot-compiler-dev at openjdk.java.net>

Hi Coleen,

If I may ...

On 11/06/2016 5:43 AM, Coleen Phillimore wrote:
>
> Hi,
>
> I'm late to the thread, which is good because it's better that this
> check is done in the rewriter, than in InstanceKlass.
>
> But I'm fuzzy on why this wasn't checked in the verifier, and what the
> purpose of the has_initialized_final_update (which means a non <init>
> method has updated a final field, which is a verify error, right?

Can't comment on the verifier part.

The check is only currently enforced for classfile versions for JDK9+

> Also, have you run the JCK tests?
>
> Why have a global flag if it's an error?

We are enforcing a rule that has not been enforced before. To deal with 
compatibility issues we have provided, as is customary for non-security 
issues, a flag that allows the enforcement to be turned off, if it 
causes a problem. (Though that is more likely needed for JDK 7/8 
compatibility than for 9).

David

> Thanks,
> Coleen
>
>
>
> On 6/10/16 1:40 PM, Zoltán Majó wrote:
>> Hi Vladimir,
>>
>>
>> On 06/10/2016 06:05 PM, Vladimir Ivanov wrote:
>>>
>>>> Please note an additional small change in rewriter.cpp:
>>>>
>>>> +if (!reverse) {
>>>> // Check if any final field of the class given as parameter is modified
>>>> // outside of initializer methods of the class. Fields that are
>>>> modified
>>>>    ...
>>>> +}
>>>>
>>>> It's sufficient to check fields during rewriting (i.e., we're coming
>>>> from Rewriter::rewrite_bytecodes()). We do not need to do the checks if
>>>> the class has already been rewritten but we're reversing changes due to
>>>> some failure that appeared during rewriting (in that case scan_method()
>>>> is called from Rewriter::restore_bytecodes()).
>>>
>>> Ok.
>>>
>>>> Here is the updated webrev:
>>>> http://cr.openjdk.java.net/~zmajo/8157181/webrev.11/
>>>
>>> Looks good.
>>>
>>> src/share/vm/runtime/globals.hpp
>>>
>>> +          "for classfile version >= 51")
>>>
>>> s/51/53/ (no need to send new webrev).
>>
>> OK, I've corrected the code in-situ (in webrev.11). I also have the
>> JPRT results now, all tests pass.
>>
>> Thank you!
>>
>> Best regards,
>>
>>
>> Zoltan
>>
>>
>>
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>
>