Bug 188917 – RELEASE_ASSERT at ../../Source/JavaScriptCore/heap/MarkedSpace.h:83

• Component: JavaScriptCore
• Status: RESOLVED FIXED

the following poc triggers a RELEASE_ASSERT at ../../Source/JavaScriptCore/heap/MarkedSpace.h:83

    static size_t sizeClassToIndex(size_t size)
    {
        RELEASE_ASSERT(size);                       ==>(1)
        return (size + sizeStep - 1) / sizeStep - 1;
    }
where size=0, without this RELEASE_ASSERT, the function will return 18446744073709551615.
the poc may need to run several times to trigger crash.

poc:
==================================================================
function foo(o){}

function test() {
    	var floatArray = foo(new Float64Array(0));
}

for (var i = 0; i < 100000; ++i){
    test();
}
test();
==================================================================

stack trace:
1   0x42c45b
2   0x7fd3cc70c9ef
3   0x7fd3cc6d05cd
4   0x7fd3cc66c682 JSC::FTL::lowerDFGToB3(JSC::FTL::State&)
5   0x7fd3cc44e7f3 JSC::DFG::Plan::compileInThreadImpl()
6   0x7fd3cc44a0da JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
7   0x7fd3cc64f372 JSC::DFG::Worklist::ThreadBody::work()
8   0x7fd3ccdaaa7c
9   0x7fd3ccdc39c1 WTF::Thread::entryPoint(WTF::Thread::NewThreadContext*)
10  0x7fd3ccdf7003
11  0x7fd3c99ba6ba
12  0x7fd3c89ed41d clone
Segmentation fault

[Keith Miller]

Created attachment 348132 [details]
Patch

[EWS Watchlist]

Comment on attachment 348132 [details]
Patch

Attachment 348132 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/8994239

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html

[EWS Watchlist]

Created attachment 348133 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348132 [details]
Patch

Attachment 348132 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/8994230

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html

[EWS Watchlist]

Created attachment 348134 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348132 [details]
Patch

Attachment 348132 [details] did not pass mac-debug-ews (mac):
Output: https://webkit-queues.webkit.org/results/8994281

New failing tests:
js/slow-stress/Int32Array-alloc-large-long-lived.html

[EWS Watchlist]

Created attachment 348137 [details]
Archive of layout-test-results from ews114 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Radar WebKit Bug Importer]

<rdar://problem/43788245>

[Ryan Haddad]

Something about this patch causes JSC EWS bots to hang while running tests.

[Mark Lam]

<rdar://problem/43433804>

[Keith Miller]

Created attachment 348935 [details]
Patch

[Mark Lam]

*** Bug 189285 has been marked as a duplicate of this bug. ***

[Mark Lam]

<rdar://problem/39380095>

[Mark Lam]

<rdar://problem/36715428>

[EWS Watchlist]

Comment on attachment 348935 [details]
Patch

Attachment 348935 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9104276

New failing tests:
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-csksc56011987.html
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-korean.html
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-ks_c_5601-1987.html
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-windows-949.html
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-ksc5601.html
imported/w3c/web-platform-tests/encoding/legacy-mb-japanese/shift_jis/sjis-encode-form-errors-han.html
imported/w3c/web-platform-tests/encoding/legacy-mb-korean/euc-kr/euckr-encode-form-iso-ir-149.html

[EWS Watchlist]

Created attachment 348952 [details]
Archive of layout-test-results from ews103 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 348935 [details]
Patch

Attachment 348935 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/9104104

New failing tests:
stress/new-array-with-spread-with-phantom-new-array-buffer.js.ftl-no-cjit-no-put-stack-validate
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager-no-cjit-b3o1
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-no-put-stack-validate
stress/new-array-with-spread-with-normal-spread-and-phantom-spread.js.ftl-eager
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.ftl-no-cjit-validate-sampling-profiler
stress/arguments-custom-properties-gc.js.ftl-eager
stress/new-array-with-spread-with-phantom-new-array-buffer.js.ftl-no-cjit-small-pool
stress/IIFE-function-name-captured.js.ftl-eager-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.ftl-eager-no-cjit
slowMicrobenchmarks.yaml/slowMicrobenchmarks/rest-parameter-allocation-elimination.js.default
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager
stress/phantom-new-array-buffer-forward-varargs.js.ftl-no-cjit-no-inline-validate
stress/arguments-custom-properties-gc.js.default
stress/typedarray-functions-with-neutered.js.ftl-eager
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-eager-no-cjit
stress/IIFE-function-name-captured.js.ftl-eager
stress/new-array-with-spread-with-normal-spread-and-phantom-spread.js.ftl-eager-no-cjit
stress/arguments-custom-properties-gc.js.ftl-no-cjit-no-put-stack-validate
jsc-layout-tests.yaml/js/script-tests/array-enumerators-functions.js.layout-ftl-eager-no-cjit
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-b3o1
stress/typedarray-functions-with-neutered.js.ftl-eager-no-cjit
stress/phantom-new-array-buffer-forward-varargs.js.ftl-no-cjit-no-put-stack-validate
jsc-layout-tests.yaml/js/script-tests/Object-getOwnPropertyNames.js.layout-ftl-eager-no-cjit
stress/arguments-custom-properties-gc.js.ftl-no-cjit-b3o1
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-no-cjit-b3o1
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-ftl-eager-no-cjit
stress/arguments-custom-properties-gc.js.ftl-eager-no-cjit-b3o1
stress/new-array-with-spread-with-phantom-new-array-buffer.js.default
stress/tagged-template-tdz.js.ftl-eager
v8-v6/v8-earley-boyer.js.ftl-eager-no-cjit-b3o1
stress/phantom-new-array-buffer-forward-varargs.js.ftl-no-cjit-validate-sampling-profiler
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-no-cjit-no-inline-validate
stress/phantom-new-array-buffer-forward-varargs.js.ftl-no-cjit-b3o1
v8-v6/v8-earley-boyer.js.ftl-eager-no-cjit
v8-v6/v8-earley-boyer.js.ftl-eager
stress/arguments-custom-properties-gc.js.ftl-eager-no-cjit
stress/new-array-with-spread-with-phantom-new-array-buffer.js.ftl-no-cjit-b3o1
stress/arguments-custom-properties-gc.js.ftl-no-cjit-small-pool
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.ftl-no-cjit-validate-sampling-profiler
stress/arguments-custom-properties-gc.js.ftl-no-cjit-no-inline-validate
stress/regexp-matches-array-bad-time.js.ftl-eager-no-cjit-b3o1
stress/v8-earley-boyer-strict.js.ftl-eager
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-validate-sampling-profiler
stress/arguments-custom-properties-gc.js.ftl-no-cjit-validate-sampling-profiler
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-no-cjit-no-put-stack-validate
stress/v8-earley-boyer-strict.js.ftl-eager-no-cjit-b3o1
stress/v8-earley-boyer-strict.js.ftl-eager-no-cjit
stress/tagged-template-tdz.js.ftl-eager-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.ftl-eager
stress/new-array-with-spread-with-phantom-new-array-buffer.js.ftl-eager-no-cjit-b3o1
stress/spread-non-array.js.ftl-eager
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.ftl-no-cjit-validate-sampling-profiler
stress/spread-non-array.js.ftl-eager-no-cjit
stress/new-array-with-spread-with-phantom-new-array-buffer.js.ftl-no-cjit-validate-sampling-profiler
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-no-inline-validate
apiTests

[Mark Lam]

Comment on attachment 348935 [details]
Patch

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

r=me if you can show that the test failures are not related to this patch.

> Source/JavaScriptCore/heap/MarkedSpace.h:88
> -        return (index + 1) * sizeStep;
> +        return index * sizeStep;

nit: can you express this as follows?
    size_t result = index * sizeStep;
    ASSERT(sizeClassToIndex(result) == index);
    return result;

I also recommend adding the following ASSERT in MarkedSpace.cpp's buildSizeClassTable:
    ASSERT(MarkedSpace::sizeClassToIndex(largeCutoff - 1) < MarkedSpace::numSizeClasses);

[Keith Miller]

Created attachment 348953 [details]
Patch for landing

[Keith Miller]

Committed r235685: <https://trac.webkit.org/changeset/235685>