On Tue, 8 Dec 2020 22:52:37 GMT, Mandy Chung <mch...@openjdk.org> wrote:
> This is a follow-up on JDK-8255342 that removes non-specified JVM checks on > classes with `RecordComponents` attributes. That introduces a regression in > `InstanceKlass::is_record` that returns true on a non-record class which has > `RecordComponents` attribute present. This causes unexpected semantics in > `JVM_IsRecord` and `JVM_GetRecordComponents` and also a regression to trust > final fields for non-record classes. > > I propose to change `InstanceKlass::is_record` to match the JLS semantic of a > record class, i.e. final direct subclass of `java.lang.Record` with the > presence of `RecordComponents` attribute. There is no change to JVM class > file validation. Also I propose clearly define: > - `JVM_IsRecord` returns true if the given class is a record i.e. final > and direct subclass of `java.lang.Record` with `RecordComponents` attribute > - `JVM_GetRecordComponents` returns an `RecordComponents` array if > `RecordComponents` attribute is present; otherwise, returns NULL. This does > not check if it's a record class or not. So it may return non-null on a > non-record class if it has `RecordComponents` attribute. So > `JVM_GetRecordComponents` returns the content of the classfile. Hi Mandy, Could you replace the comment starting at line 1854 in jvm.cpp with: // A class is a record if and only if it is final and a direct subclass // of java.lang.Record and the presence of `Record` attribute; // otherwise, it is not a record. Also, replace the comment starting at line 1872 in jvm.cpp with: // Returns an array containing the components of the Record attribute, // or NULL if the attribute is not present. // // Note that this function returns the components of the Record // attribute even if the class is not a Record. Also, please change the name of the attribute in the comments added to Class.java from RecordComponent to Record. Thanks, Harold ------------- PR: https://git.openjdk.java.net/jdk/pull/1706