Bug 183458 - Split DirectArguments into JSValueOOB and JSValueStrict parts
Summary: Split DirectArguments into JSValueOOB and JSValueStrict parts
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords: InRadar
Depends on:
Blocks: 182858
  Show dependency treegraph
 
Reported: 2018-03-08 09:38 PST by Filip Pizlo
Modified: 2018-03-12 13:20 PDT (History)
13 users (show)

See Also:


Attachments
work in progress (14.79 KB, patch)
2018-03-08 09:51 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (31.82 KB, patch)
2018-03-08 13:26 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (45.50 KB, patch)
2018-03-08 14:03 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
maybe it's done (53.02 KB, patch)
2018-03-08 14:24 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixing build issues (56.99 KB, patch)
2018-03-08 15:33 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (57.18 KB, patch)
2018-03-08 16:06 PST, Filip Pizlo
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (2.79 MB, application/zip)
2018-03-08 17:52 PST, EWS Watchlist
no flags Details
more (64.15 KB, patch)
2018-03-09 12:04 PST, Filip Pizlo
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews206 for win-future (10.33 MB, application/zip)
2018-03-09 14:29 PST, EWS Watchlist
no flags Details
more (66.38 KB, patch)
2018-03-09 15:42 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (66.69 KB, patch)
2018-03-09 19:27 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (66.69 KB, patch)
2018-03-09 19:31 PST, Filip Pizlo
ysuzuki: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2018-03-08 09:38:14 PST
Patch forthcoming.
Comment 1 Filip Pizlo 2018-03-08 09:51:50 PST
Created attachment 335312 [details]
work in progress
Comment 2 Filip Pizlo 2018-03-08 12:32:18 PST
The DFG changes in this patch will break 32-bit DFG because of register starvation.
Comment 3 Filip Pizlo 2018-03-08 13:26:17 PST
Created attachment 335332 [details]
more
Comment 4 Filip Pizlo 2018-03-08 14:03:19 PST
Created attachment 335342 [details]
more
Comment 5 Filip Pizlo 2018-03-08 14:24:02 PST
Created attachment 335345 [details]
maybe it's done
Comment 6 Filip Pizlo 2018-03-08 15:33:00 PST
Created attachment 335355 [details]
fixing build issues
Comment 7 Michael Catanzaro 2018-03-08 16:04:16 PST
(In reply to Filip Pizlo from comment #2)
> The DFG changes in this patch will break 32-bit DFG because of register
> starvation.

Guillaume will be the best person to CC. Thanks!
Comment 8 Filip Pizlo 2018-03-08 16:06:56 PST
Created attachment 335362 [details]
more
Comment 9 EWS Watchlist 2018-03-08 16:08:24 PST
Attachment 335362 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:121:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:46:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:48:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5186:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 18 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 EWS Watchlist 2018-03-08 17:39:12 PST
Comment on attachment 335362 [details]
more

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

New failing tests:
stress/arguments-custom-properties-gc.js.dfg-eager
stress/typedarray-sort.js.dfg-eager
stress/typedarray-some.js.dfg-eager-no-cjit-validate
stress/typedarray-map.js.ftl-eager
stress/get-stack-mapping.js.dfg-eager
stress/arguments-custom-properties-gc.js.no-cjit-collect-continuously
stress/typedarray-findIndex.js.ftl-eager-no-cjit
stress/typedarray-map.js.dfg-eager
stress/async-arrow-functions-lexical-arguments-binding.js.dfg-eager-no-cjit-validate
stress/arguments-bizarre-behavior.js.ftl-eager-no-cjit
stress/eval-func-decl-block-with-var-and-remove.js.dfg-eager-no-cjit-validate
stress/typedarray-findIndex.js.no-cjit-collect-continuously
microbenchmarks/call-spread-apply.js.dfg-eager-no-cjit-validate
stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager
microbenchmarks/call-spread-apply.js.dfg-eager
stress/generator-arguments.js.no-cjit-collect-continuously
stress/regress-170661.js.ftl-eager-no-cjit
microbenchmarks/v8-raytrace-with-empty-try-catch.js.no-cjit-collect-continuously
stress/typedarray-forEach.js.ftl-eager-no-cjit
stress/arrowfunction-lexical-bind-this-8.js.ftl-eager-no-cjit
stress/arguments-interference-cfg.js.ftl-eager-no-cjit
stress/rest-parameter-inlined.js.dfg-eager-no-cjit-validate
v8-v6/v8-raytrace.js.ftl-eager
stress/typedarray-fill.js.dfg-eager
stress/arguments-length-always-dont-enum.js.ftl-eager-no-cjit
stress/sorting-boolean-result-comparator.js.dfg-eager
stress/typedarray-reduce.js.dfg-eager-no-cjit-validate
stress/typedarray-subarray.js.ftl-eager
stress/typedarray-findIndex.js.dfg-eager
stress/reflect-set.js.ftl-eager-no-cjit
stress/tagged-template-tdz.js.ftl-eager
stress/typedarray-fill.js.ftl-eager
stress/async-iteration-yield-star.js.ftl-eager-no-cjit
stress/typedarray-copyWithin.js.dfg-eager-no-cjit-validate
stress/string-raw.js.dfg-eager-no-cjit-validate
stress/get-stack-mapping.js.no-cjit-collect-continuously
stress/typedarray-find.js.dfg-eager-no-cjit-validate
stress/async-iteration-yield-star.js.no-cjit-collect-continuously
stress/typedarray-copyWithin.js.ftl-eager
stress/typedarray-slice.js.ftl-eager
stress/string-raw.js.ftl-eager-no-cjit
stress/string-raw.js.no-cjit-collect-continuously
stress/typedarray-map.js.ftl-eager-no-cjit
stress/async-iteration-yield-promise.js.no-cjit-collect-continuously
stress/typedarray-sort.js.dfg-eager-no-cjit-validate
stress/arguments-bizarre-behaviour-disable-enumerability.js.dfg-eager-no-cjit-validate
stress/regress-170661.js.dfg-eager-no-cjit-validate
stress/string-raw.js.dfg-eager
stress/dfg-create-arguments-inline-alloc.js.dfg-eager-no-cjit-validate
stress/sorting-boolean-result-comparator.js.no-cjit-collect-continuously
stress/arguments-length-always-dont-enum.js.ftl-eager
stress/arrowfunction-lexical-bind-arguments-non-strict-1.js.dfg-eager-no-cjit-validate
stress/typedarray-set.js.ftl-eager-no-cjit
stress/generator-arguments.js.ftl-eager-no-cjit
stress/async-arrow-functions-lexical-arguments-binding.js.ftl-eager-no-cjit
stress/typedarray-filter.js.no-cjit-collect-continuously
microbenchmarks/call-spread-apply.js.ftl-eager
stress/typedarray-indexOf.js.ftl-eager-no-cjit
stress/arguments-interference-cfg.js.no-cjit-collect-continuously
stress/promise-finally.js.ftl-eager-no-cjit
stress/spread-calling.js.no-cjit-collect-continuously
stress/typedarray-subarray.js.ftl-eager-no-cjit
stress/typedarray-filter.js.ftl-eager
stress/arrowfunction-lexical-bind-this-2.js.dfg-eager
stress/typedarray-lastIndexOf.js.ftl-eager-no-cjit
stress/typedarray-forEach.js.dfg-eager-no-cjit-validate
stress/typedarray-sort.js.ftl-eager-no-cjit
microbenchmarks/call-spread-call.js.dfg-eager-no-cjit-validate
stress/arguments-copy-register-array-backing-store.js.ftl-eager-no-cjit
stress/direct-arguments-osr-entry.js.dfg-eager-no-cjit-validate
stress/typedarray-fill.js.ftl-eager-no-cjit
stress/typedarray-slice.js.dfg-eager-no-cjit-validate
stress/spread-calling.js.ftl-eager-no-cjit
stress/eval-func-decl-block-with-var-and-remove.js.no-cjit-collect-continuously
v8-v6/v8-earley-boyer.js.dfg-eager-no-cjit-validate
stress/typedarray-reduce.js.ftl-eager-no-cjit
stress/typedarray-reduce.js.ftl-eager
microbenchmarks/call-spread-call.js.ftl-eager-no-cjit
stress/load-varargs-then-inlined-call-and-exit.js.dfg-eager-no-cjit-validate
stress/typedarray-lastIndexOf.js.dfg-eager
stress/direct-arguments-osr-entry.js.ftl-eager
stress/typedarray-forEach.js.ftl-eager
microbenchmarks/deltablue-varargs.js.dfg-eager
stress/async-iteration-yield-star.js.dfg-eager
v8-v6/v8-earley-boyer.js.ftl-eager-no-cjit
v8-v6/v8-earley-boyer.js.ftl-eager
stress/load-varargs-then-inlined-call-inlined.js.dfg-eager
stress/typedarray-find.js.ftl-eager
stress/async-iteration-async-from-sync.js.dfg-eager-no-cjit-validate
microbenchmarks/v8-raytrace-with-empty-try-catch.js.ftl-eager-no-cjit
stress/typedarray-every.js.dfg-eager
stress/async-iteration-async-from-sync.js.ftl-eager
stress/async-iteration-async-from-sync.js.ftl-eager-no-cjit
stress/typedarray-indexOf.js.dfg-eager
stress/arguments-length-always-dont-enum.js.dfg-eager
stress/tagged-template-tdz.js.no-cjit-collect-continuously
stress/arguments-custom-properties-gc.js.ftl-eager-no-cjit
stress/typedarray-set.js.dfg-eager
stress/typedarray-subarray.js.no-cjit-collect-continuously
stress/typedarray-every.js.ftl-eager-no-cjit
stress/dfg-create-arguments-inline-alloc.js.ftl-eager-no-cjit
stress/const-not-strict-mode.js.ftl-eager-no-cjit
microbenchmarks/v8-raytrace-with-empty-try-catch.js.ftl-eager
stress/arguments-copy-register-array-backing-store.js.dfg-eager-no-cjit-validate
stress/async-iteration-yield-star.js.dfg-eager-no-cjit-validate
stress/tagged-template-tdz.js.dfg-eager-no-cjit-validate
stress/arguments-interference-cfg.js.dfg-eager-no-cjit-validate
stress/async-iteration-yield-promise.js.dfg-eager
stress/typedarray-filter.js.ftl-eager-no-cjit
stress/typedarray-map.js.dfg-eager-no-cjit-validate
stress/typedarray-copyWithin.js.no-cjit-collect-continuously
stress/typedarray-slice.js.no-cjit-collect-continuously
stress/const-not-strict-mode.js.dfg-eager
stress/async-iteration-yield-promise.js.ftl-eager-no-cjit
stress/typedarray-some.js.ftl-eager-no-cjit
stress/regress-170661.js.ftl-eager
stress/eval-func-decl-block-with-var-and-remove.js.ftl-eager
stress/typedarray-forEach.js.dfg-eager
stress/typedarray-some.js.no-cjit-collect-continuously
stress/arguments-length-always-dont-enum.js.dfg-eager-no-cjit-validate
stress/typedarray-forEach.js.no-cjit-collect-continuously
stress/arguments-copy-register-array-backing-store.js.dfg-eager
stress/regress-170661.js.dfg-eager
stress/typedarray-findIndex.js.ftl-eager
stress/eval-func-decl-block-with-var-and-remove.js.dfg-eager
stress/eval-func-decl-block-with-var-sinthesize.js.no-cjit-collect-continuously
stress/typedarray-find.js.dfg-eager
stress/async-arrow-functions-lexical-arguments-binding.js.ftl-eager
stress/const-not-strict-mode.js.no-cjit-collect-continuously
stress/async-iteration-yield-star-interface.js.no-cjit-collect-continuously
stress/tailCallForwardArguments.js.no-cjit-collect-continuously
stress/const-not-strict-mode.js.dfg-eager-no-cjit-validate
stress/async-await-syntax.js.ftl-eager
stress/arguments-custom-properties-gc.js.ftl-eager
stress/typedarray-lastIndexOf.js.dfg-eager-no-cjit-validate
stress/promise-finally.js.no-cjit-collect-continuously
stress/typedarray-find.js.ftl-eager-no-cjit
stress/arguments-bizarre-behaviour-disable-enumerability.js.dfg-eager
stress/typedarray-set.js.dfg-eager-no-cjit-validate
stress/async-await-syntax.js.ftl-eager-no-cjit
stress/typedarray-sort.js.no-cjit-collect-continuously
stress/tagged-template-tdz.js.dfg-eager
ChakraCore.yaml/ChakraCore/test/Miscellaneous/HasOnlyWritableDataPropertiesCache.js.default
stress/get-stack-mapping-with-dead-get-stack.js.no-cjit-collect-continuously
stress/typedarray-fill.js.no-cjit-collect-continuously
stress/tagged-template-tdz.js.ftl-eager-no-cjit
stress/typedarray-reduceRight.js.ftl-eager
stress/typedarray-reduceRight.js.dfg-eager
stress/typedarray-reduceRight.js.no-cjit-collect-continuously
stress/typedarray-slice.js.ftl-eager-no-cjit
stress/spread-calling.js.dfg-eager-no-cjit-validate
stress/eval-func-decl-block-with-var-and-remove.js.ftl-eager-no-cjit
stress/arguments-interference.js.no-cjit-collect-continuously
stress/typedarray-copyWithin.js.dfg-eager
stress/eval-func-decl-block-with-var-sinthesize.js.ftl-eager-no-cjit
microbenchmarks/deltablue-varargs.js.dfg-eager-no-cjit-validate
stress/sorting-boolean-result-comparator.js.ftl-eager-no-cjit
stress/get-stack-mapping.js.dfg-eager-no-cjit-validate
stress/arguments-bizarre-behavior.js.ftl-eager
microbenchmarks/v8-raytrace-with-empty-try-catch.js.dfg-eager-no-cjit-validate
stress/arguments-copy-register-array-backing-store.js.no-cjit-collect-continuously
stress/eval-func-decl-in-eval-within-block-with-let.js.ftl-eager
stress/typedarray-every.js.no-cjit-collect-continuously
stress/arguments-bizarre-behaviour-disable-enumerability.js.ftl-eager
stress/async-await-syntax.js.dfg-eager-no-cjit-validate
microbenchmarks/deltablue-varargs.js.ftl-eager-no-cjit
stress/arrowfunction-lexical-bind-this-2.js.dfg-eager-no-cjit-validate
microbenchmarks/call-spread-apply.js.no-cjit-collect-continuously
stress/typedarray-find.js.no-cjit-collect-continuously
stress/typedarray-indexOf.js.no-cjit-collect-continuously
microbenchmarks/call-spread-call.js.no-cjit-collect-continuously
stress/rest-parameter-inlined.js.ftl-eager-no-cjit
stress/typedarray-set.js.no-cjit-collect-continuously
stress/promise-finally.js.dfg-eager-no-cjit-validate
stress/typedarray-indexOf.js.dfg-eager-no-cjit-validate
stress/typedarray-sort.js.ftl-eager
v8-v6/v8-earley-boyer.js.no-cjit-collect-continuously
stress/typedarray-lastIndexOf.js.no-cjit-collect-continuously
stress/get-stack-mapping-with-dead-get-stack.js.dfg-eager
microbenchmarks/call-spread-call.js.dfg-eager
stress/typedarray-lastIndexOf.js.ftl-eager
microbenchmarks/call-spread-call.js.ftl-eager
stress/typedarray-reduceRight.js.ftl-eager-no-cjit
stress/typedarray-map.js.no-cjit-collect-continuously
stress/async-iteration-yield-star-interface.js.ftl-eager
stress/async-iteration-yield-promise.js.ftl-eager
stress/async-iteration-yield-star-interface.js.dfg-eager-no-cjit-validate
stress/typedarray-every.js.dfg-eager-no-cjit-validate
stress/typedarray-findIndex.js.dfg-eager-no-cjit-validate
stress/typedarray-subarray.js.dfg-eager
stress/sorting-boolean-result-comparator.js.ftl-eager
stress/regress-170661.js.no-cjit-collect-continuously
microbenchmarks/deltablue-varargs.js.no-cjit-collect-continuously
stress/get-my-argument-by-val-creates-arguments.js.dfg-eager-no-cjit-validate
stress/typedarray-reduce.js.no-cjit-collect-continuously
stress/typedarray-subarray.js.dfg-eager-no-cjit-validate
stress/typedarray-filter.js.dfg-eager
stress/typedarray-filter.js.dfg-eager-no-cjit-validate
stress/spread-calling.js.ftl-eager
stress/typedarray-slice.js.dfg-eager
stress/spread-calling.js.dfg-eager
stress/arguments-interference.js.dfg-eager-no-cjit-validate
microbenchmarks/v8-raytrace-with-empty-try-catch.js.dfg-eager
microbenchmarks/v8-raytrace-with-try-catch.js.dfg-eager
stress/get-stack-mapping-with-dead-get-stack.js.dfg-eager-no-cjit-validate
stress/typedarray-indexOf.js.ftl-eager
v8-v6/v8-earley-boyer.js.dfg-eager
stress/tailCallForwardArguments.js.dfg-eager
stress/async-iteration-async-from-sync.js.dfg-eager
stress/typedarray-every.js.ftl-eager
stress/typedarray-reduceRight.js.dfg-eager-no-cjit-validate
stress/typedarray-copyWithin.js.ftl-eager-no-cjit
stress/arrowfunction-lexical-bind-this-8.js.ftl-eager
stress/typedarray-reduce.js.dfg-eager
stress/direct-arguments-osr-entry.js.no-cjit-collect-continuously
stress/typedarray-set.js.ftl-eager
stress/ftl-get-my-argument-by-val-inlined-and-not-inlined.js.no-cjit-collect-continuously
stress/typedarray-some.js.ftl-eager
stress/sorting-boolean-result-comparator.js.dfg-eager-no-cjit-validate
stress/arguments-custom-properties-gc.js.dfg-eager-no-cjit-validate
stress/arrowfunction-lexical-bind-this-2.js.no-cjit-collect-continuously
stress/typedarray-some.js.dfg-eager
stress/generator-arguments.js.ftl-eager
stress/load-varargs-then-inlined-call.js.no-cjit-collect-continuously
stress/tailCallForwardArguments.js.dfg-eager-no-cjit-validate
stress/typedarray-fill.js.dfg-eager-no-cjit-validate
Comment 11 EWS Watchlist 2018-03-08 17:52:52 PST
Comment on attachment 335362 [details]
more

Attachment 335362 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6866080

New failing tests:
perf/typing-at-end-of-line.html
perf/htmlcollection-last-item.html
imported/w3c/web-platform-tests/WebCryptoAPI/sign_verify/ecdsa.worker.html
perf/svg-path-appenditem.html
perf/array-reverse.html
imported/w3c/web-platform-tests/streams/readable-streams/default-reader.serviceworker.https.html
perf/set-attribute.html
perf/htmlcollection-backwards-iteration.html
perf/table-rows-length-caching.html
Comment 12 EWS Watchlist 2018-03-08 17:52:53 PST
Created attachment 335375 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6
Comment 13 Filip Pizlo 2018-03-09 12:04:12 PST
(In reply to Michael Catanzaro from comment #7)
> (In reply to Filip Pizlo from comment #2)
> > The DFG changes in this patch will break 32-bit DFG because of register
> > starvation.
> 
> Guillaume will be the best person to CC. Thanks!

I ended up doing the 32-bit support.  We're not going to kill 32-bit DFG yet, as much as I wish we could.
Comment 14 Filip Pizlo 2018-03-09 12:04:42 PST
Created attachment 335445 [details]
more

I think it passes 64-bit tests.  It's perf neutral there.  Still working on 32-bit.
Comment 15 EWS Watchlist 2018-03-09 12:18:27 PST
Attachment 335445 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:121:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:46:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:48:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5189:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 EWS Watchlist 2018-03-09 14:29:33 PST
Comment on attachment 335445 [details]
more

Attachment 335445 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6881506

Number of test failures exceeded the failure limit.
Comment 17 EWS Watchlist 2018-03-09 14:29:42 PST
Created attachment 335464 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 18 Filip Pizlo 2018-03-09 15:42:14 PST
Created attachment 335481 [details]
more

getting 32-bit to work
Comment 19 EWS Watchlist 2018-03-09 15:45:23 PST
Attachment 335481 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:121:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:46:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:48:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5189:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 Filip Pizlo 2018-03-09 19:27:31 PST
Created attachment 335494 [details]
the patch
Comment 21 EWS Watchlist 2018-03-09 19:30:22 PST
Attachment 335494 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:121:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:46:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:48:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5189:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 22 Filip Pizlo 2018-03-09 19:31:20 PST
Created attachment 335496 [details]
the patch
Comment 23 EWS Watchlist 2018-03-09 19:34:21 PST
Attachment 335496 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/runtime/DirectArguments.h:121:  Please replace ASSERT_WITH_SECURITY_IMPLICATION() with RELEASE_ASSERT_WITH_SECURITY_IMPLICATION().  [security/assertion] [5]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:43:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:44:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:45:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:46:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:48:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
ERROR: Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5189:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:47:  Wrong number of spaces before statement. (expected: 12)  [whitespace/indent] [4]
Total errors found: 9 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 24 Yusuke Suzuki 2018-03-11 12:19:50 PDT
Comment on attachment 335496 [details]
the patch

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

Is my understanding about the threat model cared in this patch correct? If so, r=me.

> Source/JavaScriptCore/ChangeLog:12
> +        window, a write could appear to be made speculatively and rolled out later. This means that:

My understanding is that, if you have length in the object (like DirectArguments), speculative execution of `m_length = xxx` (maybe in an indirected way) in the untaken path can be done and this updated `m_length` can be used for speculative loading. Is it correct?

if (unmitigated structure check) {
    // any read and write to this JSObject inline fields should not allow attackers to speculatively read secret memory region into the cache.
    // But DirectArguments is breaking this rule right now.
}

> Source/JavaScriptCore/ChangeLog:14
> +        - JSValue objects cannot have lengths, masks, or anything else inline.

So, if this is used for spectre mitigation, attacker can perform spectre attack with speculative execution of writes to these fields.

> Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:39
>  public:

It restores the value of lengthGPR and storageGPR.

> Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:37
> +// This calls operationCreateDirectArguments but then restores the value of lengthGPR.

It restores storageGPR.

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5184
> +        fastStorage = m_out.add(fastStorage, m_out.constIntPtr(DirectArguments::storageHeaderSize()));
> +        m_out.store32(length.value, fastStorage, m_heaps.DirectArguments_Storage_length);
> +        m_out.store32(m_out.constInt32(minCapacity), fastStorage, m_heaps.DirectArguments_Storage_minCapacity);
> +        

OK, we may store the pointer to the middle of auxiliary buffer in the stack/register and perform GC. But it's OK since it is not JSCell.

> Source/JavaScriptCore/runtime/DirectArguments.h:127
> +        return *preciseIndexMaskPtr(
> +            offset.offset(),
> +            std::max(storageHeader(storage).length, storageHeader(storage).minCapacity),
> +            ptr);

It would be nice if we have a comment what preciseIndexMaskPtr does in wtf/MathExtras.h.
It masks the given ptr with 0xffffffffffffffff (ptrwidth) if `index < length`. Otherwise, it masks ptr with 0. Similar to Linux kernel's array_ptr.
Comment 25 Radar WebKit Bug Importer 2018-03-11 13:07:30 PDT
<rdar://problem/38357169>
Comment 26 Filip Pizlo 2018-03-11 13:14:00 PDT
(In reply to Yusuke Suzuki from comment #24)
> Comment on attachment 335496 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335496&action=review
> 
> Is my understanding about the threat model cared in this patch correct? 

Yes! :-)

> If
> so, r=me.
> 
> > Source/JavaScriptCore/ChangeLog:12
> > +        window, a write could appear to be made speculatively and rolled out later. This means that:
> 
> My understanding is that, if you have length in the object (like
> DirectArguments), speculative execution of `m_length = xxx` (maybe in an
> indirected way) in the untaken path can be done and this updated `m_length`
> can be used for speculative loading. Is it correct?
> 
> if (unmitigated structure check) {
>     // any read and write to this JSObject inline fields should not allow
> attackers to speculatively read secret memory region into the cache.
>     // But DirectArguments is breaking this rule right now.
> }
> 
> > Source/JavaScriptCore/ChangeLog:14
> > +        - JSValue objects cannot have lengths, masks, or anything else inline.
> 
> So, if this is used for spectre mitigation, attacker can perform spectre
> attack with speculative execution of writes to these fields.
> 
> > Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:39
> >  public:
> 
> It restores the value of lengthGPR and storageGPR.
> 
> > Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:37
> > +// This calls operationCreateDirectArguments but then restores the value of lengthGPR.
> 
> It restores storageGPR.
> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5184
> > +        fastStorage = m_out.add(fastStorage, m_out.constIntPtr(DirectArguments::storageHeaderSize()));
> > +        m_out.store32(length.value, fastStorage, m_heaps.DirectArguments_Storage_length);
> > +        m_out.store32(m_out.constInt32(minCapacity), fastStorage, m_heaps.DirectArguments_Storage_minCapacity);
> > +        
> 
> OK, we may store the pointer to the middle of auxiliary buffer in the
> stack/register and perform GC. But it's OK since it is not JSCell.
> 
> > Source/JavaScriptCore/runtime/DirectArguments.h:127
> > +        return *preciseIndexMaskPtr(
> > +            offset.offset(),
> > +            std::max(storageHeader(storage).length, storageHeader(storage).minCapacity),
> > +            ptr);
> 
> It would be nice if we have a comment what preciseIndexMaskPtr does in
> wtf/MathExtras.h.
> It masks the given ptr with 0xffffffffffffffff (ptrwidth) if `index <
> length`. Otherwise, it masks ptr with 0. Similar to Linux kernel's array_ptr.
Comment 27 Filip Pizlo 2018-03-11 13:16:45 PDT
(In reply to Yusuke Suzuki from comment #24)
> Comment on attachment 335496 [details]
> the patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=335496&action=review
> 
> Is my understanding about the threat model cared in this patch correct? If
> so, r=me.
> 
> > Source/JavaScriptCore/ChangeLog:12
> > +        window, a write could appear to be made speculatively and rolled out later. This means that:
> 
> My understanding is that, if you have length in the object (like
> DirectArguments), speculative execution of `m_length = xxx` (maybe in an
> indirected way) in the untaken path can be done and this updated `m_length`
> can be used for speculative loading. Is it correct?
> 
> if (unmitigated structure check) {
>     // any read and write to this JSObject inline fields should not allow
> attackers to speculatively read secret memory region into the cache.
>     // But DirectArguments is breaking this rule right now.
> }
> 
> > Source/JavaScriptCore/ChangeLog:14
> > +        - JSValue objects cannot have lengths, masks, or anything else inline.
> 
> So, if this is used for spectre mitigation, attacker can perform spectre
> attack with speculative execution of writes to these fields.
> 
> > Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsSlowPathGenerator.h:39
> >  public:
> 
> It restores the value of lengthGPR and storageGPR.

Fixed.

> 
> > Source/JavaScriptCore/dfg/DFGCallCreateDirectArgumentsWithKnownLengthSlowPathGenerator.h:37
> > +// This calls operationCreateDirectArguments but then restores the value of lengthGPR.
> 
> It restores storageGPR.

Fixed.

> 
> > Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:5184
> > +        fastStorage = m_out.add(fastStorage, m_out.constIntPtr(DirectArguments::storageHeaderSize()));
> > +        m_out.store32(length.value, fastStorage, m_heaps.DirectArguments_Storage_length);
> > +        m_out.store32(m_out.constInt32(minCapacity), fastStorage, m_heaps.DirectArguments_Storage_minCapacity);
> > +        
> 
> OK, we may store the pointer to the middle of auxiliary buffer in the
> stack/register and perform GC. But it's OK since it is not JSCell.

Yeah, the GC will mark the auxiliary buffer. But it will not scan it for outgoing pointers. So, it's safe to store things to it, provided you don't try to use it as the only root for other pointers.

> 
> > Source/JavaScriptCore/runtime/DirectArguments.h:127
> > +        return *preciseIndexMaskPtr(
> > +            offset.offset(),
> > +            std::max(storageHeader(storage).length, storageHeader(storage).minCapacity),
> > +            ptr);
> 
> It would be nice if we have a comment what preciseIndexMaskPtr does in
> wtf/MathExtras.h.
> It masks the given ptr with 0xffffffffffffffff (ptrwidth) if `index <
> length`. Otherwise, it masks ptr with 0. Similar to Linux kernel's array_ptr.

Yeah, I added such a comment.
Comment 28 Filip Pizlo 2018-03-11 14:09:37 PDT
Landed in https://trac.webkit.org/changeset/229518/webkit
Comment 29 Ryan Haddad 2018-03-12 13:04:30 PDT
This change caused 800 test failures on the 32-bit JSC bot:

https://build.webkit.org/builders/Apple%20High%20Sierra%2032-bit%20JSC%20%28BuildAndTest%29/builds/1392

stress/varargs-exit.js.default: ASSERTION FAILED: m_data[index].lockCount
stress/varargs-exit.js.default: ./dfg/DFGRegisterBank.h(175) : void JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(RegID, JSC::VirtualRegister, SpillHint) [BankInfo = JSC::GPRInfo]
stress/varargs-exit.js.default: 1   0x14eb9da WTFCrash
stress/varargs-exit.js.default: 2   0xa02996 JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(JSC::X86Registers::RegisterID, JSC::VirtualRegister, unsigned int)
stress/varargs-exit.js.default: 3   0xa05b46 JSC::DFG::SpeculativeJIT::cellResult(JSC::X86Registers::RegisterID, JSC::DFG::Node*, JSC::DFG::SpeculativeJIT::UseChildrenMode)
stress/varargs-exit.js.default: 4   0xa5abaf JSC::DFG::SpeculativeJIT::compileCreateDirectArguments(JSC::DFG::Node*)
stress/varargs-exit.js.default: 5   0xa21510 JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
stress/varargs-exit.js.default: 6   0xa0f1c6 JSC::DFG::SpeculativeJIT::compileCurrentBlock()
stress/varargs-exit.js.default: 7   0xa24fbf JSC::DFG::SpeculativeJIT::compile()
stress/varargs-exit.js.default: 8   0x84b27c JSC::DFG::JITCompiler::compileBody()
stress/varargs-exit.js.default: 9   0x850893 JSC::DFG::JITCompiler::compileFunction()
stress/varargs-exit.js.default: 10  0x903318 JSC::DFG::Plan::compileInThreadImpl()
stress/varargs-exit.js.default: 11  0x900485 JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
stress/varargs-exit.js.default: 12  0x7c60e8 JSC::DFG::compileImpl(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
stress/varargs-exit.js.default: 13  0x7c5b09 JSC::DFG::compile(JSC::VM&, JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int, JSC::Operands<JSC::JSValue> const&, WTF::Ref<JSC::DeferredCompilationCallback, WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
stress/varargs-exit.js.default: 14  0xdcb055 operationOptimize
stress/varargs-exit.js.default: 15  0x4565883e
stress/varargs-exit.js.default: 16  0x2fdf2b llint_entry
stress/varargs-exit.js.default: 17  0x2f8230 vmEntryToJavaScript
stress/varargs-exit.js.default: 18  0xdc1818 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
stress/varargs-exit.js.default: 19  0xd57f93 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
stress/varargs-exit.js.default: 20  0x107eb72 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
stress/varargs-exit.js.default: 21  0xf7731 runWithOptions(GlobalObject*, CommandLine&)
stress/varargs-exit.js.default: 22  0xca48c jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*) const
stress/varargs-exit.js.default: 23  0xafbff int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
stress/varargs-exit.js.default: 24  0xae3bf jscmain(int, char**)
stress/varargs-exit.js.default: 25  0xae2d7 main
stress/varargs-exit.js.default: 26  0xa759f6e1 start
stress/varargs-exit.js.default: 27  0x8
stress/varargs-exit.js.default: test_script_11047: line 2: 43850 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true varargs-exit.js )
stress/varargs-exit.js.default: ERROR: Unexpected exit code: 139
Comment 30 Filip Pizlo 2018-03-12 13:13:18 PDT
(In reply to Ryan Haddad from comment #29)
> This change caused 800 test failures on the 32-bit JSC bot:
> 
> https://build.webkit.org/builders/Apple%20High%20Sierra%2032-
> bit%20JSC%20%28BuildAndTest%29/builds/1392
> 
> stress/varargs-exit.js.default: ASSERTION FAILED: m_data[index].lockCount
> stress/varargs-exit.js.default: ./dfg/DFGRegisterBank.h(175) : void
> JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(RegID, JSC::VirtualRegister,
> SpillHint) [BankInfo = JSC::GPRInfo]
> stress/varargs-exit.js.default: 1   0x14eb9da WTFCrash
> stress/varargs-exit.js.default: 2   0xa02996
> JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(JSC::X86Registers::RegisterID,
> JSC::VirtualRegister, unsigned int)
> stress/varargs-exit.js.default: 3   0xa05b46
> JSC::DFG::SpeculativeJIT::cellResult(JSC::X86Registers::RegisterID,
> JSC::DFG::Node*, JSC::DFG::SpeculativeJIT::UseChildrenMode)
> stress/varargs-exit.js.default: 4   0xa5abaf
> JSC::DFG::SpeculativeJIT::compileCreateDirectArguments(JSC::DFG::Node*)
> stress/varargs-exit.js.default: 5   0xa21510
> JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
> stress/varargs-exit.js.default: 6   0xa0f1c6
> JSC::DFG::SpeculativeJIT::compileCurrentBlock()
> stress/varargs-exit.js.default: 7   0xa24fbf
> JSC::DFG::SpeculativeJIT::compile()
> stress/varargs-exit.js.default: 8   0x84b27c
> JSC::DFG::JITCompiler::compileBody()
> stress/varargs-exit.js.default: 9   0x850893
> JSC::DFG::JITCompiler::compileFunction()
> stress/varargs-exit.js.default: 10  0x903318
> JSC::DFG::Plan::compileInThreadImpl()
> stress/varargs-exit.js.default: 11  0x900485
> JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
> stress/varargs-exit.js.default: 12  0x7c60e8 JSC::DFG::compileImpl(JSC::VM&,
> JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int,
> JSC::Operands<JSC::JSValue> const&,
> WTF::Ref<JSC::DeferredCompilationCallback,
> WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
> stress/varargs-exit.js.default: 13  0x7c5b09 JSC::DFG::compile(JSC::VM&,
> JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int,
> JSC::Operands<JSC::JSValue> const&,
> WTF::Ref<JSC::DeferredCompilationCallback,
> WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
> stress/varargs-exit.js.default: 14  0xdcb055 operationOptimize
> stress/varargs-exit.js.default: 15  0x4565883e
> stress/varargs-exit.js.default: 16  0x2fdf2b llint_entry
> stress/varargs-exit.js.default: 17  0x2f8230 vmEntryToJavaScript
> stress/varargs-exit.js.default: 18  0xdc1818 JSC::JITCode::execute(JSC::VM*,
> JSC::ProtoCallFrame*)
> stress/varargs-exit.js.default: 19  0xd57f93
> JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*,
> JSC::JSObject*)
> stress/varargs-exit.js.default: 20  0x107eb72 JSC::evaluate(JSC::ExecState*,
> JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
> stress/varargs-exit.js.default: 21  0xf7731 runWithOptions(GlobalObject*,
> CommandLine&)
> stress/varargs-exit.js.default: 22  0xca48c jscmain(int,
> char**)::$_3::operator()(JSC::VM&, GlobalObject*) const
> stress/varargs-exit.js.default: 23  0xafbff int runJSC<jscmain(int,
> char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
> stress/varargs-exit.js.default: 24  0xae3bf jscmain(int, char**)
> stress/varargs-exit.js.default: 25  0xae2d7 main
> stress/varargs-exit.js.default: 26  0xa759f6e1 start
> stress/varargs-exit.js.default: 27  0x8
> stress/varargs-exit.js.default: test_script_11047: line 2: 43850
> Segmentation fault: 11  ( "$@"
> ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false
> --useFunctionDotArguments\=true --validateExceptionChecks\=true
> --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true
> varargs-exit.js )
> stress/varargs-exit.js.default: ERROR: Unexpected exit code: 139

Looks easy to fix.  Looking at it now.
Comment 31 Filip Pizlo 2018-03-12 13:20:14 PDT
(In reply to Filip Pizlo from comment #30)
> (In reply to Ryan Haddad from comment #29)
> > This change caused 800 test failures on the 32-bit JSC bot:
> > 
> > https://build.webkit.org/builders/Apple%20High%20Sierra%2032-
> > bit%20JSC%20%28BuildAndTest%29/builds/1392
> > 
> > stress/varargs-exit.js.default: ASSERTION FAILED: m_data[index].lockCount
> > stress/varargs-exit.js.default: ./dfg/DFGRegisterBank.h(175) : void
> > JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(RegID, JSC::VirtualRegister,
> > SpillHint) [BankInfo = JSC::GPRInfo]
> > stress/varargs-exit.js.default: 1   0x14eb9da WTFCrash
> > stress/varargs-exit.js.default: 2   0xa02996
> > JSC::DFG::RegisterBank<JSC::GPRInfo>::retain(JSC::X86Registers::RegisterID,
> > JSC::VirtualRegister, unsigned int)
> > stress/varargs-exit.js.default: 3   0xa05b46
> > JSC::DFG::SpeculativeJIT::cellResult(JSC::X86Registers::RegisterID,
> > JSC::DFG::Node*, JSC::DFG::SpeculativeJIT::UseChildrenMode)
> > stress/varargs-exit.js.default: 4   0xa5abaf
> > JSC::DFG::SpeculativeJIT::compileCreateDirectArguments(JSC::DFG::Node*)
> > stress/varargs-exit.js.default: 5   0xa21510
> > JSC::DFG::SpeculativeJIT::compile(JSC::DFG::Node*)
> > stress/varargs-exit.js.default: 6   0xa0f1c6
> > JSC::DFG::SpeculativeJIT::compileCurrentBlock()
> > stress/varargs-exit.js.default: 7   0xa24fbf
> > JSC::DFG::SpeculativeJIT::compile()
> > stress/varargs-exit.js.default: 8   0x84b27c
> > JSC::DFG::JITCompiler::compileBody()
> > stress/varargs-exit.js.default: 9   0x850893
> > JSC::DFG::JITCompiler::compileFunction()
> > stress/varargs-exit.js.default: 10  0x903318
> > JSC::DFG::Plan::compileInThreadImpl()
> > stress/varargs-exit.js.default: 11  0x900485
> > JSC::DFG::Plan::compileInThread(JSC::DFG::ThreadData*)
> > stress/varargs-exit.js.default: 12  0x7c60e8 JSC::DFG::compileImpl(JSC::VM&,
> > JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int,
> > JSC::Operands<JSC::JSValue> const&,
> > WTF::Ref<JSC::DeferredCompilationCallback,
> > WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
> > stress/varargs-exit.js.default: 13  0x7c5b09 JSC::DFG::compile(JSC::VM&,
> > JSC::CodeBlock*, JSC::CodeBlock*, JSC::DFG::CompilationMode, unsigned int,
> > JSC::Operands<JSC::JSValue> const&,
> > WTF::Ref<JSC::DeferredCompilationCallback,
> > WTF::DumbPtrTraits<JSC::DeferredCompilationCallback> >&&)
> > stress/varargs-exit.js.default: 14  0xdcb055 operationOptimize
> > stress/varargs-exit.js.default: 15  0x4565883e
> > stress/varargs-exit.js.default: 16  0x2fdf2b llint_entry
> > stress/varargs-exit.js.default: 17  0x2f8230 vmEntryToJavaScript
> > stress/varargs-exit.js.default: 18  0xdc1818 JSC::JITCode::execute(JSC::VM*,
> > JSC::ProtoCallFrame*)
> > stress/varargs-exit.js.default: 19  0xd57f93
> > JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*,
> > JSC::JSObject*)
> > stress/varargs-exit.js.default: 20  0x107eb72 JSC::evaluate(JSC::ExecState*,
> > JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
> > stress/varargs-exit.js.default: 21  0xf7731 runWithOptions(GlobalObject*,
> > CommandLine&)
> > stress/varargs-exit.js.default: 22  0xca48c jscmain(int,
> > char**)::$_3::operator()(JSC::VM&, GlobalObject*) const
> > stress/varargs-exit.js.default: 23  0xafbff int runJSC<jscmain(int,
> > char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
> > stress/varargs-exit.js.default: 24  0xae3bf jscmain(int, char**)
> > stress/varargs-exit.js.default: 25  0xae2d7 main
> > stress/varargs-exit.js.default: 26  0xa759f6e1 start
> > stress/varargs-exit.js.default: 27  0x8
> > stress/varargs-exit.js.default: test_script_11047: line 2: 43850
> > Segmentation fault: 11  ( "$@"
> > ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false
> > --useFunctionDotArguments\=true --validateExceptionChecks\=true
> > --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true
> > varargs-exit.js )
> > stress/varargs-exit.js.default: ERROR: Unexpected exit code: 139
> 
> Looks easy to fix.  Looking at it now.

Should be fixed in https://trac.webkit.org/changeset/229545/webkit