[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Integrated: 8267555: Fix class file version during redefinition after 8238048
From:       Volker Simonis <simonis () openjdk ! java ! net>
Date:       2021-05-28 8:36:10
Message-ID: EdqsL71aym5FO6eB5lO9UOIBRNg5MPM6QcUdd8yWASE=.8c24c252-8e22-47eb-bdc3-872cab2cba3f () github ! com
[Download RAW message or body]

On Fri, 21 May 2021 19:20:29 GMT, Volker Simonis <simonis@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 <simonis@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


[prev in list] [next in list] [prev in thread] [next in thread]