Closed
Bug 605013
Opened 14 years ago
Closed 14 years ago
"Assertion failure: JSID_IS_INT(id),"
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
blocking2.0 | --- | betaN+ |
People
(Reporter: gkw, Assigned: luke)
References
Details
(Keywords: assertion, regression, testcase, Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
2.41 KB,
patch
|
Waldo
:
review+
|
Details | Diff | Splinter Review |
x = /x/ Function("Object.defineProperty(x,new AttributeName,({e:true,enumerable:true}))")() { throw (Object.keys)(x, /x/) } asserts js debug shell on TM changeset 47a8311cf0bb without -m or -j at Assertion failure: JSID_IS_INT(id),
Reporter | ||
Comment 1•14 years ago
|
||
x = /x/ Function("\ Object.defineProperty(\ x,\ new AttributeName,\ ({e:true,enumerable:true})\ )\ ")() { throw (Object.keys)(x, /x/) } is a better testcase due to bugzilla wrapping long lines.
Reporter | ||
Comment 2•14 years ago
|
||
autoBisect shows this is probably related to the following changeset: The first bad revision is: changeset: 41832:b15fd8b568e4 user: Andreas Gal date: Fri May 07 17:52:52 2010 -0700 summary: fast object iteration (558754, r=brendan, CLOSED TREE).
Blocks: fastiterators
Updated•14 years ago
|
blocking2.0: ? → betaN+
Assignee | ||
Updated•14 years ago
|
Assignee: general → lw
Assignee | ||
Comment 3•14 years ago
|
||
Reduced test case: x = {}; Object.defineProperty(x, new AttributeName, { enumerable:true }); Object.keys(x); This is just obj_keys forgetting about object ids.
Assignee | ||
Comment 4•14 years ago
|
||
Waldo's suggestion is to ignore object ids. This makes sense since 15.2.4.14 Step 5 says "For each own enumerable property of O whose name **String** is P" and there is no stringification k of (new AttributeName) where o[k] will refer to the same property.
Attachment #489370 -
Flags: review?(jwalden+bmo)
Comment 5•14 years ago
|
||
Comment on attachment 489370 [details] [diff] [review] fix: ignore object ids >diff --git a/js/src/jsobj.cpp b/js/src/jsobj.cpp >--- a/js/src/jsobj.cpp >+++ b/js/src/jsobj.cpp >@@ -1784,27 +1784,28 @@ obj_keys(JSContext *cx, uintN argc, Valu > if (!GetFirstArgumentAsObject(cx, argc, vp, "Object.keys", &obj)) > return JS_FALSE; > > AutoIdVector props(cx); > if (!GetPropertyNames(cx, obj, JSITER_OWNONLY, &props)) > return JS_FALSE; > > AutoValueVector vals(cx); >- vals.resize(props.length()); >+ vals.reserve(props.length()); /* avoid oom check below */ Shouldn't this check for OOM? > for (size_t i = 0, len = props.length(); i < len; i++) { > jsid id = props[i]; > if (JSID_IS_STRING(id)) { >- vals[i].setString(JSID_TO_STRING(id)); >- } else { >- JS_ASSERT(JSID_IS_INT(id)); >+ vals.append(StringValue(JSID_TO_STRING(id))); >+ } else if (JSID_IS_INT(id)) { > JSString *str = js_IntToString(cx, JSID_TO_INT(id)); > if (!str) > return JS_FALSE; >- vals[i].setString(str); >+ vals.append(StringValue(str)); >+ } else { >+ JS_ASSERT(JSID_IS_OBJECT(id)); > } > } These should use JS_ALWAYS_OK to verify OOM-safeness.
Attachment #489370 -
Flags: review?(jwalden+bmo) → review+
Assignee | ||
Comment 6•14 years ago
|
||
Duh, thanks!
Assignee | ||
Comment 7•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/b9eac30071aa
Whiteboard: fixed-in-tracemonkey
Comment 8•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b9eac30071aa
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 9•11 years ago
|
||
Automatically extracted testcase for this bug was committed: https://hg.mozilla.org/mozilla-central/rev/efaf8960a929
Flags: in-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•