[lworld] RFR: 8253416: [lworld] C1: incorrect nestmate access to flattened field if nest-host is not loaded
thartmann at openjdk.java.net
Fri Sep 25 13:28:13 UTC 2020
On Fri, 25 Sep 2020 13:09:20 GMT, Frederic Parain <fparain 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.
> 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.
Thanks a lot for the review!
More information about the valhalla-dev