On Fri, 21 May 2021 19:20:29 GMT, Volker Simonis <simo...@openjdk.org> wrote:
> In jdk15, [JDK-8238048](https://bugs.openjdk.java.net/browse/JDK-8238048) > moved the class file versions from the `InstanceKlass` into the > `ConstantPool` and introduced a new method `ConstantPool::copy_fields(const > ConstantPool* orig)` which copies not only the `source_file_name_index`, > `generic_signature_index` and `has_dynamic_constant` from `orig` to the > current `ConstantPool` object, but also the minor and major class file > version. > > This new `copy_fields()` method is now called during class redefinition (in > `VM_RedefineClasses::merge_cp_and_rewrite()`) at places where previously only > the `has_dynamic_constant` attribute was copied from the old to the new > version of the class. If the new version of the class has a different class > file version than the previous one, this different class file version will > now be overwritten with the class file version of the previous, original > class. > > In `VM_RedefineClasses::load_new_class_versions()`, after > `VM_RedefineClasses::merge_cp_and_rewrite()` has completed, we do another > verification step to check the results of constant pool merging (in jdk15 > this was controlled by `VerifyMergedCPBytecodes` which was on by default, in > jdk16 and later this extra verification step only happens in debug builds). > If the new class instance uses features which are not available for the class > version of the previous class, this verification step will fail. > > The solution is simple. Don't overwrite the class file version of the new > class any more. This also requires reintroducing the update of the class file > version for the newly redefined class in > `VM_RedefineClasses::redefine_single_class()` like this was done before > [JDK-8238048](https://bugs.openjdk.java.net/browse/JDK-8238048). > > I'm just not sure about the additional new field updates for > `source_file_name_index` and `generic_signature_index` in > `ConstantPool::copy_fields()` which were not done before > [JDK-8238048](https://bugs.openjdk.java.net/browse/JDK-8238048). Do we want > to preserve these attributes from the original class and write them into the > new redefined version? If yes, the new code would be correct and we could > remove the following code from `VM_RedefineClasses::redefine_single_class()` > because that was already done in `ConstantPool::copy_fields()`: > > // Copy the "source file name" attribute from new class version > the_class->set_source_file_name_index( > scratch_class->source_file_name_index()); > > Otherwise the new would be wrong in the same sense like it was wrong for the > class file versions. The differences of between the class file versions and > `source_file_name_index`/`generic_signature_index` is that the former > attributes are mandatory and therefor always available in the new class > version while the latter ones are optional. So maybe we should only copy them > from the original to the new class if they are not present there already? It > currently seems like there's no optimal solution for these attributes? Hi Coleen, thanks for reviewing my change. I've updated the code as suggested and moved the test to the new location. Thanks for pointing me to `RedefineClassHelper`. That considerably simplifies the test. Please let me know what you think? Best regards, Volker ------------- PR: https://git.openjdk.java.net/jdk/pull/4149
