906891 - allocation overflow when enumerating properties of typed arrays via isFrozen

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

I hit an allocation overflow while debugging; looks like we're trying to enumerate all the properties of a typed array in a hash table, and we die after we hit 12 millionish entries:

>	mozjs.dll!js_ReportAllocationOverflow(js::ThreadSafeContext * cxArg) Line 452	C++
 	mozjs.dll!js::detail::HashTable<js::Shape * __ptr64 const,js::HashSet<js::Shape * __ptr64,js::DefaultHasher<js::Shape * __ptr64>,js::TempAllocPolicy>::SetOps,js::TempAllocPolicy>::changeTableSize(int deltaLog2) Line 1149	C++
 	mozjs.dll!Enumerate(JSContext * cx, JS::Handle<JSObject *> pobj, __int64 id, bool enumerable, unsigned int flags, js::HashSet<__int64,IdHashPolicy,js::TempAllocPolicy> & ht, JS::AutoIdVector * props) Line 120	C++
 	mozjs.dll!Snapshot(JSContext * cx, JSObject * pobj_, unsigned int flags, JS::AutoIdVector * props) Line 245	C++
 	mozjs.dll!JSObject::isSealedOrFrozen(JSContext * cx, JS::Handle<JSObject *> obj, JSObject::ImmutabilityType it, bool * resultp) Line 1244	C++
 	mozjs.dll!DebuggerObject_isSealedHelper(JSContext * cx, unsigned int argc, JS::Value * vp, SealHelperOp op, const char * name) Line 4904	C++
 	mozjs.dll!DebuggerObject_isFrozen(JSContext * cx, unsigned int argc, JS::Value * vp) Line 4924	C++
 	mozjs.dll!js::Invoke(JSContext * cx, JS::CallArgs args, js::MaybeConstruct construct) Line 482	C++
 	mozjs.dll!Interpret(JSContext * cx, js::RunState & state) Line 2484	C++

Comment 1

[Brandon Benvie [:benvie]]

STR:

> Object.isFrozen(new Uint8Array(1024 * 1024 * 32))

Comment 2

[Brandon Benvie [:benvie]]

Same thing happens with isSealed.

Related, typed arrays can't be frozen because attempting to change property attributes of an indexed property throws. An optimization (and indirect fix to this bug) would be to optimize `isFrozen` for object types that can never be frozen. This doesn't fix the isSealed case though, since these objects can be sealed.

Comment 3

[Brandon Benvie [:benvie]]

Since a typed array never has any own non-indexed properties, and is non-extensible at birth, a typed array should always be considered sealed. A 0 length typed array is also considered frozen, but a typed array with more than 0 length should always be frozen. This should be a good optimization for these operations on typed arrays and fix the bug at the same time.

Comment 4

[Brandon Benvie [:benvie]]

Sorry, a typed array with more than 0 length should always be *non* frozen

Comment 5

[Jim Blandy :jimb]

Attachment: Short-circuit isSealed and isFrozen for typed arrays.

Typed arrays are sometimes ginormous, and it's pretty surprising when applying Object.isSealed or Object.isFrozen to an object OOMs - which they will do when trying to build an array of index ids for a 16Mb Uint8Array. In particular, this implementation makes innocent-looking code like the following (in the JS debugger server) vulnerable:

    let g = {
      "type": "object",
      "class": this.obj.class,
      "actor": this.actorID,
      "extensible": this.obj.isExtensible(),
      "frozen": this.obj.isFrozen(),
      "sealed": this.obj.isSealed()
    };

This patch short-circuits JSObject::isSealed and JSObject::isFrozen for typed arrays.

Comment 6

[Steve Fink [:sfink] [:s:]]

Comment on attachment 792503 [details] [diff] [review]
Short-circuit isSealed and isFrozen for typed arrays.

Review of attachment 792503 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/basic/typed-array-sealed-frozen.js
@@ +1,1 @@
> +// Typed arrays are always sealed.

I think these are supposed to have a license, which is usually:

// Any copyright is dedicated to the Public Domain.
// http://creativecommons.org/licenses/publicdomain/

Comment 7

[Jim Blandy :jimb]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c0209f449921

Comment 8

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/c0209f449921