Skip to content

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
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