ASSERTION FAILED: length <= maximumLength in js-fixed-array-out-of-memory.js

[Mark Lam]

Pretty sure this is due to r253520.

[Mark Lam]

To reproduce the failure: 
% VM=WebKitBuild/Debug && DYLD_FRAMEWORK_PATH=$VM lldb $VM/jsc -- --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true --maxSingleAllocationSize\=1048576 JSTests/stress/js-fixed-array-out-of-memory.js

The stack trace:

* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xbbadbeef)
  * frame #0: 0x0000000100286b0e JavaScriptCore`::WTFCrash() at Assertions.cpp:305:35
    frame #1: 0x0000000100975cfb JavaScriptCore`WTFCrashWithInfo((null)=60, (null)="./runtime/IndexingHeader.h", (null)="void JSC::IndexingHeader::setVectorLength(uint32_t)", (null)=698) at Assertions.h:622:5
    frame #2: 0x000000010173f7aa JavaScriptCore`JSC::IndexingHeader::setVectorLength(this=0x00000018d5fa6070, length=500000000) at IndexingHeader.h:60:9
    frame #3: 0x0000000100d6e691 JavaScriptCore`JSC::JSImmutableButterfly::JSImmutableButterfly(this=0x00000018d5fa6068, vm=0x0000000109200000, structure=0x00000001094f05b0, length=500000000) at JSImmutableButterfly.h:192:18
    frame #4: 0x0000000100d6e0bb JavaScriptCore`JSC::JSImmutableButterfly::JSImmutableButterfly(this=0x00000018d5fa6068, vm=0x0000000109200000, structure=0x00000001094f05b0, length=500000000) at JSImmutableButterfly.h:191:5
    frame #5: 0x0000000100d6e030 JavaScriptCore`JSC::JSImmutableButterfly::tryCreate(vm=0x0000000109200000, structure=0x00000001094f05b0, size=500000000) at JSImmutableButterfly.h:59:62
    frame #6: 0x00000001010f0c3f JavaScriptCore`JSC::JSImmutableButterfly::createFromArray(globalObject=0x00000001093fc068, vm=0x0000000109200000, array=0x00000001094ebec0) at JSImmutableButterfly.h:84:40
    frame #7: 0x00000001017b89e5 JavaScriptCore`::slow_path_spread(callFrame=0x00007ffeefbfd7d0, pc=0x00000001090919fc) at CommonSlowPaths.cpp:1469:13
    frame #8: 0x000000010075fa91 JavaScriptCore`llint_entry at LowLevelInterpreter64.asm:108

[Radar WebKit Bug Importer]

<rdar://problem/57978411>

[Yusuke Suzuki]

Created attachment 385806 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 385806 [details]
Patch

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

> Source/JavaScriptCore/runtime/IndexingHeader.h:42
> +    static constexpr unsigned maximumLength = 0x10000000;

I think this size limit for JSImmutableButterfly is reasonable: since each element is 8 byte, the size of bytes become 2GB, which already exceeds various limits (like, we kill such a WebProcess).

[Yusuke Suzuki]

Created attachment 385822 [details]
Patch

[Yusuke Suzuki]

Created attachment 385823 [details]
Patch

[Yusuke Suzuki]

Created attachment 385824 [details]
Patch

[Mark Lam]

Comment on attachment 385824 [details]
Patch

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

r=me

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7967
>          m_jit.mutatorFence(vm());

Is this mutatorFence executed on any path?  It looks to me like this is dead code.  do we need it?

> Source/JavaScriptCore/runtime/JSImmutableButterfly.h:55
> +        // Because of the above maximumLength requirement, overflowing never happens.

I suggest rephrasing this comment slightly as /overflowing never happens/allocationSize can never overflow/.  In the context of this patch, this is obvious.  But reading back later without this patch in mind, it may not be clear what the "overflow" is referring to.  Rephrasing it thus should make that clearer.

[Yusuke Suzuki]

Comment on attachment 385824 [details]
Patch

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

Thanks!

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:7967
>>          m_jit.mutatorFence(vm());
> 
> Is this mutatorFence executed on any path?  It looks to me like this is dead code.  do we need it?

We should call it. Fixed.

>> Source/JavaScriptCore/runtime/JSImmutableButterfly.h:55
>> +        // Because of the above maximumLength requirement, overflowing never happens.
> 
> I suggest rephrasing this comment slightly as /overflowing never happens/allocationSize can never overflow/.  In the context of this patch, this is obvious.  But reading back later without this patch in mind, it may not be clear what the "overflow" is referring to.  Rephrasing it thus should make that clearer.

Fixed.

[Yusuke Suzuki]

Committed r253608: <https://trac.webkit.org/changeset/253608>