8257596: Clarify trusted final fields for record classes

[mlchung]

This is a follow-up on JDK-8255342 that removes non-specified JVM checks on classes with RecordComponents attributes.

See the discussion at https://mail.openjdk.java.net/pipermail/amber-spec-experts/2020-December/002670.html

That fixes trusting final fields of records to align with the JLS definition of a record class. InstanceKlass::is_record is fixed to check a record class must be final and a direct subclass of java.lang.Record with the presence of the Record attribute. There is no change to JVM class file validation. I also 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 Record attribute
• JVM_GetRecordComponents returns an RecordComponents array if Record 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 Record attribute. So JVM_GetRecordComponents returns the content of the classfile.

tier1-tier3 and jck-runtime:vm and jck-runtime:lang tests all passed.

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed

Issue

• JDK-8257596: Clarify trusted final fields for record classes

Reviewers

• Harold Seigel (@hseigel - Reviewer) ⚠️ Review applies to 4ebee9b
• Chris Hegarty (@ChrisHegarty - Reviewer) ⚠️ Review applies to 7deefcc
• Paul Sandoz (@PaulSandoz - Reviewer) ⚠️ Review applies to 7deefcc

Download

$ git fetch https://git.openjdk.java.net/jdk16 pull/14/head:pull/14
$ git checkout pull/14

[bridgekeeper]

👋 Welcome back mchung! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@mlchung The following labels will be automatically applied to this pull request:

• core-libs
• hotspot

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 02: Full - Incremental (f0df17a)
• 01: Full - Incremental (4ebee9b)
• 00: Full (7deefcc)

[hseigel]

Hi Mandy,
The changes look good. Thanks for running the tests and changing the attribute name in the comments.
Could you also change the two comments that you added to jvm.c with these comments:

Comment for JVM_IsRecord():
// A class is a record if and only if it is final and a direct subclass of
// java.lang.Record and has a Record attribute; otherwise, it is not a record.

Comment for JVM_GetRecordComponents():
// 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.

Thanks, Harold

[openjdk]

@mlchung This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8257596: Clarify trusted final fields for record classes

Reviewed-by: hseigel, chegar, psandoz

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 6 new commits pushed to the master branch:

• b7ac32d: 8257598: Clarify what component values are used in Record::equals
• a280182: 8258060: Update @jls tags for renamed/renumbered sections
• bacf22b: 8256641: CDS VM operations do not lock the heap
• 58dca92: 8257910: [JVMCI] Set exception_seen accordingly in the runtime.
• e90d0d1: 8258065: ProblemList JfrGTestAdaptiveSampling
• fa77008: 8258015: [JVMCI] JVMCI_lock shouldn't be held while initializing box classes

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[hseigel]

Looks Good!

[PaulSandoz]

IIUC, in summary this correctly aligns the runtime definition of a record with the language, which therefore and appropriately constrains the trusting of non-static final fields to only fields of records, and not more broadly to record-like classes.

[mlchung]

/integrate

[openjdk]

@mlchung Since your change was applied there have been 7 commits pushed to the master branch:

• b1afed7: 8257919: [JVMCI] profiling info didn't change after reprofile
• b7ac32d: 8257598: Clarify what component values are used in Record::equals
• a280182: 8258060: Update @jls tags for renamed/renumbered sections
• bacf22b: 8256641: CDS VM operations do not lock the heap
• 58dca92: 8257910: [JVMCI] Set exception_seen accordingly in the runtime.
• e90d0d1: 8258065: ProblemList JfrGTestAdaptiveSampling
• fa77008: 8258015: [JVMCI] JVMCI_lock shouldn't be held while initializing box classes

Your commit was automatically rebased without conflicts.

Pushed as commit 2001da3.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.