Fix --max_old_space_size=4096 integer overflow.

Fix --max_old_space_size=4096 integer overflow.

R=yangguo@chromium.org

Committed: https://crrev.com/6253aa8908c55535473ddbe1db8e5a6f5e559b73
Cr-Commit-Position: refs/heads/master@{#26371}

[bnoordhuis, 2015-01-29 16:50:18]

ben@strongloop.com changed reviewers:
+ yangguo@chromium.org

[bnoordhuis, 2015-01-29 16:50:19]

See https://github.com/iojs/io.js/issues/648 for background.  Large
--max_old_space_size values wrap around when multiplied by MB.

[Yang, 2015-01-30 10:14:59]

yangguo@chromium.org changed reviewers:
+ svenpanne@chromium.org

[Yang, 2015-01-30 10:14:59]

Not sure about this CL. The flags by themselves are fine. The problem only
happens if you multiply the flag values after reading them out when we
initialize the heap. Can't we simply cast up before multiplying?

[bnoordhuis, 2015-01-30 11:06:26]

On 2015/01/30 10:14:59, Yang wrote:
> Not sure about this CL. The flags by themselves are fine. The problem only
> happens if you multiply the flag values after reading them out when we
> initialize the heap. Can't we simply cast up before multiplying?

Certainly, but then people need to remember to widen the type every time they
use it in a calculation.  Changing the type avoids a whole class of future bugs.

My initial take changed all int flags to int64_t.  I still think that's a good
idea but I figured it was perhaps too audacious.

[Yang, 2015-01-30 13:23:18]

On 2015/01/30 11:06:26, bnoordhuis wrote:
> On 2015/01/30 10:14:59, Yang wrote:
> > Not sure about this CL. The flags by themselves are fine. The problem only
> > happens if you multiply the flag values after reading them out when we
> > initialize the heap. Can't we simply cast up before multiplying?
> 
> Certainly, but then people need to remember to widen the type every time they
> use it in a calculation.  Changing the type avoids a whole class of future
bugs.
> 
> My initial take changed all int flags to int64_t.  I still think that's a good
> idea but I figured it was perhaps too audacious.

alright lgtm.

[bnoordhuis, 2015-01-30 16:03:21]

The CQ bit was checked by ben@strongloop.com

[commit-bot: I haz the power, 2015-01-30 16:06:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-01-30 16:06:24]

Commit queue rejected this change because it did not recognize the base URL.
Please commit your change manually.

[Yang, 2015-01-31 20:45:55]

Committed patchset #1 (id:1) manually as
6253aa8908c55535473ddbe1db8e5a6f5e559b73 (presubmit successful).

[commit-bot: I haz the power, 2015-01-31 20:45:57]

Patchset 1 (id:??) landed as
https://crrev.com/6253aa8908c55535473ddbe1db8e5a6f5e559b73
Cr-Commit-Position: refs/heads/master@{#26371}

[Yang, 2015-01-31 21:44:56]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/875053005/ by yangguo@chromium.org.

The reason for reverting is: Win64 compile failure..

[Michael Achenbach, 2015-02-01 08:04:50]

How was this CL uploaded? It is missing the target ref property. Maybe the
gclient version was out-of-date?

[Yang, 2015-02-01 08:12:21]

On 2015/02/01 08:04:50, Michael Achenbach wrote:
> How was this CL uploaded? It is missing the target ref property. Maybe the
> gclient version was out-of-date?

Ben might be using an old git checkout. CQ didn't work, and since it's a rather
small change, I thought I could skip CQ and land manually. Turned out I was
wrong. The automatically created revert also had the wrong base URL, so I had to
do it manually again.

Ben, could you base your v8 patches on the new git workflow?

[bnoordhuis, 2015-02-01 14:11:47]

On 2015/02/01 08:12:21, Yang wrote:
> Ben, could you base your v8 patches on the new git workflow?

Sorry about that.  Yes, I'll start doing that from now on.