Fix a segment-fault caused by Value::IntegerValue

Fix a segment-fault caused by Value::IntegerValue

First discovered in a Node.js bug, cf. [1] and [2]. Handle zapping helps
exposing this bug in Release builds while hiding it in Debug. (Thank you
indutny<fedor@indutny.com>; and skomski<mail@skomski.com>; for the detail.)

The macros hide it but PREPARE_FOR_EXECUTION_PRIMITIVE is basically a
scoped operation. Outside the else block, num is no longer rooted.
(Thank you bnoordhuis<ben@strongloop.com>; for the detail.)

[1] https://github.com/nodejs/node/issues/2721
[2] https://github.com/nodejs/node/pull/2722

TEST=tools/run-tests.py --arch-and-mode=x64.release cctest/test-api/NoSegmentationFault

[P.S.V.R, 2015-09-08 09:11:07]

pmq2001@gmail.com changed reviewers:
+ machenbach@chromium.org

[P.S.V.R, 2015-09-08 09:12:56]

pmq2001@gmail.com changed reviewers:
+ ben@strongloop.com, fedor@indutny.com, mail@skomski.com

[P.S.V.R, 2015-09-08 09:12:57]

Fix a segment-fault caused by Value::IntegerValue

First discovered in a Node.js bug, cf. [1] and [2]. Handle zapping helps
exposing this bug in Release builds while hiding it in Debug. (Thank you
indutny<fedor@indutny.com>; and skomski<mail@skomski.com>; for the detail.)

The macros hide it but PREPARE_FOR_EXECUTION_PRIMITIVE is basically a
scoped operation. Outside the else block, num is no longer rooted.
(Thank you bnoordhuis<ben@strongloop.com>; for the detail.)

[1] https://github.com/nodejs/node/issues/2721
[2] https://github.com/nodejs/node/pull/2722

TEST=tools/run-tests.py --arch-and-mode=x64.release
cctest/test-api/NoSegmentationFault

[Michael Achenbach, 2015-09-09 10:43:51]

machenbach@chromium.org changed reviewers:
+ jochen@chromium.org

[Michael Achenbach, 2015-09-09 10:43:52]

Please find more appropriate owner-reviewers.

[jochen (gone - plz use gerrit), 2015-09-09 14:16:59]

thanks for the fix

https://codereview.chromium.org/1303093004/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1303093004/diff/1/src/api.cc#newcode3271
src/api.cc:3271: has_pending_exception =
can you pull the if() case in here instead of duplicating the returns?

something like

PREPARE_FOR...
if (obj->isNumber())
 num = obj;
else {
 has_pending = ...

[noordhuis, 2015-09-09 23:35:56]

info@bnoordhuis.nl changed reviewers:
+ info@bnoordhuis.nl

[noordhuis, 2015-09-09 23:35:56]

https://codereview.chromium.org/1303093004/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/1303093004/diff/1/src/api.cc#newcode3271
src/api.cc:3271: has_pending_exception =
On 2015/09/09 14:16:59, jochen wrote:
> can you pull the if() case in here instead of duplicating the returns?
> 
> something like
> 
> PREPARE_FOR...
> if (obj->isNumber())
>  num = obj;
> else {
>  has_pending = ...

That pessimizes the common case when the input is a SMI or double. 
PREPARE_FOR_EXECUTION_PRIMITIVE mutates a couple of per-isolate data structures
and is fairly heavy-weight.

[jochen (gone - plz use gerrit), 2015-09-16 07:24:55]

On 2015/09/09 at 23:35:56, info wrote:
> https://codereview.chromium.org/1303093004/diff/1/src/api.cc
> File src/api.cc (right):
> 
> https://codereview.chromium.org/1303093004/diff/1/src/api.cc#newcode3271
> src/api.cc:3271: has_pending_exception =
> On 2015/09/09 14:16:59, jochen wrote:
> > can you pull the if() case in here instead of duplicating the returns?
> > 
> > something like
> > 
> > PREPARE_FOR...
> > if (obj->isNumber())
> >  num = obj;
> > else {
> >  has_pending = ...
> 
> That pessimizes the common case when the input is a SMI or double. 
PREPARE_FOR_EXECUTION_PRIMITIVE mutates a couple of per-isolate data structures
and is fairly heavy-weight.

yeah, that's true

lgtm then