Currently array_lookupProperty from jsarray.cpp uses a special slot, JSSLOT_ARRAY_LOOKUP_HOLDER, to store the index passed to the function. But this is not necessary since to satisfy JSObjectOps requirements it is sufficient to convert the id itself to JSProperty* and cast it back in array_dropProperty and array_getAttributes. It not only shrinks the code but also free the slot for other uses.
When creating a patch for this bug I noticed a regression that I introduced in bug 486106. There I missed that array_lookupProperty also takes care about the length property. So I will use this bug to fix that as well. I also nominate for 1.9.1 - the bug does not affect browser or js shell, but it could bite other SpiderMonkey users.
Severity: enhancement → normal
Created attachment 372236 [details] [diff] [review] v1 The essence of the patch is casting jsid to (JSProperty *) in array_lookupProperty. The rest is fixes to make sure that js_GetDenseArrayElementValue returns the correct result for any id that was accepted in array_lookupProperty. The change required to add JSContext *cx parameter to js_GetDenseArrayElementValue triggering a cascade of changes for Lookup* API in jsapi.cpp.
Attachment #372236 - Flags: review?(mrbkap)
Comment on attachment 372236 [details] [diff] [review] v1 >Index: tm/js/src/jsarray.cpp > * In dense mode, holes in the array are represented by JSVAL_HOLE. The final >- * slot in fslots (JSSLOT_ARRAY_LOOKUP_HOLDER) is used to store the single jsid >- * "in use" by a lookupProperty caller. >+ * slot in fslots is unused.. Nit: extra period.
Attachment #372236 - Flags: review?(mrbkap) → review+
landed to TM with the nit addressed - http://hg.mozilla.org/tracemonkey/rev/ccb029897983
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Flags: wanted1.9.1? → wanted1.9.1+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.