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? This pull request has now been integrated. Changeset: 1d2c7ac3 Author: Volker Simonis <simo...@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/1d2c7ac3f7492b335757bf0fd3f6ca3941c5fc72 Stats: 208 lines in 5 files changed: 204 ins; 1 del; 3 mod 8267555: Fix class file version during redefinition after 8238048 Reviewed-by: coleenp, sspitsyn ------------- PR: https://git.openjdk.java.net/jdk/pull/4149
