RFR (S): 8157181: Compilers accept modification of final fields outside initializer methods
vladimir.x.ivanov at oracle.com
Wed May 18 13:47:39 UTC 2016
>> Otherwise, it's a bug in C1 and patching is broken for unlinked field
> I did what you suggested and patching works fine. The putstatic (and the
> corresponding StoreField from above) is treated as a unlinked (as
> However, linking does not generate the appropriate exception. The reason
> is that LinkResolver::resolve_field() checks only if the holder of the
> field is the same as the holder of the method, but not if the method is
> an initializer. So execution continues to bytecode @29 (that is constant
> folded by the compiler) and the original failure still appears.
It's contradicts JVMS, which requires an IAE to be thrown.
I think major release is the right place to fix it.
>> I vaguely remember JVMS allowed changing final fields in the same
>> class from any code, but then that part was tightened (in 5?).
>> I'd prefer to avoid discrepancy in behavior between different
>> execution modes. Either disable problematic constant folding in
>> compilers for now or fix compilers and interpreter at once.
> I assume by "problematic constant folding" you mean "constant fold final
> field loads (from the same class) in initializers" from above.
Yes, because JVMS allows multiple updates of final fields from
initializers and compilers should respect that.
> My impression is that the compilers can be safe only by completely
> disabling folding of final fields. The reason is that, currently, any
> method of a class 'C' declaring a static final field 'f' can change the
> field 'f' after class 'C' has been initialized. So even if we are
> compiling a method 'm' of a class 'X' that is accessing field 'C::f',
> 'f' can change after 'm' has been compiled.
> So folding final fields must be disabled for all compilations. Or am I
> missing something?
Yes, you are right. The only way to completely avoid discrepancy is to
disable constant folding of final fields.
So, I'm in favor of a generic fix to align the behavior with JVMS.
> I agree with you, alternatively we can propose a more generic fix (fix
> the interpreter and the compilers as well). The fix would most likely
> affect field resolution in LinkResolver::resolve_field() around here:
> Changing resolve_field() to throw an IllegalAccessError for
> inappropriate field access will propagate the exception to the
> interpreter. Changing ciField::will_link() will most likely kill (some)
> compilations (if, e.g., fields are linked through ciField::will_link()).
> I'd prefer to take the second option (changing field resolution), but
> there might be some disadvantages related to option I might be overseeing.
> Thank you!
> Best regards,
>>> More details about the failure are here .
>>> 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:
>> Bailing out the whole compilation in C2 is even less appropriate. It
>> should generate an uncommon trap for such accesses instead (see
>> Best regards,
>> Vladimir Ivanov
More information about the hotspot-compiler-dev