617593 - for-in statement visits deleted properties

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Wolfgang Rosenauer [:wolfiR]]

I have not analyzed this by myself but do forward it as good as I can.
Running Firefox 4.0b8pre (as of 20101207)
the following code returns 0, 1, and 2 and should only return 0

var x = ["a", "b", "c"];
for (var i in x) {
      console.log(i);
      for (var j in x) delete x[j];
}

Comment 1

[Boris Zbarsky [:bzbarsky]]

Yeah, this looks broken.  It's also a regression since 1.9.2.

Comment 2

[Andreas Gal :gal]

Whats console here?

Comment 3

[Wolfgang Rosenauer [:wolfiR]]

(In reply to comment #2)
> Whats console here?

AFAIK this is defined in firebug

Comment 4

[Andreas Gal :gal]

Its probably some sort of wrapper around some sort of XPCOM object? We need some details here. The test case above doesn't fail in general, only in this specific case.

Comment 5

[Boris Zbarsky [:bzbarsky]]

console is browser stuff.  If I replace |console.log| with |print| the above testcase fails for me in m-c js shell.

Comment 6

[Andreas Gal :gal]

Ok, nvm, my mistake. In that case we broke something.

Comment 7

[Boris Zbarsky [:bzbarsky]]

The testcase passes if |x| is not a dense array (e.g. if I use a vanilla Object, or add an expando to |x|).

Comment 8

[Andreas Gal :gal]

Breakpoint 1, js_SuppressDeletedProperty (cx=0x100813200, obj=0x100f0d068, id={asBits = 4299026016}) at ../jsiter.cpp:970
970	    return SuppressDeletedPropertyHelper(cx, obj, SingleIdPredicate(id));
(gdb) p/x id
$1 = {
  asBits = 0x1003dee60
}
(gdb) call js_DumpId(id)
jsid 0x1003dee60 = "0"
(gdb) 

We fail to integer-ize the id.

Comment 9

[Andreas Gal :gal]

Attachment: patch

Thanks for the report and the test case.

Comment 10

[Jeff Walden [:Waldo]]

Comment on attachment 496275 [details] [diff] [review]
patch

I am not entirely certain this is the right place to be doing this.  But it does pick up all the deletion-locations, so it is at least "good enough" for now.  We really should have two non-interconvertible types here, one for ids which are canonical (guaranteed to be integral if possible) and one for ids which may not be; if we had this now, this bug would have been a type error.  But that's for a future time (alas), not now.

Comment 11

[Andreas Gal :gal]

http://hg.mozilla.org/tracemonkey/rev/54b2d6f9948c

Comment 12

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/54b2d6f9948c

Comment 13

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it MUST be in the next beta (ie: trunk is in an unshippable state without this)