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

[Zoltán Majó]

Hi Vladimir,


thank you for the feedback!

On 05/18/2016 12:02 PM, Vladimir Ivanov wrote:
>> Webrev:
>> http://cr.openjdk.java.net/~zmajo/8157181/webrev.00/
>
> src/share/vm/c1/c1_GraphBuilder.cpp:
> +       if (field->is_constant() &&
> +           strcmp(method()->name()->as_quoted_ascii(), "<clinit>") != 
> 0) {
>
> +       if (field->is_final() &&
> +           strcmp(method()->name()->as_quoted_ascii(), "<init>") != 0) {
>
> Use Method::is_initializer()/is_static_initializer() instead.

OK.

The method is_static_initializer() is available only on Method but not 
on ciMethod. So I've "exported" it to ciMethod as well.

The method is_initializer() returns true for both <clinit> and <init>. 
So I've added a new method, is_instance_initializer(), that returns true 
only for <init>.

>
> Also, instead of bailing out the whole compilation, you can emit an 
> access as if the field access isn't linked yet (C1 supports that by 
> setting field offset to -1). The JVM should throw an appropriate 
> exception when trying to link the field.

I tried that (by setting the variable 'offset' in 
GraphBuilder::access_field to -1. But C1 will generate an access to an 
offset of -1; executing that code results in data corruption.

>
>> It seems to me that the interpeter is affected by a related problem
>> (i.e., the interpreter does not throw an IllegalAccessError if (2) does
>> not hold). I'd prefer to file a separate bug for that problem.
> I'm confused. How do you observe the difference between -Xint & -Xcomp 
> mode then?

Without the patch applied:
* With -Xint, the VM  does not throw an exception IllegalAccessError (as 
required by the VM spec). Instead, the VM executes the putstatic to the 
static final field as any other putstatic.
* -Xcomp generates optimized code that folds a constant and results in a 
NullPointerException being thrown.

The behavior with -Xint is the behavior expected and accepted by Jython. 
The problem was there since at least JDK 6, but was never observed "in 
the wild". The reason is that the method triggering the problem is not 
hot enough to be compiled (and so it is only interpreted).

More details about the failure are here [3].

With the patch applied, the program always completes as it is expected 
by Jython, no matter if we use -Xint, -Xcomp, or -Xmixed (because we 
always interpret methods non-conforming with the VM spec).

Here is the updated webrev:
http://cr.openjdk.java.net/~zmajo/8157181/webrev.01/

Thank you and best regards,


Zoltan

[3] 
https://bugs.openjdk.java.net/browse/JDK-8157181?focusedCommentId=13942824&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13942824

>
> Best regards,
> Vladimir Ivanov