Closed Bug 617593 Opened 9 years ago Closed 9 years ago

for-in statement visits deleted properties

Categories

(Core :: JavaScript Engine, defect)

All
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: wolfiR, Assigned: gal)

Details

(Keywords: regression, Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

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];
}
Yeah, this looks broken.  It's also a regression since 1.9.2.
blocking2.0: --- → ?
Keywords: regression
Whats console here?
(In reply to comment #2)
> Whats console here?

AFAIK this is defined in firebug
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.
console is browser stuff.  If I replace |console.log| with |print| the above testcase fails for me in m-c js shell.
Ok, nvm, my mistake. In that case we broke something.
Assignee: general → gal
blocking2.0: ? → beta9+
The testcase passes if |x| is not a dense array (e.g. if I use a vanilla Object, or add an expando to |x|).
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.
Attached patch patchSplinter Review
Thanks for the report and the test case.
Attachment #496275 - Flags: review?
Attachment #496275 - Flags: review? → review?(jwalden+bmo)
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.
Attachment #496275 - Flags: review?(jwalden+bmo) → review+
http://hg.mozilla.org/tracemonkey/rev/54b2d6f9948c
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/54b2d6f9948c
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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)
blocking2.0: beta9+ → betaN+
Keywords: regression
You need to log in before you can comment on or make changes to this bug.