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.
Created attachment 318315 [details] Patch
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.
Created attachment 318318 [details] Patch
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?
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.
Created attachment 318711 [details] Patch
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
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
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
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
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
Created attachment 318725 [details] Patch
Created attachment 320413 [details] Patch
Ping Review
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.
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.
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.
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
Created attachment 321153 [details] Patch Updated Fixing crashes
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
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
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
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
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
Created attachment 321165 [details] Patch Updated Fixing segfault
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
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
Created attachment 321176 [details] Patch
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.
(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!
Created attachment 322365 [details] Patch New simpler solution
Comment on attachment 322365 [details] Patch Clearing flags on attachment: 322365 Committed r222724: <http://trac.webkit.org/changeset/222724>
All reviewed patches have been landed. Closing bug.
<rdar://problem/34771343>