Bug 149811 - Iterator loops over key twice after delete
Summary: Iterator loops over key twice after delete
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All Unspecified
: P2 Critical
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-10-05 10:29 PDT by Timon Orawski
Modified: 2015-10-12 20:24 PDT (History)
9 users (show)

See Also:


Attachments
reproducible test case (753 bytes, text/html)
2015-10-05 10:29 PDT, Timon Orawski
no flags Details
Patch (3.98 KB, patch)
2015-10-11 09:24 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (4.06 KB, patch)
2015-10-11 09:26 PDT, Yusuke Suzuki
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timon Orawski 2015-10-05 10:29:13 PDT
Created attachment 262446 [details]
reproducible test case

The following code iterates twice over the key "0" in the object cols - despite it only existing in the object once.

<html>
<head>
<script>
var f = function() {
    "use strict";
    var cols = {"col":{"title":"&nbsp;","type":"sys","events":[],"name":0,"id":0,"_i":0}};
    var len = 0;
    var remapcols = ['col'];
    for (var i = 0; i < remapcols.length; i++) {
        cols[cols[remapcols[i]].name] = cols[remapcols[i]];
        delete cols[remapcols[i]];
    }
    var count = 0;
    console.group("object:")
    console.log(cols);
    console.groupEnd();
    console.group("This group should only contain one line");
    for (var col2 in cols) { console.log("" + count++ +": Iterating over key: " + col2); }
    console.groupEnd();
};
f();</script>
</head>
<body>
Check console log, expected output is a single log entry "0: Iterating over key: 0"
</body>
</html>
Comment 1 Radar WebKit Bug Importer 2015-10-06 11:04:08 PDT
<rdar://problem/22993722>
Comment 2 Yusuke Suzuki 2015-10-11 06:11:54 PDT
Still investigating. But possible fix is,

setting indexedLength = 0; for non-generic JSPropertyNameEnumerator creation case.
Comment 3 Yusuke Suzuki 2015-10-11 09:24:34 PDT
Created attachment 262862 [details]
Patch
Comment 4 Yusuke Suzuki 2015-10-11 09:26:51 PDT
Created attachment 262863 [details]
Patch
Comment 5 Geoffrey Garen 2015-10-12 12:06:56 PDT
Comment on attachment 262863 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262863&action=review

r=me

> Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h:128
> +        // So disabling indexed property enumeration phase by setting |indexedLength| to 0.

disabling => disable
Comment 6 Yusuke Suzuki 2015-10-12 20:23:23 PDT
Comment on attachment 262863 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=262863&action=review

>> Source/JavaScriptCore/runtime/JSPropertyNameEnumerator.h:128
>> +        // So disabling indexed property enumeration phase by setting |indexedLength| to 0.
> 
> disabling => disable

Thanks. Fixed.
Comment 7 Yusuke Suzuki 2015-10-12 20:24:39 PDT
Committed r190923: <http://trac.webkit.org/changeset/190923>