620173 - crash [@ Snapshot | GetIterator] because null check is inverted

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[timeless]

619 GetIterator(JSContext *cx, JSObject *obj, uintN flags, Value *vp)

713     if (flags & JSITER_FOREACH) {
715         if (JS_LIKELY(obj != NULL) && !Snapshot<ValueEnumeration>(cx, obj, flags, &vals))
716             return false;

720     } else {
722         if (JS_LIKELY(obj != NULL) && !Snapshot<KeyEnumeration>(cx, obj, flags, &keys))
723             return false;

Comment 1

[timeless]

Attachment: flip check

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 498586 [details] [diff] [review]
flip check

Review of attachment 498586 [details] [diff] [review]:

Is this really a bug? The code says:

>    if (JS_LIKELY(obj != NULL) && !Snapshot<ValueEnumeration>(cx, obj, flags, &vals))
>        return false;

which is a cryptic way of saying:

>    if (JS_LIKELY(obj != NULL)) {
>        if (!Snapshot<ValueEnumeration>(cx, obj, flags, &vals))
>            return false;
>    }

If obj is null, this is supposed to succeed (see the comment above this chunk of code: /* NB: for (var p in null) succeeds by iterating over no properties.*/). I think we're supposed to call VectorToValueIterator, passing NULL as the second parameter, to create an iterator that produces no property names.

If this is really a crash, supply the test case and I'll re-review (with less latency this time).

Comment 3

[timeless]

i don't want to think about this anymore.