-
Notifications
You must be signed in to change notification settings - Fork 150
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
8250825: C2 crashes with assert(field != __null) failed: missing field #552
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back syan! A progress list of the required criteria for merging this PR into |
❗ This change is not yet ready to be integrated. |
This backport pull request has now been updated with issue from the original commit. |
Webrevs
|
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.
The new test fails GHA.
Yes, that's the reason this PR still in |
|
After the additional test finish, I will make approval request. |
The test fails has been fixed now. |
The GHA report several failures:
|
/approval request Backport JDK-8250825, New test fails without the patch, passes with it. All the jtreg tests on linux x86_64 and aarch64 shows no new failure. |
@sendaoYan |
All these mac issues should be unrelated, see: #544 |
|
Thanks for the review. |
I have classified problems on MacOS in this coment: #544 (comment) |
@sendaoYan This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Hi, can anyone take look this backport to jdk8u. |
Thanks for the review. |
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.
The change to the test for Unsafe
looks fine and matches other 8u tests that use Unsafe
.
I'm not sure what patch you're backporting from, but the 11u version is already adjusted for the absence of JDK-8230505. If you fix up the indentation to match 11u, this should be good to go.
/approval request Fixes the corner case in C2. Patch does not apply cleanly due to different context. New test fails without the patch, passes with it. jtreg tier{1,2,3} also passes. |
@sendaoYan |
@sendaoYan This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
/approval request Fixes the corner case in C2. Patch does not apply cleanly due to different context. New test fails without the patch, passes with it. jtreg tier{1,2,3,etc.} also passes on linux x86_64 and linux aarch64. |
@sendaoYan |
@sendaoYan This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
/open |
@sendaoYan This pull request is already open |
@sendaoYan This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
/open |
@sendaoYan This pull request is already open |
Hi all,
This is backport of JDK-8250825. It's prefixed PR for JDK-8255466 backport.
New test fails without the patch, passes with it.
There are two parts make this backport not clean:
import jdk.internal.misc.Unsafe
should instead ofimport sun.misc.Unsafe
in jdk8u.Additional testing:
Progress
Issue
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/552/head:pull/552
$ git checkout pull/552
Update a local copy of the PR:
$ git checkout pull/552
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/552/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 552
View PR using the GUI difftool:
$ git pr show -t 552
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/552.diff
Using Webrev
Link to Webrev Comment