Skip to content
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

8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large #350

Closed
wants to merge 1 commit into from

Conversation

jerboaa
Copy link
Contributor

@jerboaa jerboaa commented Aug 12, 2022

PR to backout the change of JDK-8279219. I'll change this to the proper backport title once JDK 19 backout is done.


Progress

  • Change must be properly reviewed (1 review required, with at least 1 Reviewer)
  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue

Issue

  • JDK-8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large

Reviewers

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u pull/350/head:pull/350
$ git checkout pull/350

Update a local copy of the PR:
$ git checkout pull/350
$ git pull https://git.openjdk.org/jdk17u pull/350/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 350

View PR using the GUI difftool:
$ git pr show -t 350

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u/pull/350.diff

@bridgekeeper
Copy link

bridgekeeper bot commented Aug 12, 2022

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

@openjdk openjdk bot added the rfr Pull request is ready for review label Aug 12, 2022
@mlbridge
Copy link

mlbridge bot commented Aug 12, 2022

Webrevs

@jerboaa jerboaa changed the title 8292260: Backout [REDO] C2 crash when allocating array of size too large 8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large Aug 12, 2022
@jerboaa jerboaa changed the title 8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large Backport 967a28c3d85fdde6d5eb48aa0edd8f7597772469 Aug 12, 2022
@openjdk openjdk bot changed the title Backport 967a28c3d85fdde6d5eb48aa0edd8f7597772469 8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large Aug 12, 2022
@openjdk
Copy link

openjdk bot commented Aug 12, 2022

This backport pull request has now been updated with issue from the original commit.

@openjdk openjdk bot added the backport label Aug 12, 2022
Copy link
Contributor

@RealCLanger RealCLanger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. JDK-8284369 is backouted by this as well but that was a specific fix for/after JDK-8279219 anyway.

@openjdk
Copy link

openjdk bot commented Aug 15, 2022

@jerboaa 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:

8292260: [BACKOUT] JDK-8279219: [REDO] C2 crash when allocating array of size too large

Reviewed-by: clanger

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 no new commits pushed to the jdk17.0.4.1 branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

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

@openjdk openjdk bot added the ready Pull request is ready to be integrated label Aug 15, 2022
@jerboaa
Copy link
Contributor Author

jerboaa commented Aug 16, 2022

/integrate

@openjdk
Copy link

openjdk bot commented Aug 16, 2022

Going to push as commit 02fa4be.

@openjdk openjdk bot added the integrated Pull request has been integrated label Aug 16, 2022
@openjdk openjdk bot closed this Aug 16, 2022
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Aug 16, 2022
@openjdk
Copy link

openjdk bot commented Aug 16, 2022

@jerboaa Pushed as commit 02fa4be.

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

@jerboaa
Copy link
Contributor Author

jerboaa commented Aug 16, 2022

@erikj79 @kevinrushforth Would either of you know why we didn't get a 17.0.4.1 backport bug for this? Similar issue to openjdk/jdk11u#49 (comment) ?

@erikj79
Copy link
Member

erikj79 commented Aug 16, 2022

(I don't currently have any github notifications turned on so tagging me doesn't help)
The reason this commit hasn't been notified to JBS is that Skara is currently choking on a tag that was updated/changed in this repo. A fix for that is currently on review in SKARA-1355. I will manually go and get this unstuck for now.

@RealCLanger
Copy link
Contributor

Thanks. I already thought, force-pushing the tag would cause trouble somehwere. But it seems like in this case it helps to get the tooling more robust 😄
One other thing I noted: The backport for 17.0.4.1 now mentions "Resolved in build: master". However, the b01 tag was already set, so it should be b01. Is that an expected outcome due to working on another branch in the repo? Or because 17.0.5 tags are already in place as well?

@jerboaa
Copy link
Contributor Author

jerboaa commented Aug 17, 2022

One other thing I noted: The backport for 17.0.4.1 now mentions "Resolved in build: master". However, the b01 tag was already set, so it should be b01.

Fixed this manually. Still interested to hear why that didn't work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport integrated Pull request has been integrated
3 participants