On Jul 14, 2011, at 10:04 AM, Allen Wirfs-Brock wrote:

> This appears to be a (possible) specification bug that first occurs in ES3 
> and which was not corrected in ES5.
> 
> I appears in concat and push. But no other algorithms.  The problem occurs 
> when when a length value is incremented and then used as an array index 
> without first applying ToUint32 to the value.  It only occurs concat and push 
> because all the other algorithms are bounded by a length value that has had 
> ToUint32 applied to it.

FYI, SpiderMonkey (Firefox) results:

js> a=[]
[]
js> a.length=0xffffffff
4294967295
js> b=a.concat([1])
[]
js> b.length
0

js> a=[]
[]
js> a.length=0xffffffff
4294967295
js> a.push(1)
typein:3: RangeError: invalid array length


> It is probably a bug, because array index based operations generally warp 
> around to 0 at 2^32.  I assume that the Firefox implementation was the  
> initial one and it reflects that behavior.

SpiderMonkey in Firefox 3 era does the same as above.


>  However, all array objects (and generic array like objects) are permitted to 
> have integer  property names >= 2^32.  These properties are not reflected in 
> the value of the 'length' property (when set by generic array methods for 
> array-like objects).  So, there is a slim chance that the specified behavior 
> was intentional in allowing concat and push to leak past the 2^32 element 
> boundary. 

I doubt anything was intentional here, but I wasn't in on this part of ES3 in 
detail.

Jeff Walden had to do a bunch of work in SpiderMonkey to avoid wrongly wrapping 
uint32 index computations that must overflow to double or (safely constrained) 
uint64 before being stored as property names.

I don't think length should ever wrap around for a property name that marches 
past 0xffffffff. That's just asking for trouble. It's not as bad as the 
equivalent bug in unsafe languages (size overflow resulting in buffer 
under-allocation and indexing overruns), but it's in that direction, and a 
separate bug could combine to make a nasty exploit.


> We have to possible courses of action:
>    1) consider the specification to be correct as written. 
>    2) consider this to be a spec. but that we correct in the errata and in 
> future editions.
> 
> This is an edge case that probably never occurs in the real world so the 
> choice probably has no real impact.  #1 is most consistent with the rest of 
> the ES spec.  However, the 2^32 length wrap semantics of arrays is kind of 
> bogus and it might be nice to try to eliminate it in the future.  #2 would be 
> a step away from that.
> 
> this now bug: https://bugs.ecmascript.org/show_bug.cgi?id=131 

Would appreciate Jeff's thoughts. I've learned my lesson many times on 
wraparound being wrong, so I'm in favor of RangeError. We do that for push but 
not concat in SpiderMonkey, so AFAIK we could just fix concat, as you note no 
one would care except our testsuites, and we'd be safer and more consistent at 
the margin.

/be
_______________________________________________
es-discuss mailing list
[email protected]
https://mail.mozilla.org/listinfo/es-discuss

Reply via email to