Don't NULL-check GlobalHandle::Node::object_

Don't NULL-check GlobalHandle::Node::object_

If it's Smi::FromInt(0), the NULL check would trigger. Instead, use the
handle-zap value to mean "not set".

BUG=v8:3647, chromium:580651
R=vogelheim@chromium.org
LOG=y

Committed: https://crrev.com/85f32f1fc8b59a1b91404b0515a935df1f86b7f9
Cr-Commit-Position: refs/heads/master@{#33492}

[vogelheim, 2016-01-25 12:00:19]

lgtm

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc#newco...
src/global-handles.cc:81: object_ =
reinterpret_cast<Object*>(kGlobalHandleZapValue);
nitpick: Wouldn't it be better for debugging to distinguish the "not
initialized" guard value from the "already dead" value? I.e., introduce a
kGlobalHandleUninitialized?

(Not sure if worth the hassle, though.)

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc#newco...
src/global-handles.cc:93: object_ = object;
Maybe DCHECK(object != nullptr)?

[jochen (gone - plz use gerrit), 2016-01-25 13:05:49]

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc
File src/global-handles.cc (right):

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc#newco...
src/global-handles.cc:81: object_ =
reinterpret_cast<Object*>(kGlobalHandleZapValue);
On 2016/01/25 at 12:00:19, vogelheim wrote:
> nitpick: Wouldn't it be better for debugging to distinguish the "not
initialized" guard value from the "already dead" value? I.e., introduce a
kGlobalHandleUninitialized?
> 
> (Not sure if worth the hassle, though.)

since we recycle handles, they're expected to constantly go back and forth
between zapped and valid

https://codereview.chromium.org/1628173002/diff/1/src/global-handles.cc#newco...
src/global-handles.cc:93: object_ = object;
On 2016/01/25 at 12:00:19, vogelheim wrote:
> Maybe DCHECK(object != nullptr)?

nullptr is valid here (since that's Smi::FromInt(0))

the zap value is a heap ptr (ie the lowest bit is 1), so that should actually
never happen

[jochen (gone - plz use gerrit), 2016-01-25 13:07:48]

The CQ bit was checked by jochen@chromium.org

[commit-bot: I haz the power, 2016-01-25 13:07:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1628173002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1628173002/1

[commit-bot: I haz the power, 2016-01-25 13:09:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-01-25 13:10:13]

Description was changed from

==========
Don't NULL-check GlobalHandle::Node::object_

If it's Smi::FromInt(0), the NULL check would trigger. Instead, use the
handle-zap value to mean "not set".

BUG=v8:3647,chromium:580651
R=vogelheim@chromium.org
LOG=y
==========

to

==========
Don't NULL-check GlobalHandle::Node::object_

If it's Smi::FromInt(0), the NULL check would trigger. Instead, use the
handle-zap value to mean "not set".

BUG=v8:3647,chromium:580651
R=vogelheim@chromium.org
LOG=y

Committed: https://crrev.com/85f32f1fc8b59a1b91404b0515a935df1f86b7f9
Cr-Commit-Position: refs/heads/master@{#33492}
==========

[commit-bot: I haz the power, 2016-01-25 13:10:14]

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