Bug 175642 – ChakraCore/test/Function/apply3.js is resulting wrong result in x86_64

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Caio Lima]

I didn't test in architectures other than TRADITIONAL_ARM and x86_64. According to https://github.com/Microsoft/ChakraCore/blob/master/test/Function/apply3.baseline the result in test case for:

```
guarded_call(function() {
    dump_args.apply(o, {length: 4294967295 + 1}); //UINT32_MAX + 1
});
```

Should be:

```
Exception: RangeError : Argument list too large to apply
```

The problem is that into Source/JavaScriptCore/interpreter/Interpreter.cpp::sizeOfVarargs() the call ```unsigned length = toLength(callFrame, jsCast<JSObject*>(cell));``` is coercing a double to a unsigned and the result is "0" to length into x86_64.

[Caio Lima]

Created attachment 318315 [details]
Patch

[Build Bot]

Attachment 318315 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/ChangeLog:12:  Line contains tab character.  [whitespace/tab] [5]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Caio Lima]

Created attachment 318318 [details]
Patch

[Yusuke Suzuki]

Comment on attachment 318318 [details]
Patch

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

> Source/JavaScriptCore/dfg/DFGOperations.cpp:1886
> +    return static_cast<int32_t>(sizeOfVarargs(exec, arguments, firstVarArgOffset));

What happens if sizeOfVarargs returns "4294967295 + 1" value?

> Source/JavaScriptCore/interpreter/Interpreter.cpp:241
> +    CallFrame* calleeFrame = calleeFrameForVarargs(callFrame, numUsedStackSlots, static_cast<unsigned>(length) + 1);

What happens if the length is larger than UINT32_MAX?

[Caio Lima]

Comment on attachment 318318 [details]
Patch

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

I'm not liking this solution. I need to rethink how could I handle the case properly.

>> Source/JavaScriptCore/dfg/DFGOperations.cpp:1886
>> +    return static_cast<int32_t>(sizeOfVarargs(exec, arguments, firstVarArgOffset));
> 
> What happens if sizeOfVarargs returns "4294967295 + 1" value?

It will follow the same behavior as before, and cast it to int32_t. In such case, in x86_64 it is coercing to 0.

>> Source/JavaScriptCore/interpreter/Interpreter.cpp:241
>> +    CallFrame* calleeFrame = calleeFrameForVarargs(callFrame, numUsedStackSlots, static_cast<unsigned>(length) + 1);
> 
> What happens if the length is larger than UINT32_MAX?

It will throw in stackOverflow in line 242.

[Caio Lima]

Created attachment 318711 [details]
Patch

[Build Bot]

Comment on attachment 318711 [details]
Patch

Attachment 318711 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/4357017

New failing tests:
stress/construct-forward-varargs-for-inlined-escaped-arguments.js.ftl-eager-no-cjit
es6.yaml/es6/spread_..._operator_with_generic_iterables_in_calls.js.default
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.dfg-maximal-flush-validate-no-cjit
stress/put-direct-index-broken-2.js.no-llint
microbenchmarks/call-spread-call.js.no-ftl
stress/spread-multi-layers.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-inlined-simple-exit-aliasing.js.no-cjit-collect-continuously
stress/rest-parameter-many-arguments.js.default
stress/varargs-inlined-simple-exit-aliasing.js.ftl-no-cjit-no-inline-validate
microbenchmarks/call-spread-apply.js.dfg-eager-no-cjit-validate
stress/function-constructor-semantics.js.ftl-eager
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-b3o1
basic-tests.yaml/stress-test.js.ftl-eager-no-cjit
stress/call-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.dfg-eager
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.no-cjit
stress/varargs-inlined-simple-exit-aliasing.js.no-cjit-validate-phases
v8-v6/v8-raytrace.js.dfg-maximal-flush-validate-no-cjit
stress/call-forward-varargs-for-inlined-escaped-arguments.js.default
stress/tail-call-varargs-no-stack-overflow.js.ftl-no-cjit-no-inline-validate
stress/apply-second-argument-must-be-array-like.js.default
stress/put-direct-index-broken-2.js.ftl-eager
v8-v6/v8-raytrace.js.ftl-eager
stress/put-direct-index-broken-2.js.dfg-eager-no-cjit-validate
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.default
stress/construct-varargs-no-inline.js.ftl-eager
stress/rest-parameter-many-arguments.js.ftl-no-cjit-no-put-stack-validate
stress/spread-multi-layers.js.no-cjit-validate-phases
stress/liveness-pruning-needed-for-osr-availability-eager.js.dfg-maximal-flush-validate-no-cjit
stress/rest-parameter-having-a-bad-time.js.ftl-eager-no-cjit-b3o1
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.ftl-no-cjit-no-inline-validate
stress/varargs-closure-inlined-exit.js.ftl-no-cjit-no-inline-validate
stress/put-direct-index-broken-2.js.ftl-no-cjit-no-put-stack-validate
stress/varargs-inlined-simple-exit-aliasing.js.ftl-no-cjit-small-pool
microbenchmarks/call-spread-apply.js.ftl-no-cjit-b3o1
stress/super-property-access.js.ftl-eager-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.ftl-no-cjit-no-inline-validate
microbenchmarks/super-get-by-id-with-this-monomorphic.js.ftl-eager-no-cjit-b3o1
ChakraCore.yaml/ChakraCore/test/es5/augmentPrimitive.js.default
microbenchmarks/call-spread-apply.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.no-llint
microbenchmarks/call-spread-apply.js.default
stress/inline-call-varargs-and-call.js.ftl-no-cjit-no-inline-validate
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout-no-ftl
stress/put-direct-index-broken-2.js.ftl-no-cjit-no-inline-validate
stress/call-forward-varargs-for-inlined-escaped-arguments.js.no-ftl
microbenchmarks/super-get-by-id-with-this-monomorphic.js.no-ftl
ChakraCore.yaml/ChakraCore/test/UnifiedRegex/SourceToString.js.default
microbenchmarks/call-spread-call.js.ftl-eager-no-cjit-b3o1
stress/put-direct-index-broken-2.js.ftl-eager-no-cjit-b3o1
microbenchmarks/deltablue-varargs.js.no-ftl
stress/put-direct-index-broken-2.js.ftl-no-cjit-b3o1
stress/call-varargs-from-inlined-code.js.dfg-maximal-flush-validate-no-cjit
ChakraCore.yaml/ChakraCore/test/strict/22.callerCalleeArguments.js.default
stress/varargs-inlined-simple-exit-aliasing-weird.js.dfg-maximal-flush-validate-no-cjit
stress/spread-multi-layers.js.no-llint
microbenchmarks/super-get-by-val-with-this-monomorphic.js.default
stress/super-property-access.js.no-ftl
stress/call-forward-varargs-for-inlined-escaped-arguments.js.no-cjit-collect-continuously
stress/spread-multi-layers.js.ftl-no-cjit-no-inline-validate
ChakraCore.yaml/ChakraCore/test/UnifiedRegex/mru.js.default
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/call-spread-call.js.ftl-no-cjit-small-pool
basic-tests.yaml/stress-test.js.no-ftl
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.ftl-no-cjit-b3o1
stress/call-forward-varargs-for-inlined-escaped-arguments.js.no-cjit-validate-phases
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout-no-cjit
stress/spread-capture-rest.js.ftl-no-cjit-no-inline-validate
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.ftl-no-cjit-no-put-stack-validate
stress/spread-multi-layers.js.dfg-eager-no-cjit-validate
stress/call-forward-varargs-for-inlined-escaped-arguments.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-varargs-closure-inlined-exit.js.ftl-no-cjit-no-inline-validate
stress/call-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-validate-sampling-profiler
stress/varargs-inlined-simple-exit-aliasing.js.no-llint
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-eager
microbenchmarks/call-spread-call.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/call-spread-call.js.ftl-eager-no-cjit
microbenchmarks/deltablue-varargs.js.no-cjit-validate-phases
stress/spread-multi-layers.js.no-cjit-collect-continuously
stress/call-forward-varargs-for-inlined-escaped-arguments.js.dfg-eager-no-cjit-validate
stress/class-subclassing-array.js.ftl-eager-no-cjit
stress/put-direct-index-broken-2.js.ftl-no-cjit-validate-sampling-profiler
ChakraCore.yaml/ChakraCore/test/Operators/equals.js.default
stress/argument-count-bytecode.js.default
microbenchmarks/deltablue-varargs.js.dfg-eager
stress/put-direct-index-broken-2.js.default
ChakraCore.yaml/ChakraCore/test/strict/23.reservedWords_sm.js.default
stress/varargs-then-slow-call.js.ftl-no-cjit-no-inline-validate
microbenchmarks/call-spread-apply.js.ftl-no-cjit-small-pool
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-validate-sampling-profiler
stress/put-direct-index-broken-2.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-dfg-eager-no-cjit
stress/varargs-inlined-simple-exit-aliasing.js.dfg-eager
stress/function-constructor-semantics.js.ftl-eager-no-cjit
stress/spread-multi-layers.js.no-ftl
microbenchmarks/call-spread-apply.js.ftl-eager-no-cjit
stress/arguments-callee-uninitialized.js.ftl-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.no-ftl
microbenchmarks/deltablue-varargs.js.dfg-eager-no-cjit-validate
stress/varargs-inlined-simple-exit-aliasing.js.ftl-eager
microbenchmarks/super-get-by-id-with-this-monomorphic.js.default
stress/varargs-simple.js.dfg-maximal-flush-validate-no-cjit
stress/spread-multi-layers.js.default
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout-ftl-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.no-cjit-collect-continuously
stress/call-forward-varargs-for-inlined-escaped-arguments.js.dfg-eager
stress/put-direct-index-broken-2.js.no-cjit-collect-continuously
stress/phantom-direct-arguments-clobber-callee.js.ftl-eager
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-no-ftl
microbenchmarks/deltablue-varargs.js.default
stress/varargs-inlined-simple-exit-aliasing.js.no-ftl
stress/spread-non-varargs.js.ftl-no-cjit-no-inline-validate
stress/varargs-inlined-exit.js.ftl-no-cjit-no-inline-validate
stress/argument-count-bytecode.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout
stress/call-varargs-from-inlined-code-with-odd-number-of-arguments.js.dfg-maximal-flush-validate-no-cjit
stress/spread-multi-layers.js.ftl-eager
stress/varargs-exit.js.ftl-no-cjit-no-inline-validate
microbenchmarks/deltablue-varargs.js.ftl-eager
stress/varargs-varargs-inlined-exit.js.ftl-no-cjit-no-inline-validate
stress/construct-varargs-inline.js.ftl-eager-no-cjit
microbenchmarks/super-get-by-val-with-this-monomorphic.js.no-llint
stress/rest-parameter-many-arguments.js.ftl-eager-no-cjit-b3o1
stress/phantom-direct-arguments-clobber-callee.js.default
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.ftl-no-cjit-small-pool
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout-ftl-eager-no-cjit
stress/spread-multi-layers.js.ftl-no-cjit-small-pool
stress/phantom-direct-arguments-clobber-callee.js.ftl-eager-no-cjit-b3o1
stress/arguments-elimination-varargs-too-many-args-arg-count.js.dfg-eager-no-cjit-validate
microbenchmarks/super-get-by-val-with-this-monomorphic.js.no-ftl
stress/rest-parameter-many-arguments.js.ftl-no-cjit-small-pool
stress/rest-parameter-many-arguments.js.ftl-no-cjit-no-inline-validate
stress/spread-capture-rest.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-no-cjit-small-pool
stress/call-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-no-put-stack-validate
ChakraCore.yaml/ChakraCore/test/Function/argumentsResolution.js.default
microbenchmarks/call-spread-call.js.no-cjit-validate-phases
stress/varargs-inlined-simple-exit-aliasing.js.default
microbenchmarks/call-spread-call.js.no-cjit-collect-continuously
microbenchmarks/call-spread-apply.js.ftl-eager-no-cjit-b3o1
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.default
stress/function-constructor-semantics.js.ftl-no-cjit-small-pool
microbenchmarks/call-spread-call.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-no-inline-validate
basic-tests.yaml/stress-test.js.default
stress/put-direct-index-broken-2.js.no-cjit-validate-phases
ChakraCore.yaml/ChakraCore/test/Miscellaneous/HasOnlyWritableDataPropertiesCache.js.default
microbenchmarks/call-spread-apply.js.ftl-no-cjit-validate-sampling-profiler
stress/construct-varargs-no-inline.js.ftl-eager-no-cjit
stress/put-direct-index-broken-2.js.ftl-eager-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird.js.no-cjit-collect-continuously
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.dfg-eager
microbenchmarks/call-spread-call.js.ftl-no-cjit-no-inline-validate
microbenchmarks/super-get-by-id-with-this-monomorphic.js.no-llint
stress/spread-calling.js.ftl-eager-no-cjit
stress/call-forward-varargs-for-inlined-escaped-arguments.js.ftl-eager
stress/arguments-elimination-varargs-too-many-args-arg-count.js.no-cjit-validate-phases
stress/super-property-access.js.no-llint
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-ftl-eager-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-ftl-no-cjit
stress/spread-multi-layers.js.ftl-no-cjit-b3o1
stress/put-direct-index-broken-2.js.dfg-eager
microbenchmarks/super-get-by-val-with-this-monomorphic.js.ftl-eager-no-cjit-b3o1
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-no-cjit-b3o1
stress/varargs-inlined-simple-exit-aliasing.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/call-spread-apply.js.ftl-no-cjit-no-put-stack-validate
stress/phantom-direct-arguments-clobber-callee.js.ftl-eager-no-cjit
stress/spread-multi-layers.js.dfg-eager
stress/construct-varargs-no-inline.js.ftl-eager-no-cjit-b3o1
ChakraCore.yaml/ChakraCore/test/strict/23.reservedWords.js.default
microbenchmarks/deltablue-varargs.js.ftl-eager-no-cjit
stress/varargs-inlined-simple-exit-aliasing.js.dfg-maximal-flush-validate-no-cjit
jsc-layout-tests.yaml/js/script-tests/basic-spread.js.layout-no-cjit
stress/rest-parameter-many-arguments.js.ftl-no-cjit-validate-sampling-profiler
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/call-spread-apply.js.no-cjit-collect-continuously
microbenchmarks/varargs-call.js.dfg-maximal-flush-validate-no-cjit
stress/phantom-direct-arguments-clobber-callee.js.ftl-no-cjit-no-inline-validate
stress/rest-parameter-many-arguments.js.dfg-eager-no-cjit-validate
stress/rest-parameter-many-arguments.js.ftl-eager-no-cjit
stress/spread-multi-layers.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/call-spread-call.js.dfg-eager
microbenchmarks/deltablue-varargs.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/call-spread-call.js.ftl-no-cjit-b3o1
stress/function-constructor-semantics.js.dfg-eager
stress/varargs-inlined-simple-exit-aliasing-weird.js.no-llint
stress/call-forward-varargs-for-inlined-escaped-arguments.js.no-llint
microbenchmarks/deltablue-varargs.js.no-cjit-collect-continuously
microbenchmarks/call-spread-call.js.ftl-eager
stress/arguments-elimination-varargs-too-many-args-arg-count.js.no-cjit-collect-continuously
stress/phantom-direct-arguments-clobber-argument-count.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/call-spread-call.js.default
stress/function-constructor-semantics.js.ftl-eager-no-cjit-b3o1
stress/function-constructor-semantics.js.dfg-eager-no-cjit-validate
stress/arguments-elimination-varargs-too-many-args-arg-count.js.ftl-no-cjit-no-inline-validate
stress/rest-parameter-many-arguments.js.ftl-eager
stress/varargs-inlined-simple-exit-aliasing.js.ftl-no-cjit-no-put-stack-validate
v8-v6/v8-raytrace.js.dfg-eager
stress/phantom-direct-arguments-clobber-callee.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.no-ftl
microbenchmarks/call-spread-apply.js.ftl-eager
stress/super-property-access.js.default
microbenchmarks/super-get-by-val-with-this-monomorphic.js.ftl-eager
stress/varargs-inlined-simple-exit-aliasing-weird.js.no-cjit-validate-phases
stress/super-property-access.js.ftl-eager-no-cjit
stress/varargs-varargs-closure-inlined-exit.js.dfg-maximal-flush-validate-no-cjit
stress/call-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-small-pool
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.ftl-no-cjit-validate-sampling-profiler
basic-tests.yaml/stress-test.js.ftl-no-cjit
stress/spread-multi-layers.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/call-spread-call.js.dfg-maximal-flush-validate-no-cjit
stress/super-property-access.js.ftl-eager
microbenchmarks/call-spread-apply.js.no-cjit-validate-phases
stress/varargs-inlined-simple-exit-aliasing-weird.js.dfg-eager
stress/phantom-direct-arguments-clobber-callee.js.dfg-eager-no-cjit-validate
stress/tail-call-varargs-no-stack-overflow.js.ftl-eager
stress/varargs-inlined-simple-exit-aliasing.js.ftl-no-cjit-b3o1
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-no-cjit-no-inline-validate
microbenchmarks/deltablue-varargs.js.ftl-eager-no-cjit-b3o1
microbenchmarks/call-spread-call.js.dfg-eager-no-cjit-validate
stress/varargs-inlined-simple-exit-aliasing-weird.js.ftl-no-cjit-validate-sampling-profiler
stress/super-property-access.js.dfg-eager
stress/varargs-inlined-simple-exit-aliasing-weird.js.default
ChakraCore.yaml/ChakraCore/test/Regex/BolEol.js.default
stress/varargs-then-slow-call.js.dfg-maximal-flush-validate-no-cjit
stress/super-property-access.js.dfg-eager-no-cjit-validate
stress/put-direct-index-broken-2.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/deltablue-varargs.js.ftl-no-cjit-small-pool
jsc-layout-tests.yaml/js/script-tests/function-apply.js.layout-dfg-eager-no-cjit
stress/varargs-inlined-simple-exit-aliasing-weird-reversed-args.js.no-cjit-validate-phases
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-no-inline-validate

[Build Bot]

Comment on attachment 318711 [details]
Patch

Attachment 318711 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4357039

New failing tests:
webgl/1.0.2/conformance/more/functions/uniformi.html
webgl/1.0.2/conformance/more/functions/deleteBufferBadArgs.html
js/slow-stress/call-spread.html
webgl/1.0.2/conformance/more/functions/uniformf.html
webgl/1.0.2/conformance/more/functions/uniformMatrixBadArgs.html
webgl/1.0.2/conformance/more/functions/uniformMatrix.html
webgl/1.0.2/conformance/more/functions/bindBuffer.html
js/slow-stress/variadic-closure-call.html
webgl/1.0.2/conformance/more/functions/bindBufferBadArgs.html
js/function-apply.html
js/array-filter.html
js/slow-stress/new-spread.html
webgl/1.0.2/conformance/more/functions/texImage2DHTMLBadArgs.html
webgl/1.0.2/conformance/more/functions/readPixels.html
jquery/event.html
js/basic-spread.html

[Build Bot]

Created attachment 318719 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

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

[Build Bot]

Comment on attachment 318711 [details]
Patch

Attachment 318711 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4357209

New failing tests:
webgl/1.0.2/conformance/more/functions/uniformi.html
webgl/1.0.2/conformance/more/functions/deleteBufferBadArgs.html
js/slow-stress/call-spread.html
webgl/1.0.2/conformance/more/functions/uniformf.html
webgl/1.0.2/conformance/more/functions/uniformMatrixBadArgs.html
webgl/1.0.2/conformance/more/functions/uniformMatrix.html
webgl/1.0.2/conformance/more/functions/bindBuffer.html
js/slow-stress/variadic-closure-call.html
webgl/1.0.2/conformance/more/functions/bindBufferBadArgs.html
js/function-apply.html
js/array-filter.html
js/slow-stress/new-spread.html
webgl/1.0.2/conformance/more/functions/texImage2DHTMLBadArgs.html
webgl/1.0.2/conformance/more/functions/readPixels.html
jquery/event.html
js/basic-spread.html

[Build Bot]

Created attachment 318721 [details]
Archive of layout-test-results from ews105 for mac-elcapitan-wk2

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

[Caio Lima]

Created attachment 318725 [details]
Patch

[Caio Lima]

Created attachment 320413 [details]
Patch

[Caio Lima]

Ping Review

[Saam Barati]

Comment on attachment 320413 [details]
Patch

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

> Source/JavaScriptCore/interpreter/Interpreter.cpp:243
> +    unsigned length = sizeOfVarargs(callFrame, arguments, firstVarArgOffset, &overflows32Bits);

how is the add below on "length" safe? What if toLength returns UINT_MAX, length+1 will be zero b/c of overflow.

[Saam Barati]

Comment on attachment 320413 [details]
Patch

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

> Source/JavaScriptCore/interpreter/Interpreter.cpp:211
> +        uint64_t length64 = static_cast<uint64_t>(toLength(callFrame, jsCast<JSObject*>(cell)));
> +        if (overflows32Bit)
> +            *overflows32Bit = length64 != static_cast<unsigned>(length64);

I think we already have code that'll do what you want here. Perhaps some variant of Checked?

>> Source/JavaScriptCore/interpreter/Interpreter.cpp:243
>> +    unsigned length = sizeOfVarargs(callFrame, arguments, firstVarArgOffset, &overflows32Bits);
> 
> how is the add below on "length" safe? What if toLength returns UINT_MAX, length+1 will be zero b/c of overflow.

Oh I see, there is a maxArguments check below.

[Caio Lima]

Created attachment 321099 [details]
Patch updated

Updated Patch to use Checked. Let's see what bot builds think about it. Also, requiring review agin, since it's a different approach.

[Build Bot]

Comment on attachment 321099 [details]
Patch updated

Attachment 321099 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/4583934

New failing tests:
ChakraCore.yaml/ChakraCore/test/Function/apply3.js.default

[Caio Lima]

Created attachment 321153 [details]
Patch Updated

Fixing crashes

[Build Bot]

Comment on attachment 321153 [details]
Patch Updated

Attachment 321153 [details] did not pass jsc-ews (mac):
Output: http://webkit-queues.webkit.org/results/4589181

New failing tests:
stress/load-varargs-then-inlined-call.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-then-inlined-call.js.ftl-no-cjit-b3o1
stress/weird-put-stack-varargs.js.default
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-eager-no-cjit-b3o1
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.dfg-eager-no-cjit-validate
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-eager-no-cjit-b3o1
stress/varargs-closure-inlined-exit-strict-mode.js.no-llint
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.no-ftl
stress/call-apply-exponential-bytecode-size.js.ftl-no-cjit-small-pool
stress/proxy-call.js.no-cjit-validate-phases
stress/phantom-spread-forward-varargs.js.ftl-eager
stress/phantom-new-array-with-spread-osr-exit.js.ftl-no-cjit-no-put-stack-validate
stress/varargs-closure-inlined-exit-strict-mode.js.no-cjit-validate-phases
microbenchmarks/rest-parameter-allocation-elimination.js.no-cjit-validate-phases
stress/phantom-spread-osr-exit.js.ftl-eager
stress/call-apply-exponential-bytecode-size.js.ftl-eager
stress/tailCallForwardArguments.js.ftl-eager-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/call-spread-call.js.ftl-eager-no-cjit-b3o1
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.dfg-eager
stress/call-apply-exponential-bytecode-size.js.ftl-eager-no-cjit-b3o1
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-eager
stress/tailCallForwardArguments.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/varargs-strict-mode.js.dfg-maximal-flush-validate-no-cjit
stress/phantom-spread-forward-varargs.js.dfg-eager
stress/phantom-spread-forward-varargs.js.default
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-validate-sampling-profiler
stress/proxy-call.js.ftl-no-cjit-validate-sampling-profiler
stress/argument-intrinsic-inlining-with-vararg.js.dfg-eager-no-cjit-validate
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-no-cjit-validate-sampling-profiler
stress/call-apply-exponential-bytecode-size.js.no-ftl
stress/phantom-direct-arguments-clobber-argument-count.js.dfg-maximal-flush-validate-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.ftl-no-cjit-validate-sampling-profiler
stress/phantom-spread-forward-varargs.js.ftl-eager-no-cjit-b3o1
stress/rest-parameter-inlined.js.no-cjit-validate-phases
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.default
stress/tailCallForwardArguments.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-inlined.js.dfg-maximal-flush-validate-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/rest-parameter-allocation-elimination.js.default
stress/phantom-direct-arguments-clobber-argument-count.js.no-ftl
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-inlined.js.dfg-eager
microbenchmarks/call-spread-apply.js.ftl-eager-no-cjit
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-b3o1
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-eager-no-cjit
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.default
microbenchmarks/call-spread-call.js.no-llint
stress/argument-intrinsic-inlining-with-vararg.js.default
stress/load-varargs-then-inlined-call-and-exit.js.default
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-no-cjit-validate-sampling-profiler
stress/get-my-argument-by-val-creates-arguments.js.ftl-eager
stress/call-apply-exponential-bytecode-size.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call-and-exit.js.ftl-eager
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-eager-no-cjit
stress/inline-varargs-get-arguments.js.ftl-no-cjit-b3o1
stress/call-apply-exponential-bytecode-size.js.dfg-maximal-flush-validate-no-cjit
stress/real-forward-varargs-for-inlined-escaped-arguments.js.no-ftl
stress/weird-put-stack-varargs.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-and-exit.js.dfg-eager
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-small-pool
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-no-cjit-b3o1
stress/tailCallForwardArguments.js.no-cjit-collect-continuously
stress/inline-varargs-get-arguments.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call-inlined.js.no-llint
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-no-cjit-validate-sampling-profiler
stress/phantom-new-array-with-spread-osr-exit.js.ftl-no-cjit-small-pool
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/call-spread-apply.js.ftl-eager-no-cjit-b3o1
stress/phantom-direct-arguments-clobber-argument-count.js.dfg-eager
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.default
stress/tailCallForwardArguments.js.ftl-eager-no-cjit
microbenchmarks/call-spread-call.js.default
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.ftl-no-cjit-validate-sampling-profiler
stress/rest-parameter-inlined.js.ftl-eager
stress/real-forward-varargs-for-inlined-escaped-arguments.js.dfg-eager
stress/weird-put-stack-varargs.js.no-cjit-validate-phases
microbenchmarks/call-spread-apply.js.dfg-eager
stress/inline-varargs-get-arguments.js.ftl-eager-no-cjit
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-validate-sampling-profiler
stress/call-apply-exponential-bytecode-size.js.ftl-eager-no-cjit
airjs-tests.yaml/stress-test.js.default
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-eager-no-cjit-b3o1
airjs-tests.yaml/stress-test.js.no-ftl
stress/weird-put-stack-varargs.js.ftl-eager
stress/get-my-argument-by-val-creates-arguments.js.ftl-eager-no-cjit-b3o1
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-eager
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-no-cjit-no-put-stack-validate
stress/varargs-closure-inlined-exit-strict-mode.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/call-spread-call.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call.js.no-llint
stress/load-varargs-then-inlined-call-inlined.js.ftl-no-cjit-b3o1
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-no-cjit-b3o1
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-no-cjit-small-pool
microbenchmarks/call-spread-call.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-varargs-inlined-exit-strict-mode.js.no-cjit-validate-phases
stress/tailCallForwardArguments.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/apply-varargs.js.layout-dfg-eager-no-cjit
stress/load-varargs-then-inlined-call.js.dfg-eager
stress/load-varargs-then-inlined-call.js.ftl-no-cjit-validate-sampling-profiler
jsc-layout-tests.yaml/js/script-tests/apply-varargs.js.layout-ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-and-exit.js.ftl-no-cjit-no-put-stack-validate
stress/phantom-new-array-with-spread-osr-exit.js.ftl-no-cjit-validate-sampling-profiler
microbenchmarks/rest-parameter-allocation-elimination.js.no-llint
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.dfg-eager-no-cjit-validate
stress/proxy-call.js.ftl-no-cjit-small-pool
stress/varargs-varargs-inlined-exit-strict-mode.js.no-ftl
microbenchmarks/call-spread-apply.js.dfg-eager-no-cjit-validate
stress/weird-put-stack-varargs.js.ftl-no-cjit-validate-sampling-profiler
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call.js.no-cjit-validate-phases
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-eager-no-cjit-b3o1
stress/phantom-new-array-with-spread-osr-exit.js.default
stress/phantom-spread-osr-exit.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call-and-exit-strict.js.default
stress/load-varargs-then-inlined-call-exit-in-foo.js.no-cjit-collect-continuously
stress/call-apply-exponential-bytecode-size.js.no-llint
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.no-llint
stress/inline-varargs-get-arguments.js.no-cjit-collect-continuously
microbenchmarks/rest-parameter-allocation-elimination.js.dfg-maximal-flush-validate-no-cjit
stress/phantom-spread-osr-exit.js.ftl-no-cjit-b3o1
stress/proxy-call.js.no-llint
stress/phantom-spread-osr-exit.js.no-llint
stress/argument-intrinsic-inlining-with-vararg.js.no-ftl
stress/tailCallForwardArguments.js.dfg-maximal-flush-validate-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-and-exit-strict.js.no-llint
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-eager
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-eager
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.no-llint
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-small-pool
stress/inline-varargs-get-arguments.js.ftl-eager
stress/phantom-spread-osr-exit.js.no-cjit-collect-continuously
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.dfg-maximal-flush-validate-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.no-llint
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-no-cjit-no-put-stack-validate
microbenchmarks/call-spread-call.js.ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-and-exit.js.dfg-eager-no-cjit-validate
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-inlined.js.no-cjit-validate-phases
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.no-cjit-validate-phases
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-no-cjit-no-put-stack-validate
stress/get-my-argument-by-val-creates-arguments.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-exit-in-foo.js.no-llint
stress/get-my-argument-by-val-creates-arguments.js.no-llint
stress/phantom-new-array-with-spread-osr-exit.js.ftl-eager
stress/phantom-new-array-with-spread-osr-exit.js.no-cjit-validate-phases
stress/argument-intrinsic-inlining-with-vararg.js.ftl-eager-no-cjit
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-eager-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.ftl-eager
stress/proxy-call.js.ftl-eager
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-inlined.js.no-ftl
stress/argument-intrinsic-inlining-with-vararg.js.ftl-no-cjit-no-put-stack-validate
stress/inline-varargs-get-arguments.js.ftl-no-cjit-no-put-stack-validate
stress/varargs-varargs-inlined-exit-strict-mode.js.default
stress/v8-raytrace-strict.js.dfg-maximal-flush-validate-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.dfg-maximal-flush-validate-no-cjit
stress/varargs-varargs-inlined-exit-strict-mode.js.no-llint
microbenchmarks/call-spread-call.js.ftl-no-cjit-no-put-stack-validate
stress/phantom-spread-osr-exit.js.no-cjit-validate-phases
stress/get-my-argument-by-val-creates-arguments.js.no-ftl
airjs-tests.yaml/stress-test.js.ftl-eager-no-cjit
stress/proxy-call.js.ftl-no-cjit-b3o1
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.no-ftl
stress/real-forward-varargs-for-inlined-escaped-arguments.js.no-cjit-validate-phases
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-eager
stress/proxy-call.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call-and-exit.js.ftl-eager-no-cjit-b3o1
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.no-cjit-validate-phases
stress/phantom-new-array-with-spread-osr-exit.js.dfg-eager-no-cjit-validate
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-no-cjit-b3o1
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.no-cjit-collect-continuously
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-eager
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-dfg-eager-no-cjit
stress/rest-parameter-inlined.js.no-llint
microbenchmarks/call-spread-call.js.dfg-eager
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.no-ftl
stress/argument-intrinsic-inlining-with-vararg.js.ftl-eager-no-cjit-b3o1
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-eager-no-cjit
stress/call-apply-exponential-bytecode-size.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call.js.default
stress/phantom-new-array-with-spread-osr-exit.js.dfg-eager
stress/rest-parameter-inlined.js.dfg-eager
microbenchmarks/call-spread-apply.js.ftl-eager
stress/proxy-call.js.ftl-eager-no-cjit
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.no-ftl
stress/phantom-direct-arguments-clobber-argument-count.js.dfg-eager-no-cjit-validate
microbenchmarks/call-spread-apply.js.no-llint
stress/phantom-direct-arguments-clobber-argument-count.js.default
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.no-cjit-collect-continuously
stress/phantom-spread-osr-exit.js.dfg-maximal-flush-validate-no-cjit
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-eager
stress/phantom-spread-osr-exit.js.ftl-eager-no-cjit-b3o1
stress/phantom-spread-forward-varargs.js.dfg-maximal-flush-validate-no-cjit
stress/tailCallForwardArguments.js.ftl-eager
microbenchmarks/rest-parameter-allocation-elimination.js.dfg-eager-no-cjit-validate
stress/varargs-varargs-inlined-exit-strict-mode.js.no-cjit-collect-continuously
stress/load-varargs-then-inlined-call-and-exit-strict.js.dfg-maximal-flush-validate-no-cjit
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-ftl-eager-no-cjit
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-and-exit.js.ftl-no-cjit-validate-sampling-profiler
stress/call-apply-exponential-bytecode-size.js.no-cjit-collect-continuously
stress/load-varargs-then-inlined-call-exit-in-foo.js.no-cjit-validate-phases
stress/phantom-new-array-with-spread-osr-exit.js.ftl-eager-no-cjit
stress/phantom-new-array-with-spread-osr-exit.js.no-cjit-collect-continuously
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.dfg-eager-no-cjit-validate
stress/load-varargs-then-inlined-call-and-exit-strict.js.dfg-eager
stress/load-varargs-then-inlined-call-and-exit-strict.js.no-cjit-collect-continuously
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-no-cjit-no-put-stack-validate
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-eager-no-cjit
stress/real-forward-varargs-for-inlined-escaped-arguments.js.dfg-eager-no-cjit-validate
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.no-cjit-collect-continuously
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-eager
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-validate-sampling-profiler
stress/proxy-call.js.dfg-eager-no-cjit-validate
stress/phantom-spread-osr-exit.js.no-ftl
stress/load-varargs-then-inlined-call.js.ftl-eager-no-cjit
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call-inlined.js.no-cjit-collect-continuously
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-eager
stress/proxy-call.js.ftl-no-cjit-no-put-stack-validate
stress/weird-put-stack-varargs.js.dfg-eager
stress/load-varargs-then-inlined-call-and-exit.js.ftl-no-cjit-b3o1
stress/varargs-varargs-inlined-exit-strict-mode.js.dfg-maximal-flush-validate-no-cjit
stress/phantom-new-array-with-spread-osr-exit.js.dfg-maximal-flush-validate-no-cjit
stress/call-apply-exponential-bytecode-size.js.no-cjit-validate-phases
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-eager-no-cjit-b3o1
stress/phantom-spread-forward-varargs.js.dfg-eager-no-cjit-validate
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-no-cjit-b3o1
stress/argument-intrinsic-inlining-with-vararg.js.no-cjit-validate-phases
stress/load-varargs-then-inlined-call-and-exit-strict.js.no-ftl
stress/load-varargs-then-inlined-call-inlined.js.ftl-eager
stress/phantom-spread-osr-exit.js.ftl-eager-no-cjit
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.dfg-eager
stress/inline-varargs-get-arguments.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-eager-no-cjit
stress/varargs-closure-inlined-exit-strict-mode.js.dfg-eager
stress/load-varargs-then-inlined-call-inlined.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-no-cjit-small-pool
stress/load-varargs-then-inlined-call-inlined.js.default
stress/load-varargs-then-inlined-call-exit-in-foo.js.dfg-eager-no-cjit-validate
microbenchmarks/rest-parameter-allocation-elimination.js.ftl-no-cjit-no-put-stack-validate
stress/call-apply-exponential-bytecode-size.js.dfg-eager-no-cjit-validate
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-eager-no-cjit-b3o1
stress/inline-varargs-get-arguments.js.dfg-eager
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-no-ftl
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.default
stress/varargs-closure-inlined-exit-strict-mode.js.dfg-eager-no-cjit-validate
microbenchmarks/call-spread-apply.js.no-ftl
stress/phantom-spread-forward-varargs.js.no-cjit-collect-continuously
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-eager-no-cjit-b3o1
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-and-exit.js.no-ftl
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-eager
stress/load-varargs-then-inlined-call.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-and-exit.js.dfg-maximal-flush-validate-no-cjit
stress/inline-varargs-get-arguments.js.dfg-eager-no-cjit-validate
stress/rest-parameter-inlined.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-then-inlined-call.js.no-cjit-collect-continuously
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-b3o1
stress/proxy-call.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-eager-no-cjit-b3o1
stress/real-forward-varargs-for-inlined-escaped-arguments.js.no-cjit-collect-continuously
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-no-put-stack-validate
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-ftl-no-cjit
stress/rest-parameter-inlined.js.ftl-no-cjit-validate-sampling-profiler
stress/argument-intrinsic-inlining-with-vararg.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-then-inlined-call.js.no-ftl
stress/get-my-argument-by-val-creates-arguments.js.ftl-eager-no-cjit
stress/varargs-varargs-inlined-exit-strict-mode.js.dfg-eager-no-cjit-validate
stress/argument-intrinsic-inlining-with-vararg.js.no-cjit-collect-continuously
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-then-inlined-call-and-exit-strict.js.dfg-eager-no-cjit-validate
microbenchmarks/call-spread-call.js.ftl-eager
stress/varargs-closure-inlined-exit-strict-mode.js.no-ftl
stress/rest-parameter-inlined.js.no-ftl
stress/inline-varargs-get-arguments.js.no-cjit-validate-phases
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.no-cjit
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.default
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-eager-no-cjit
stress/weird-put-stack-varargs.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call.js.dfg-eager-no-cjit-validate
stress/load-varargs-then-inlined-call-inlined.js.ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-and-exit-strict.js.no-cjit-validate-phases
stress/tailCallForwardArguments.js.dfg-eager
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.no-llint
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-no-cjit-small-pool
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout-no-llint
stress/phantom-new-array-with-spread-osr-exit.js.no-llint
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call.js.ftl-eager-no-cjit-b3o1
stress/weird-put-stack-varargs.js.ftl-eager-no-cjit
stress/phantom-direct-arguments-clobber-argument-count.js.ftl-no-cjit-no-put-stack-validate
stress/phantom-spread-forward-varargs.js.ftl-no-cjit-no-put-stack-validate
stress/proxy-call.js.no-ftl
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.ftl-eager-no-cjit
microbenchmarks/call-spread-call.js.no-ftl
stress/inline-varargs-get-arguments.js.no-ftl
stress/weird-put-stack-varargs.js.no-ftl
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-no-put-stack-validate
stress/tailCallForwardArguments.js.default
microbenchmarks/rest-parameter-allocation-elimination.js.no-ftl
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-eager
stress/phantom-spread-forward-varargs.js.no-cjit-validate-phases
stress/real-forward-varargs-for-inlined-escaped-arguments.js.dfg-maximal-flush-validate-no-cjit
jsc-layout-tests.yaml/js/slow-stress/script-tests/variadic-closure-call.js.ftl-no-cjit-validate-sampling-profiler
stress/inline-varargs-get-arguments.js.dfg-maximal-flush-validate-no-cjit
stress/apply-second-argument-must-be-array-like.js.default
stress/rest-parameter-inlined.js.dfg-eager-no-cjit-validate
stress/get-my-argument-by-val-creates-arguments.js.ftl-no-cjit-no-put-stack-validate
stress/rest-parameter-inlined.js.ftl-no-cjit-b3o1
stress/phantom-spread-forward-varargs.js.no-ftl
stress/load-varargs-then-inlined-call-exit-in-foo.js.no-ftl
microbenchmarks/call-spread-apply.js.ftl-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.default
stress/varargs-varargs-inlined-exit-strict-mode.js.dfg-eager
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.dfg-eager
airjs-tests.yaml/stress-test.js.ftl-no-cjit
stress/phantom-new-array-with-spread-osr-exit.js.no-ftl
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-exit-in-foo.js.dfg-maximal-flush-validate-no-cjit
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.dfg-eager-no-cjit-validate
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-no-cjit-b3o1
microbenchmarks/rest-parameter-allocation-elimination.js.no-cjit-collect-continuously
stress/phantom-spread-osr-exit.js.ftl-no-cjit-small-pool
stress/get-my-argument-by-val-creates-arguments.js.dfg-eager
stress/real-forward-varargs-for-inlined-escaped-arguments.js.default
stress/phantom-direct-arguments-clobber-argument-count.js.no-cjit-collect-continuously
stress/load-varargs-then-inlined-call-inlined.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-and-exit.js.ftl-no-cjit-small-pool
stress/phantom-direct-arguments-clobber-argument-count.js.no-cjit-validate-phases
stress/call-apply-exponential-bytecode-size.js.default
microbenchmarks/call-spread-call.js.ftl-no-cjit-small-pool
stress/call-apply-exponential-bytecode-size.js.dfg-eager
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-no-cjit-validate-sampling-profiler
stress/varargs-closure-inlined-exit-strict-mode.js.no-cjit-collect-continuously
microbenchmarks/rest-parameter-allocation-elimination.js.dfg-eager
microbenchmarks/call-spread-call.js.ftl-no-cjit-validate-sampling-profiler
stress/rest-parameter-inlined.js.default
stress/weird-put-stack-varargs.js.dfg-maximal-flush-validate-no-cjit
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.no-ftl
stress/load-varargs-then-inlined-call-inlined.js.ftl-no-cjit-validate-sampling-profiler
stress/weird-put-stack-varargs.js.ftl-no-cjit-no-put-stack-validate
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-eager-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.dfg-eager
stress/load-varargs-then-inlined-call-and-exit.js.no-llint
stress/proxy-call.js.no-cjit-collect-continuously
stress/phantom-new-array-with-spread-osr-exit.js.ftl-no-cjit-b3o1
stress/real-forward-varargs-for-inlined-escaped-arguments.js.no-llint
stress/phantom-spread-forward-varargs.js.ftl-eager-no-cjit
stress/rest-parameter-inlined.js.ftl-no-cjit-no-put-stack-validate
stress/load-varargs-then-inlined-call-exit-in-foo.js.dfg-eager
microbenchmarks/call-spread-apply.js.no-cjit-validate-phases
stress/proxy-call.js.dfg-eager
stress/get-my-argument-by-val-creates-arguments.js.ftl-no-cjit-small-pool
stress/varargs-varargs-inlined-exit-strict-mode.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call-and-exit.js.no-cjit-collect-continuously
stress/real-forward-varargs-for-inlined-escaped-arguments.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call.js.ftl-no-cjit-small-pool
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.dfg-eager
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.no-llint
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-inlined.js.ftl-no-cjit-small-pool
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/call-spread-call.js.no-cjit-validate-phases
stress/argument-intrinsic-inlining-with-vararg-with-enough-arguments.js.no-cjit-validate-phases
stress/phantom-new-array-with-spread-osr-exit.js.ftl-eager-no-cjit-b3o1
microbenchmarks/call-spread-call.js.no-cjit-collect-continuously
stress/weird-put-stack-varargs.js.ftl-no-cjit-b3o1
stress/weird-put-stack-varargs.js.no-llint
stress/load-varargs-then-inlined-call-and-exit-strict.js.ftl-no-cjit-validate-sampling-profiler
stress/phantom-spread-osr-exit.js.ftl-no-cjit-no-put-stack-validate
stress/call-apply-exponential-bytecode-size.js.ftl-no-cjit-no-put-stack-validate
stress/tailCallForwardArguments.js.no-cjit-validate-phases
stress/load-varargs-then-inlined-call-exit-in-foo.js.ftl-no-cjit-validate-sampling-profiler
stress/load-varargs-then-inlined-call-exit-in-foo.js.default
stress/rest-parameter-inlined.js.ftl-no-cjit-small-pool
stress/weird-put-stack-varargs.js.no-cjit-collect-continuously
stress/get-my-argument-by-val-creates-arguments.js.dfg-eager-no-cjit-validate
jsc-layout-tests.yaml/js/script-tests/regress-150220.js.layout
stress/rest-parameter-inlined.js.ftl-eager-no-cjit-b3o1
microbenchmarks/call-spread-apply.js.ftl-no-cjit-no-put-stack-validate
stress/phantom-spread-osr-exit.js.default
stress/inline-varargs-get-arguments.js.no-llint
stress/load-varargs-then-inlined-call.js.ftl-eager
microbenchmarks/call-spread-apply.js.no-cjit-collect-continuously
stress/rest-parameter-inlined.js.ftl-eager-no-cjit
stress/argument-intrinsic-inlining-with-vararg.js.ftl-no-cjit-small-pool
stress/get-my-argument-by-val-creates-arguments.js.no-cjit-collect-continuously
stress/load-varargs-on-new-array-with-spread-convert-to-static-loads.js.no-cjit-collect-continuously
stress/varargs-closure-inlined-exit-strict-mode.js.ftl-eager-no-cjit-b3o1
stress/tailCallForwardArguments.js.ftl-no-cjit-b3o1
stress/tailCallForwardArguments.js.no-llint
stress/weird-put-stack-varargs.js.dfg-eager-no-cjit-validate
stress/rest-parameter-inlined.js.no-cjit-collect-continuously
stress/inline-varargs-get-arguments.js.default
stress/phantom-direct-arguments-clobber-argument-count.js.no-llint
stress/phantom-spread-osr-exit.js.dfg-eager-no-cjit-validate
stress/inline-varargs-get-arguments.js.ftl-no-cjit-validate-sampling-profiler
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.no-cjit-validate-phases
stress/phantom-spread-osr-exit.js.dfg-eager
microbenchmarks/call-spread-call.js.dfg-eager-no-cjit-validate
stress/load-varargs-then-inlined-call-and-exit.js.no-cjit-validate-phases
stress/proxy-revoke.js.ftl-eager-no-cjit-b3o1
stress/load-varargs-then-inlined-call-and-exit.js.ftl-eager-no-cjit
stress/get-my-argument-by-val-creates-arguments.js.default
stress/phantom-spread-forward-varargs.js.no-llint
stress/tailCallForwardArguments.js.dfg-eager-no-cjit-validate
stress/get-argument-by-val-in-inlined-varargs-call-out-of-bounds.js.ftl-no-cjit-b3o1
stress/load-varargs-then-inlined-call-inlined.js.dfg-eager-no-cjit-validate
stress/varargs-closure-inlined-exit-strict-mode.js.default
stress/get-argument-by-val-safe-in-inlined-varargs-call-out-of-bounds.js.no-cjit-validate-phases

[Build Bot]

Comment on attachment 321153 [details]
Patch Updated

Attachment 321153 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/4589245

New failing tests:
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-dataChange.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-insertData.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-appendData.html
js/slow-stress/variadic-closure-call.html
js/regress-150220.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-deleteData.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-replaceData.html

[Build Bot]

Created attachment 321160 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

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

[Build Bot]

Comment on attachment 321153 [details]
Patch Updated

Attachment 321153 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/4589295

New failing tests:
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-dataChange.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-insertData.html
imported/w3c/web-platform-tests/css/css-shapes-1/shape-outside/values/shape-outside-ellipse-004.html
js/slow-stress/variadic-closure-call.html
js/regress-150220.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-replaceData.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-deleteData.html

[Build Bot]

Created attachment 321163 [details]
Archive of layout-test-results from ews104 for mac-elcapitan-wk2

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

[Caio Lima]

Created attachment 321165 [details]
Patch Updated

Fixing segfault

[Build Bot]

Comment on attachment 321165 [details]
Patch Updated

Attachment 321165 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/4589986

New failing tests:
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-dataChange.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-appendData.html
js/slow-stress/variadic-closure-call.html
css3/shapes/shape-outside/values/shape-outside-ellipse-004.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-replaceData.html
imported/w3c/web-platform-tests/dom/ranges/Range-mutations-deleteData.html

[Build Bot]

Created attachment 321175 [details]
Archive of layout-test-results from ews114 for mac-elcapitan

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

[Caio Lima]

Created attachment 321176 [details]
Patch

[Darin Adler]

Comment on attachment 321176 [details]
Patch

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

> Source/JavaScriptCore/interpreter/Interpreter.cpp:212
> -        length = toLength(callFrame, jsCast<JSObject*>(cell));
> +        WTF::CheckedUint32 checkedLength(static_cast<uint64_t>(toLength(callFrame, jsCast<JSObject*>(cell))));
> +        auto lengthGetResult = checkedLength.safeGet(length);
> +        if (overflows32Bits)
> +            *overflows32Bits = lengthGetResult == WTF::CheckedState::DidOverflow;

I have a simpler fix to suggest. If we clamp the value to fit in unsigned, it’s fine for this function return the maximum unsigned value; that will get us the same stack overflow error that a special boolean gets us. So the fix can be just this:

    length = clampToUnsigned(toLength(callFrame, jsCast<JSObject*>(cell)));

What’s nice about this solution is that it just adds one more function call, and there is then no need for CheckedUInt32, uint64_t, or for changes outside this function.

[Caio Lima]

(In reply to Darin Adler from comment #29)
> Comment on attachment 321176 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=321176&action=review
> 
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:212
> > -        length = toLength(callFrame, jsCast<JSObject*>(cell));
> > +        WTF::CheckedUint32 checkedLength(static_cast<uint64_t>(toLength(callFrame, jsCast<JSObject*>(cell))));
> > +        auto lengthGetResult = checkedLength.safeGet(length);
> > +        if (overflows32Bits)
> > +            *overflows32Bits = lengthGetResult == WTF::CheckedState::DidOverflow;
> 
> I have a simpler fix to suggest. If we clamp the value to fit in unsigned,
> it’s fine for this function return the maximum unsigned value; that will get
> us the same stack overflow error that a special boolean gets us. So the fix
> can be just this:
> 
>     length = clampToUnsigned(toLength(callFrame, jsCast<JSObject*>(cell)));
> 
> What’s nice about this solution is that it just adds one more function call,
> and there is then no need for CheckedUInt32, uint64_t, or for changes
> outside this function.

I think you are right here. Thanks!

[Caio Lima]

Created attachment 322365 [details]
Patch

New simpler solution

[WebKit Commit Bot]

Comment on attachment 322365 [details]
Patch

Clearing flags on attachment: 322365

Committed r222724: <http://trac.webkit.org/changeset/222724>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/34771343>