-
Notifications
You must be signed in to change notification settings - Fork 5.8k
8307521: Introduce check_oop infrastructure to check oops in the oop class #13825
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
/label add hotspot |
👋 Welcome back stefank! A progress list of the required criteria for merging this PR into |
@stefank |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
@stefank 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:
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 11 new commits pushed to the
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine.
Thanks for reviewing! |
Going to push as commit 959e62c.
Your commit was automatically rebased without conflicts. |
I'd like to add some extra verification to our C++ usages of oops. The intention is to quickly find when we are passing around an oop that wasn't fetched via a required load barrier. We have found this kind of verification crucial when developing Generational ZGC.
My proposal is to hook into the CHECK_UNHANDLED_OOPS code, which is only compiled when building fastdebug builds. In release and slowdebug builds,
oops
are simpleoopDesc*
, but with CHECK_UNHANDLED_OOPS oop is a class where we can easily hook in verification code.The actual verification code is not included in the patch, but the required infrastructure is. Then when we deliver Generational ZGC, it will install a verification function pointer during initialization. See: https://github.com/openjdk/zgc/blob/349cf9ae38664991879402a90c5e23e291f1c1c3/src/hotspot/share/gc/z/zAddress.cpp#L92
We've separated out this code from the larger Generational ZGC PR, so that it can get a proper review without being hidden together with all other changes.
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13825/head:pull/13825
$ git checkout pull/13825
Update a local copy of the PR:
$ git checkout pull/13825
$ git pull https://git.openjdk.org/jdk.git pull/13825/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13825
View PR using the GUI difftool:
$ git pr show -t 13825
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13825.diff
Webrev
Link to Webrev Comment