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

[Coleen Phillimore]

One more question.

On 6/11/16 6:13 AM, Zoltán Majó wrote:
> Hi David,
>
>
> On 06/11/2016 02:26 AM, David Holmes wrote:
>> <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).
>
> thank you for clarifying that!

Can we time-bomb this option, so we can remove it in 10?

thanks,
Coleen

>
> Best regards,
>
>
> Zoltan
>
>>
>> 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
>>>>
>>>
>