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

deps: upgrade v8 to 4.1.0.25 #1224

Closed
wants to merge 1 commit into from
Closed

Conversation

jbergstroem
Copy link
Member

4.1.0.21..4.1.0.25 contains two of the patches we were floating (solaris build fix and heap size). PTAL since this is the "first time" I'm upgrading v8.

R=@indutny, @bnoordhuis

@indutny
Copy link
Member

indutny commented Mar 21, 2015

LGTM. What does CI say?

@jbergstroem
Copy link
Member Author

@jbergstroem jbergstroem added the v8 engine Issues and PRs related to the V8 dependency. label Mar 21, 2015
@@ -45,6 +45,7 @@ Jan de Mooij <jandemooij@gmail.com>
Jay Freeman <saurik@saurik.com>
James Pike <g00gle@chilon.net>
Joel Stanley <joel.stan@gmail.com>
Johan Bergström <johan@bergstroem.nu>
Copy link
Member

Choose a reason for hiding this comment

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

🎉 congrats @jbergstroem!

@rvagg
Copy link
Member

rvagg commented Mar 21, 2015

LGTM but I'd love to know what this includes other than the Solaris stuff, for instance what's the story with the changes in hydrogen-check-elimination.cc? Is there anything we can say about this in "notable items" in a release other than Solaris?

@jbergstroem
Copy link
Member Author

@rvagg here's all commits. Nothing overly exciting, but at least makes us carry two less commits: v8/v8@4.1.0.21...4.1.0.25

@bnoordhuis
Copy link
Member

LGTM but did you fold a558cd0 into the upgrade commit? I suppose that could cause some confusion for people that don't know we're floating that patch.

what's the story with the changes in hydrogen-check-elimination.cc?

It fixes a type transition bug that results in a crash when the function gets optimized. It's possibly exploitable although it's probably very hard to exploit remotely.

@jbergstroem
Copy link
Member Author

@bnoordhuis I based the patch on the diff between 21..25 which didn't touch NamedPropertyHandlerConfiguration. Should I make some notes about this in the commit message?

@bnoordhuis
Copy link
Member

Should I make some notes about this in the commit message?

No need, I think. I was just pleasantly surprised there was less churn than I expected. :-)

jbergstroem added a commit that referenced this pull request Mar 23, 2015
PR-URL: #1224
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@jbergstroem
Copy link
Member Author

Merged in fe4434b. Thanks for helping out with the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants