[lworld] RFR: 8253416: [lworld] C1: incorrect nestmate access to flattened field if nest-host is not loaded

[Frederic Parain]

On Thu, 24 Sep 2020 14:08:11 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

> C1 incorrectly assumes that a nestmate field access is not flat if the nest-host is not loaded.
> 
> We are creating the ciField instance for a field in the nest-host via:
> `ciField::ciField -> Reflection::verify_member_access -> InstanceKlass::has_nestmate_access_to ->
> InstanceKlass::nest_host` Resolving the nest host then requires class loading which is not possible from the C1
> compiler thread and as a result we end up with partial field information (is_flattened is always false). The code in
> LIRGenerator::flattened_field_load_prolog then assumes that if !field->is_flattened(), the field is indeed not
> flattened. However, we can only rely on that information if !needs_patching.  I've completely re-wrote and simplified
> the complex logic in LIRGenerator::flattened_field_load_prolog and added asserts/checks to make sure we are not
> attempting to patch flattened inline type field accesses (that functionality could be added though but it's not easy).
> This patch also includes the following unrelated changes:
> - The "new" bytecode needs to throw an exception on inline klasses (the klass might be unloaded). The fix makes sure we
>   always take the slow path to "new_instance_no_inline" in that case.
> - The "defaultvalue" bytecode needs to throw an exception on non-inline klasses. We simply deoptimize in that case.
> - Load/stores from/to fields of an empty inline type should be removed.
> - Some refactoring and new asserts.

Tobias,

Thank you for the explanation on the needs_patching flag and the Withfield node.
So, did you keep the needs_patching argument in the copy_inline_content()  method only for verification purpose?

Overall, changes look good to me.

Fred

-------------

Marked as reviewed by fparain (Committer).

PR: https://git.openjdk.java.net/valhalla/pull/198