Bug 174727 - Bmalloc and GC should put auxiliaries (butterflies, typed array backing stores) in a gigacage (separate multi-GB VM region)
Summary: Bmalloc and GC should put auxiliaries (butterflies, typed array backing store...
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: 174811
Blocks: 174917
  Show dependency treegraph
 
Reported: 2017-07-21 14:39 PDT by Filip Pizlo
Modified: 2017-08-04 19:05 PDT (History)
17 users (show)

See Also:


Attachments
it's coming along! (37.10 KB, patch)
2017-07-21 14:40 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
a little more (52.56 KB, patch)
2017-07-21 18:40 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
couldn't help myself (76.67 KB, patch)
2017-07-21 19:20 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (101.40 KB, patch)
2017-07-21 21:32 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
pretty good initial version (87.65 KB, patch)
2017-07-22 10:12 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
starting to compile (91.30 KB, patch)
2017-07-22 10:42 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
JSC compiles! (107.02 KB, patch)
2017-07-22 16:29 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it can print hello! (107.41 KB, patch)
2017-07-22 16:54 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it does sensible things for a program that accesses arrays! (109.48 KB, patch)
2017-07-22 18:17 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (122.70 KB, patch)
2017-07-23 17:55 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (127.12 KB, patch)
2017-07-23 19:22 PDT, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews105 for mac-elcapitan-wk2 (1.06 MB, application/zip)
2017-07-23 20:33 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews102 for mac-elcapitan (1.23 MB, application/zip)
2017-07-23 20:38 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews115 for mac-elcapitan (1.21 MB, application/zip)
2017-07-23 20:46 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews121 for ios-simulator-wk2 (988.82 KB, application/zip)
2017-07-23 20:56 PDT, Build Bot
no flags Details
passing more tests (134.91 KB, patch)
2017-07-24 20:59 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
rebased (138.25 KB, patch)
2017-07-25 19:01 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
getting closer (146.39 KB, patch)
2017-07-25 20:24 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
maybe it's done now (165.57 KB, patch)
2017-07-25 20:57 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it compiles! (174.80 KB, patch)
2017-07-25 21:30 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it runs wasmbench! (199.66 KB, patch)
2017-07-27 11:47 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
came up with a more principled way of handling null (201.66 KB, patch)
2017-07-27 14:19 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
fixed more wasm memory bugs (201.78 KB, patch)
2017-07-27 15:12 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
it has a chance of passing JSC tests (209.93 KB, patch)
2017-07-27 17:12 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
passes JSC tests (209.94 KB, patch)
2017-07-27 18:14 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
rebased (209.94 KB, patch)
2017-07-27 18:32 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
fix 32-bit (209.96 KB, patch)
2017-07-27 19:18 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
try to fix gcc build (209.95 KB, patch)
2017-07-27 19:27 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
fix more builds (210.08 KB, patch)
2017-07-27 19:34 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (211.61 KB, patch)
2017-07-27 20:01 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more fixes (212.18 KB, patch)
2017-07-27 20:26 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (212.39 KB, patch)
2017-07-27 20:30 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
more (212.39 KB, patch)
2017-07-27 21:16 PDT, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews106 for mac-elcapitan-wk2 (1.13 MB, application/zip)
2017-07-27 22:37 PDT, Build Bot
no flags Details
fixed metal (215.69 KB, patch)
2017-07-28 15:40 PDT, Filip Pizlo
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews100 for mac-elcapitan (1.01 MB, application/zip)
2017-07-28 18:19 PDT, Build Bot
no flags Details
fix some issues (215.94 KB, patch)
2017-07-29 12:14 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (216.04 KB, patch)
2017-07-29 12:52 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
trial patch (216.10 KB, patch)
2017-07-29 13:29 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (216.05 KB, patch)
2017-07-29 13:57 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (216.07 KB, patch)
2017-07-29 14:47 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (216.09 KB, patch)
2017-07-29 15:41 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
maybe this will do it (216.13 KB, patch)
2017-07-29 17:23 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
rebased (223.28 KB, patch)
2017-08-01 12:16 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
with more fixes (223.20 KB, patch)
2017-08-01 13:13 PDT, Filip Pizlo
mark.lam: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews124 for ios-simulator-wk2 (975.30 KB, application/zip)
2017-08-01 14:59 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews117 for mac-elcapitan (1.87 MB, application/zip)
2017-08-01 15:03 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2017-07-21 14:39:49 PDT
So fun.
Comment 1 Filip Pizlo 2017-07-21 14:40:39 PDT
Created attachment 316124 [details]
it's coming along!
Comment 2 Filip Pizlo 2017-07-21 18:40:29 PDT
Created attachment 316156 [details]
a little more
Comment 3 Filip Pizlo 2017-07-21 19:20:18 PDT
Created attachment 316161 [details]
couldn't help myself
Comment 4 Filip Pizlo 2017-07-21 21:32:34 PDT
Created attachment 316167 [details]
more
Comment 5 Filip Pizlo 2017-07-22 10:12:47 PDT
Created attachment 316199 [details]
pretty good initial version

I haven't tried compiling it yet.
Comment 6 Filip Pizlo 2017-07-22 10:42:10 PDT
Created attachment 316200 [details]
starting to compile
Comment 7 Filip Pizlo 2017-07-22 16:29:37 PDT
Created attachment 316210 [details]
JSC compiles!
Comment 8 Filip Pizlo 2017-07-22 16:54:48 PDT
Created attachment 316212 [details]
it can print hello!
Comment 9 Filip Pizlo 2017-07-22 18:17:48 PDT
Created attachment 316221 [details]
it does sensible things for a program that accesses arrays!
Comment 10 Filip Pizlo 2017-07-23 17:55:29 PDT
Created attachment 316245 [details]
more
Comment 11 Filip Pizlo 2017-07-23 19:22:37 PDT
Created attachment 316250 [details]
the patch

I think it's passing tests now.
Comment 12 Build Bot 2017-07-23 19:24:32 PDT
Attachment 316250 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/Gigacage.h:60:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:117:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:188:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:423:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 11 in 58 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Filip Pizlo 2017-07-23 19:27:26 PDT
Comment on attachment 316250 [details]
the patch

It's not ready for review yet.  I'm seeing some test failures.
Comment 14 Filip Pizlo 2017-07-23 20:01:34 PDT
Ooops, I forgot about wasm memory.
Comment 15 Filip Pizlo 2017-07-23 20:05:51 PDT
I think I can get this to work by just rewriting how WasmMemory works.  It should just use the Gigacage.  We should probably just have the Gigacage be structured as follows:

- 64GB for the normal Gigacage
- 64GB for WebAssembly memories

Then we just need a GC control system that can juggle 16 wasm memories gracefully.

Sounds pretty easy.
Comment 16 Build Bot 2017-07-23 20:33:56 PDT
Comment on attachment 316250 [details]
the patch

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

Number of test failures exceeded the failure limit.
Comment 17 Build Bot 2017-07-23 20:33:59 PDT
Created attachment 316255 [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
Comment 18 Build Bot 2017-07-23 20:38:23 PDT
Comment on attachment 316250 [details]
the patch

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

Number of test failures exceeded the failure limit.
Comment 19 Build Bot 2017-07-23 20:38:24 PDT
Created attachment 316257 [details]
Archive of layout-test-results from ews102 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 20 Build Bot 2017-07-23 20:41:49 PDT
Comment on attachment 316250 [details]
the patch

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

New failing tests:
stress/typed-array-byte-offset.js.default
wasm.yaml/wasm/function-tests/loop-mult.js.default-wasm
stress/put-direct-index-broken-2.js.no-llint
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-eager-jettison
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-llint
wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-eager-jettison
microbenchmarks/asmjs_bool_bug.js.no-cjit-collect-continuously
wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Instance.js.default-wasm
stress/jsc-read.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/function-tests/rotl.js.wasm-no-tls-context
stress/jsc-read.js.no-cjit-validate-phases
wasm.yaml/wasm/js-api/validate.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/Module.customSection.js.default-wasm
wasm.yaml/wasm/js-api/test_Data.js.wasm-eager-jettison
stress/typed-array-byte-offset.js.dfg-eager
wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/add-12.js.default-wasm
wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/globals-import.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/globals-export.js.default-wasm
wasm.yaml/wasm/function-tests/f32-const.js.default-wasm
wasm.yaml/wasm/fuzz/export-function.js.default-wasm
microbenchmarks/typed-array-subarray.js.ftl-eager-no-cjit-b3o1
wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-tls-context
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout
stress/jsc-read.js.no-llint
wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/grow-memory.js.default-wasm
wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-tls-context
stress/typed-array-byte-offset.js.ftl-no-cjit-no-inline-validate
wasm.yaml/wasm/js-api/global-error.js.wasm-no-cjit-yes-tls-context
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-ftl-no-cjit
wasm.yaml/wasm/function-tests/factorial.js.default-wasm
wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-cjit-yes-tls-context
stress/jsc-read.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/js-api/element.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Start.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-tls-context
microbenchmarks/typed-array-subarray.js.no-ftl
wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/globals-import.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/struct.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/export-arity.js.default-wasm
wasm.yaml/wasm/function-tests/load-offset.js.wasm-eager-jettison
wasm.yaml/wasm/fuzz/memory.js.wasm-eager-jettison
stress/jsc-read.js.ftl-no-cjit-no-inline-validate
wasm.yaml/wasm/js-api/test_Data.js.default-wasm
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-tls-context
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager-no-cjit-b3o1
wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/memory-grow.js.default-wasm
wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.default-wasm
microbenchmarks/asmjs_bool_bug.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/function-tests/load-offset.js.default-wasm
wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_Data.js.wasm-no-cjit-yes-tls-context
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager-no-cjit
wasm.yaml/wasm/js-api/table.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-tls-context
wasm.yaml/wasm/fuzz/export-function.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.default-wasm
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/float-sub.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/factorial.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/wrapper-function.js.default-wasm
wasm.yaml/wasm/spec-tests/jsapi.js.default-wasm
wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/unique-signature.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.default-wasm
wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-cjit-yes-tls-context
stress/typed-array-byte-offset.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-dfg-eager-no-cjit
wasm.yaml/wasm/function-tests/table-basic.js.wasm-eager-jettison
stress/put-direct-index-broken-2.js.dfg-eager
wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/nameSection.js.default-wasm
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-no-inline-validate
stress/jsc-read.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-tls-context
microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/export-void-is-undef.js.default-wasm
wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/validate.js.default-wasm
wasm.yaml/wasm/function-tests/shr-u.js.default-wasm
wasm.yaml/wasm/js-api/global-mutate.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/brTableManyValues.js.default-wasm
wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/shl.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/factorial.js.wasm-no-tls-context
microbenchmarks/typed-array-subarray.js.no-cjit-validate-phases
wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-cjit-yes-tls-context
stress/put-direct-index-broken-2.js.ftl-eager
stress/typedarray-slice.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-call-ic
wasm.yaml/wasm/lowExecutableMemory/exports-oom.js.default-wasm
wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.default-wasm
wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/rotl.js.wasm-no-call-ic
stress/typed-array-byte-offset.js.dfg-maximal-flush-validate-no-cjit
microbenchmarks/typed-array-get-set-by-val-profiling.js.no-ftl
wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_Module.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/struct.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/factorial.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-const.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_memory.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/Module.exports.js.default-wasm
wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.default-wasm
wasm.yaml/wasm/js-api/call-indirect.js.default-wasm
wasm.yaml/wasm/js-api/element-data.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-cjit-yes-tls-context
stress/typedarray-slice.js.ftl-no-cjit-b3o1
wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/rotl.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/element.js.wasm-no-cjit-yes-tls-context
stress/typedarray-slice.js.ftl-eager
wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/table.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_Module.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/add-12.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_Start.js.default-wasm
wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-call-ic
stress/put-direct-index-broken-2.js.ftl-eager-no-cjit-b3o1
wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-tls-context
stress/put-direct-index-broken-2.js.ftl-no-cjit-b3o1
wasm.yaml/wasm/fuzz/memory.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/struct.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/test_Instance.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-eager-jettison
stress/typedarray-slice.js.ftl-no-cjit-no-put-stack-validate
wasm.yaml/wasm/fuzz/memory.js.default-wasm
wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-tls-context
microbenchmarks/asmjs_bool_bug.js.ftl-eager-no-cjit-b3o1
microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/rotr.js.default-wasm
wasm.yaml/wasm/js-api/globals-import.js.default-wasm
stress/jsc-read.js.ftl-eager
wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/rotr.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/ret5.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-call-ic
wasm.yaml/wasm/lowExecutableMemory/executable-memory-oom.js.default-wasm
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-eager-jettison
stress/typedarray-slice.js.ftl-no-cjit-no-inline-validate
wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-tls-context
stress/jsc-read.js.ftl-eager-no-cjit
microbenchmarks/typed-array-subarray.js.default
wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-call-ic
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/nameSection.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/globals-export.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/grow-memory.js.wasm-eager-jettison
stress/put-direct-index-broken-2.js.dfg-eager-no-cjit-validate
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-cjit
wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-cjit-yes-tls-context
stress/typedarray-slice.js.ftl-eager-no-cjit-b3o1
wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/f64-const.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/basic-element.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-cjit-yes-tls-context
stress/typed-array-byte-offset.js.ftl-no-cjit-b3o1
stress/typed-array-byte-offset.js.ftl-no-cjit-no-put-stack-validate
wasm.yaml/wasm/function-tests/function-import-return-value.js.default-wasm
wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-call-ic
stress/typed-array-byte-offset.js.no-llint
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/table.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/ret5.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/f64-const.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-call-ic
microbenchmarks/asmjs_bool_bug.js.default
wasm.yaml/wasm/function-tests/loop-mult.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-no-call-ic
wasm.yaml/wasm/fuzz/memory.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/element.js.default-wasm
wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/element-data.js.default-wasm
microbenchmarks/typed-array-subarray.js.ftl-no-cjit-no-put-stack-validate
stress/put-direct-index-broken-2.js.ftl-eager-no-cjit
microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/function-tests/context-switch.js.default-wasm
wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/Module-compile.js.default-wasm
wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/rotr.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-tls-context
microbenchmarks/typed-array-subarray.js.ftl-no-cjit-b3o1
microbenchmarks/asmjs_bool_bug.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/fuzz/export-function.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-const.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/f64-const.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/global-error.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/Module.exports.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-tls-context
microbenchmarks/typed-array-get-set-by-val-profiling.js.no-llint
wasm.yaml/wasm/function-tests/trap-store-2.js.default-wasm
wasm.yaml/wasm/js-api/test_Data.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/grow-memory.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/global-external-init-from-import.js.default-wasm
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-ftl-eager-no-cjit
microbenchmarks/typed-array-get-set-by-val-profiling.js.no-cjit-validate-phases
stress/typedarray-slice.js.dfg-eager
stress/typed-array-byte-offset.js.ftl-eager-no-cjit
wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-eager-jettison
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-no-put-stack-validate
wasm.yaml/wasm/function-tests/exceptions.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/Module.customSection.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/br-as-return.js.wasm-no-tls-context
microbenchmarks/asmjs_bool_bug.js.ftl-eager-no-cjit
microbenchmarks/typed-array-subarray.js.ftl-eager-no-cjit
wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-call-ic
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.default-wasm
wasm.yaml/wasm/function-tests/shr-s.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.default-wasm
stress/typedarray-slice.js.no-cjit-validate-phases
wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Start.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/element.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-3.js.default-wasm
microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-no-inline-validate
wasm.yaml/wasm/function-tests/context-switch.js.wasm-no-call-ic
stress/jsc-read.js.ftl-no-cjit-b3o1
wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-tls-context
microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-eager
wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-cjit-yes-tls-context
stress/typedarray-slice.js.no-ftl
wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/loop-sum.js.default-wasm
wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/Module.imports.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/trap-load.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-tls-context
microbenchmarks/asmjs_bool_bug.js.dfg-eager
wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/exceptions.js.default-wasm
stress/typedarray-slice.js.ftl-eager-no-cjit
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.default-wasm
wasm.yaml/wasm/js-api/export-arity.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-call-ic
stress/put-direct-index-broken-2.js.ftl-no-cjit-no-put-stack-validate
wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/rotr.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/global-mutate.js.wasm-no-cjit-yes-tls-context
microbenchmarks/typed-array-subarray.js.dfg-eager
wasm.yaml/wasm/function-tests/memory-section-and-import.js.default-wasm
wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.wasm-no-tls-context
stress/put-direct-index-broken-2.js.ftl-no-cjit-no-inline-validate
microbenchmarks/typed-array-get-set-by-val-profiling.js.no-cjit-collect-continuously
wasm.yaml/wasm/function-tests/ret5.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/shr-s.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/shl.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/brTableAsIf.js.default-wasm
wasm.yaml/wasm/function-tests/stack-trace.js.default-wasm
wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/memory-grow-invalid.js.default-wasm
wasm.yaml/wasm/function-tests/trap-store.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/grow-memory-4.js.default-wasm
wasm.yaml/wasm/js-api/global-error.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/Module.exports.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/table-basic.js.default-wasm
wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/lowExecutableMemory/imports-oom.js.default-wasm
wasm.yaml/wasm/function-tests/f32-const.js.wasm-eager-jettison
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-eager
stress/typedarray-slice.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/function-tests/rotl.js.wasm-eager-jettison
stress/put-direct-index-broken-2.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/js-api/memory-grow.js.wasm-no-cjit-yes-tls-context
stress/typed-array-byte-offset.js.no-cjit-collect-continuously
wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.default-wasm
wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/wrapper-function.js.wasm-no-tls-context
stress/typed-array-byte-offset.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/js-api/call-indirect.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.default-wasm
wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-cjit-yes-tls-context
stress/put-direct-index-broken-2.js.default
wasm.yaml/wasm/function-tests/i32-load8-s.js.default-wasm
wasm.yaml/wasm/js-api/global-internal-init-from-import.js.default-wasm
wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/basic-element.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/globals-export.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-no-call-ic
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/load-offset.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Start.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/web-assembly-instantiate-parallel.js.wasm-eager-jettison
microbenchmarks/typed-array-subarray.js.ftl-eager
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-b3o1
wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/dead-call.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/trap-load.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/stack-overflow.js.default-wasm
stress/jsc-read.js.ftl-no-cjit-no-put-stack-validate
stress/put-direct-index-broken-2.js.no-ftl
wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-no-call-ic
wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.default-wasm
wasm.yaml/wasm/function-tests/many-args-tail-call-sp-restored.js.default-wasm
wasm.yaml/wasm/function-tests/stack-trace.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/ret5.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/call-indirect.js.wasm-eager-jettison
stress/typedarray-slice.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/js-api/export-arity.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/element-data.js.wasm-no-cjit-yes-tls-context
microbenchmarks/typed-array-subarray.js.no-cjit-collect-continuously
wasm.yaml/wasm/js-api/memory-grow.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-eager-jettison
microbenchmarks/typed-array-get-set-by-val-profiling.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/js-api/test_Instance.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i32-const.js.default-wasm
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/br-as-return.js.default-wasm
wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/trap-load-2.js.default-wasm
wasm.yaml/wasm/js-api/table.js.wasm-no-call-ic
stress/put-direct-index-broken-2.js.no-cjit-validate-phases
microbenchmarks/typed-array-subarray.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/dead-call.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/basic-element.js.default-wasm
wasm.yaml/wasm/js-api/element-data.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/web-assembly-instantiate.js.default-wasm
wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/extension-MemoryMode.js.default-wasm
wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.default-wasm
wasm.yaml/wasm/function-tests/br-if-loop-less-than.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/memory-alignment.js.default-wasm
wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-eager-jettison
microbenchmarks/asmjs_bool_bug.js.no-ftl
jsc-layout-tests.yaml/js/script-tests/regress-155776.js.layout-no-ftl
wasm.yaml/wasm/function-tests/add-12.js.wasm-no-tls-context
microbenchmarks/asmjs_bool_bug.js.no-llint
wasm.yaml/wasm/function-tests/factorial.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/global-error.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_memory.js.default-wasm
wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.default-wasm
wasm.yaml/wasm/function-tests/context-switch.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.default-wasm
wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-cjit-yes-tls-context
stress/put-direct-index-broken-2.js.no-cjit-collect-continuously
wasm.yaml/wasm/function-tests/table-basic.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i32-load.js.default-wasm
stress/jsc-read.js.ftl-eager-no-cjit-b3o1
microbenchmarks/asmjs_bool_bug.js.ftl-eager
wasm.yaml/wasm/function-tests/shr-u.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/unique-signature.js.default-wasm
stress/typed-array-byte-offset.js.no-cjit-validate-phases
wasm.yaml/wasm/function-tests/memory-grow-invalid.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/element.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/br-table-as-return.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/validate.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/br-table-as-return.js.wasm-no-call-ic
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint32.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-load.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/add-12.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/validate.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/table.js.default-wasm
wasm.yaml/wasm/js-api/Module.customSection.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/br-if-as-return.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/globals-import.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/br-as-return.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/global-error.js.default-wasm
wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/Module.imports.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/if-no-else-non-void.js.default-wasm
wasm.yaml/wasm/function-tests/f32-const.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/i32-load.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/shl.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-tls-context
stress/jsc-read.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/js-api/globals-export.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/brTableWithLoop.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/rotl.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.wasm-no-tls-context
microbenchmarks/typed-array-subarray.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/function-tests/table-basic-2.js.default-wasm
wasm.yaml/wasm/function-tests/memory-alignment.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/i64-from-js-exceptions.js.default-wasm
wasm.yaml/wasm/js-api/global-external-init-from-import.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/Module-compile.js.wasm-no-cjit-yes-tls-context
microbenchmarks/asmjs_bool_bug.js.no-cjit-validate-phases
wasm.yaml/wasm/function-tests/float-sub.js.default-wasm
wasm.yaml/wasm/function-tests/shl.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/invalid-duplicate-export.js.wasm-no-tls-context
stress/jsc-read.js.no-ftl
wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-no-tls-context
microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-no-put-stack-validate
stress/typed-array-byte-offset.js.ftl-eager
wasm.yaml/wasm/function-tests/dead-call.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_memory_constructor.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/float-sub.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/br-if-as-return.js.default-wasm
wasm.yaml/wasm/function-tests/nameSection.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/rotr.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_string.js.default-wasm
microbenchmarks/typed-array-subarray.js.no-llint
wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint8.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-section-and-import.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/loop-sum.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/i32-load8-s.js.wasm-no-cjit-yes-tls-context
microbenchmarks/typed-array-subarray.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/function-tests/struct.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/stack-trace.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_Data.js.wasm-no-call-ic
stress/put-direct-index-broken-2.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/export-arity.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/wasm-to-wasm.js.default-wasm
wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.wasm-eager-jettison
microbenchmarks/asmjs_bool_bug.js.ftl-no-cjit-b3o1
wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/table-basic-2.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/function-import-return-value.js.wasm-eager-jettison
wasm.yaml/wasm/fuzz/memory.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/Module.imports.js.default-wasm
wasm.yaml/wasm/function-tests/trap-load.js.default-wasm
wasm.yaml/wasm/js-api/test_memory.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/brTableWithLoop.js.default-wasm
stress/jsc-read.js.default
wasm.yaml/wasm/js-api/globals-export.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/trap-after-cross-instance-call.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/Instance.imports.exports.unicode.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/exceptions.js.wasm-eager-jettison
stress/typedarray-slice.js.no-llint
wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/trap-store-2.js.wasm-no-cjit-yes-tls-context
stress/jsc-read.js.dfg-eager
microbenchmarks/typed-array-subarray.js.ftl-no-cjit-no-inline-validate
wasm.yaml/wasm/self-test/test_BuilderWebAssembly.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/dont-mmap-zero-byte-memory.js.default-wasm
wasm.yaml/wasm/spec-tests/jsapi.js.wasm-eager-jettison
stress/typedarray-slice.js.no-cjit-collect-continuously
wasm.yaml/wasm/function-tests/loop-sum.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-import-and-grow.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/test_Start.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/export-void-is-undef.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/Module-compile.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/global-mutate.js.default-wasm
wasm.yaml/wasm/js-api/test_memory_constructor.js.default-wasm
wasm.yaml/wasm/function-tests/brTableAsIf.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/if-no-else-non-void.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/many-arguments-to-function.js.default-wasm
wasm.yaml/wasm/js-api/wasm-to-wasm.js.wasm-eager-jettison
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint16.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/trap-load-2.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/shr-s.js.default-wasm
stress/put-direct-index-broken-2.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/js-api/globals-import.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/web-assembly-compile-parallel.js.wasm-eager-jettison
microbenchmarks/typed-array-subarray.js.dfg-maximal-flush-validate-no-cjit
wasm.yaml/wasm/function-tests/memory-access-past-4gib.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/wasm-to-wasm-bad-signature.js.default-wasm
wasm.yaml/wasm/js-api/validate.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/web-assembly-function.js.default-wasm
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint24.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_memory.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/grow-memory-2.js.default-wasm
stress/typedarray-slice.js.default
wasm.yaml/wasm/function-tests/trap-store.js.default-wasm
wasm.yaml/wasm/js-api/unique-signature.js.wasm-no-call-ic
stress/typed-array-byte-offset.js.dfg-eager-no-cjit-validate
wasm.yaml/wasm/js-api/web-assembly-function.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_Module.js.wasm-no-call-ic
wasm.yaml/wasm/fuzz/export-function.js.wasm-no-call-ic
wasm.yaml/wasm/js-api/test_Module.js.default-wasm
wasm.yaml/wasm/function-tests/shr-u.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/element-data.js.wasm-no-tls-context
wasm.yaml/wasm/js-api/wrapper-function.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/grow-memory-4.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/brTableManyValues.js.wasm-eager-jettison
wasm.yaml/wasm/fuzz/export-function.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/stack-overflow.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/ret5.js.default-wasm
wasm.yaml/wasm/function-tests/shl.js.default-wasm
wasm.yaml/wasm/js-api/web-assembly-instantiate.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/add-12.js.wasm-no-call-ic
microbenchmarks/typed-array-get-set-by-val-profiling.js.ftl-no-cjit-validate-sampling-profiler
wasm.yaml/wasm/self-test/test_LowLevelBinary_uint32.js.wasm-no-call-ic
stress/typedarray-slice.js.ftl-no-cjit-small-pool
wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/struct.js.default-wasm
wasm.yaml/wasm/js-api/global-internal-init-from-import.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/grow-memory-3.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/grow-memory-2.js.wasm-eager-jettison
stress/typed-array-byte-offset.js.ftl-eager-no-cjit-b3o1
wasm.yaml/wasm/js-api/export-arity.js.wasm-eager-jettison
wasm.yaml/wasm/function-tests/loop-mult.js.wasm-no-call-ic
wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/memory-import-and-grow.js.default-wasm
microbenchmarks/typed-array-get-set-by-val-profiling.js.default
wasm.yaml/wasm/js-api/extension-MemoryMode.js.wasm-no-tls-context
wasm.yaml/wasm/function-tests/many-arguments-to-function.js.wasm-eager-jettison
wasm.yaml/wasm/js-api/test_memory.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/js-api/test_Module.js.wasm-no-tls-context
stress/typed-array-byte-offset.js.ftl-no-cjit-validate-sampling-profiler
stress/jsc-read.js.no-cjit-collect-continuously
wasm.yaml/wasm/self-test/test_LowLevelBinary_varuint1.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_varint32.js.wasm-no-tls-context
wasm.yaml/wasm/self-test/test_LowLevelBinary_encode.js.wasm-no-cjit-yes-tls-context
wasm.yaml/wasm/function-tests/trap-store.js.wasm-no-call-ic
Comment 21 Build Bot 2017-07-23 20:46:08 PDT
Comment on attachment 316250 [details]
the patch

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

Number of test failures exceeded the failure limit.
Comment 22 Build Bot 2017-07-23 20:46:09 PDT
Created attachment 316258 [details]
Archive of layout-test-results from ews115 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 23 Build Bot 2017-07-23 20:56:00 PDT
Comment on attachment 316250 [details]
the patch

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

Number of test failures exceeded the failure limit.
Comment 24 Build Bot 2017-07-23 20:56:02 PDT
Created attachment 316260 [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.5
Comment 25 Filip Pizlo 2017-07-24 20:59:10 PDT
Created attachment 316349 [details]
passing more tests

I'm going to have to fix some things about how we do destructors so that we can easily destruct JSWebAssemblyMemory eagerly.
Comment 26 Filip Pizlo 2017-07-25 19:01:25 PDT
Created attachment 316418 [details]
rebased

Also started getting the wasm memory to do what I want.  Now I just have to glue the bmalloc and GC support for wasm memory into WasmMemory.cpp.
Comment 27 Filip Pizlo 2017-07-25 20:24:41 PDT
Created attachment 316424 [details]
getting closer
Comment 28 Filip Pizlo 2017-07-25 20:57:25 PDT
Created attachment 316427 [details]
maybe it's done now
Comment 29 Filip Pizlo 2017-07-25 21:30:10 PDT
Created attachment 316431 [details]
it compiles!
Comment 30 Filip Pizlo 2017-07-27 11:47:40 PDT
Created attachment 316560 [details]
it runs wasmbench!
Comment 31 Filip Pizlo 2017-07-27 14:19:46 PDT
Created attachment 316572 [details]
came up with a more principled way of handling null
Comment 32 Filip Pizlo 2017-07-27 15:12:32 PDT
Created attachment 316574 [details]
fixed more wasm memory bugs
Comment 33 Filip Pizlo 2017-07-27 17:12:28 PDT
Created attachment 316583 [details]
it has a chance of passing JSC tests

Fixed handling of WasmMemory OOM
Comment 34 Filip Pizlo 2017-07-27 18:14:15 PDT
Created attachment 316587 [details]
passes JSC tests
Comment 35 Filip Pizlo 2017-07-27 18:32:33 PDT
Created attachment 316593 [details]
rebased

I'm only now trying to build WebCore...
Comment 36 Build Bot 2017-07-27 18:34:55 PDT
Attachment 316593 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 37 Filip Pizlo 2017-07-27 19:18:35 PDT
Created attachment 316602 [details]
fix 32-bit
Comment 38 Build Bot 2017-07-27 19:21:38 PDT
Attachment 316602 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 39 Filip Pizlo 2017-07-27 19:27:13 PDT
Created attachment 316603 [details]
try to fix gcc build
Comment 40 Build Bot 2017-07-27 19:29:08 PDT
Attachment 316603 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 41 Filip Pizlo 2017-07-27 19:34:27 PDT
Created attachment 316604 [details]
fix more builds
Comment 42 Build Bot 2017-07-27 19:36:47 PDT
Attachment 316604 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 88 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 43 Filip Pizlo 2017-07-27 20:01:20 PDT
Created attachment 316606 [details]
the patch

Fixed more build issues. Referenced a bunch of other bugs.
Comment 44 Build Bot 2017-07-27 20:03:58 PDT
Attachment 316606 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 90 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 45 Filip Pizlo 2017-07-27 20:26:26 PDT
Created attachment 316607 [details]
more fixes
Comment 46 Build Bot 2017-07-27 20:29:57 PDT
Attachment 316607 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 91 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 47 Filip Pizlo 2017-07-27 20:30:54 PDT
Created attachment 316608 [details]
more
Comment 48 Build Bot 2017-07-27 20:33:32 PDT
Attachment 316608 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 91 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 49 Filip Pizlo 2017-07-27 21:16:19 PDT
Created attachment 316612 [details]
more

Trying to fix windows.
Comment 50 Build Bot 2017-07-27 21:19:58 PDT
Attachment 316612 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WTF/wtf/Gigacage.h:54:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:55:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:68:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:69:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 18 in 91 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 51 Build Bot 2017-07-27 22:37:55 PDT
Comment on attachment 316612 [details]
more

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

New failing tests:
fast/canvas/webgpu/webgpu-dispatch.html
Comment 52 Build Bot 2017-07-27 22:37:57 PDT
Created attachment 316617 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-elcapitan-wk2  Platform: Mac OS X 10.11.6
Comment 53 Saam Barati 2017-07-28 01:45:21 PDT
Comment on attachment 316612 [details]
more

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

some brief comments. I'll look more later

> Source/JavaScriptCore/wasm/WasmMemory.cpp:298
> +        if (mprotect(fastMemory + initialBytes, Memory::fastMappedBytes() - initialBytes, PROT_NONE)) {

Do we want an abstraction for this since you use the OSAllocator abstraction above?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:367
> +    bool done = tryAndGC(

nit: I'm not sure what you mean by "done" here. Maybe there is a better name?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:381
> +        memset(newMemory, 0, desiredSize - m_size);

This looks wrong. I think you want (newMemory + m_size) as the ptr to memset. If this didn't fail any tests, perhaps you should add one.

> Source/bmalloc/bmalloc/Gigacage.cpp:35
> +// OOPS! (need bug title)

oops

> Source/bmalloc/bmalloc/Gigacage.cpp:78
> +#endif // BOS(DARWIN) && BCPU(X86_64)

Should be: // GIGACAGE_ENABLED

> Source/bmalloc/bmalloc/Gigacage.h:35
> +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024)

This is unused.
Comment 54 Filip Pizlo 2017-07-28 13:25:45 PDT
(In reply to Build Bot from comment #51)
> Comment on attachment 316612 [details]
> more
> 
> Attachment 316612 [details] did not pass mac-wk2-ews (mac-wk2):
> Output: http://webkit-queues.webkit.org/results/4200372
> 
> New failing tests:
> fast/canvas/webgpu/webgpu-dispatch.html

This is real.  Investigating...
Comment 55 Filip Pizlo 2017-07-28 15:40:57 PDT
Created attachment 316680 [details]
fixed metal
Comment 56 Build Bot 2017-07-28 15:44:01 PDT
Attachment 316680 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:35:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 19 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 57 Sam Weinig 2017-07-28 15:48:42 PDT
For the curious among us, what is the goal / purpose of the gigacage?
Comment 58 Build Bot 2017-07-28 16:59:56 PDT
Comment on attachment 316680 [details]
fixed metal

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

New failing tests:
wasm.yaml/wasm/stress/oom.js.wasm-no-call-ic
Comment 59 Build Bot 2017-07-28 18:19:24 PDT
Comment on attachment 316680 [details]
fixed metal

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

New failing tests:
fast/cookies/cookie-averse-document.html
Comment 60 Build Bot 2017-07-28 18:19:26 PDT
Created attachment 316689 [details]
Archive of layout-test-results from ews100 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 61 Myles C. Maxfield 2017-07-29 00:11:06 PDT
“Gigacage” was the prison they held Professor X in, right?
Comment 62 Filip Pizlo 2017-07-29 12:14:31 PDT
Created attachment 316721 [details]
fix some issues

I think I fixed the crash, the JSC test failure, and the Windows build error.
Comment 63 Build Bot 2017-07-29 12:17:58 PDT
Attachment 316721 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 64 Filip Pizlo 2017-07-29 12:24:50 PDT
(In reply to Saam Barati from comment #53)
> Comment on attachment 316612 [details]
> more
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316612&action=review
> 
> some brief comments. I'll look more later
> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:298
> > +        if (mprotect(fastMemory + initialBytes, Memory::fastMappedBytes() - initialBytes, PROT_NONE)) {
> 
> Do we want an abstraction for this since you use the OSAllocator abstraction
> above?

Nah.  I think that abstractions for this are overkill unless we actually want to port this code to non-POSIX platforms.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:367
> > +    bool done = tryAndGC(
> 
> nit: I'm not sure what you mean by "done" here. Maybe there is a better name?

I changed it to success.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:381
> > +        memset(newMemory, 0, desiredSize - m_size);
> 
> This looks wrong. I think you want (newMemory + m_size) as the ptr to
> memset. If this didn't fail any tests, perhaps you should add one.

Fixed.  I'll look into the test.

> 
> > Source/bmalloc/bmalloc/Gigacage.cpp:35
> > +// OOPS! (need bug title)
> 
> oops

Fixed.

> 
> > Source/bmalloc/bmalloc/Gigacage.cpp:78
> > +#endif // BOS(DARWIN) && BCPU(X86_64)
> 
> Should be: // GIGACAGE_ENABLED

Fixed.

> 
> > Source/bmalloc/bmalloc/Gigacage.h:35
> > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024)
> 
> This is unused.

Oh right, I meant to use that.  I'll fix that.
Comment 65 Filip Pizlo 2017-07-29 12:52:43 PDT
Created attachment 316723 [details]
the patch

Try EWS again now that I fixed the build.  Also, this includes some fixes to address Saam's feedback.
Comment 66 Build Bot 2017-07-29 12:56:38 PDT
Attachment 316723 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 67 Filip Pizlo 2017-07-29 13:29:42 PDT
Created attachment 316726 [details]
trial patch

Attempting to add a test for the WasmMemory bug.  Attempting to fix windows.

This has the WasmMemory bug and the test - I want to see if EWS catches it.
Comment 68 Build Bot 2017-07-29 13:33:03 PDT
Attachment 316726 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:381:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 21 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 69 Filip Pizlo 2017-07-29 13:57:14 PDT
Created attachment 316727 [details]
the patch

Fixed Windows.  Confirmed that the WasmMemory test works.
Comment 70 Build Bot 2017-07-29 13:59:29 PDT
Attachment 316727 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 71 Filip Pizlo 2017-07-29 14:47:52 PDT
Created attachment 316728 [details]
the patch

Fixing more build failures.
Comment 72 Build Bot 2017-07-29 14:50:22 PDT
Attachment 316728 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 73 Filip Pizlo 2017-07-29 15:41:41 PDT
Created attachment 316729 [details]
the patch

More fixes...
Comment 74 Build Bot 2017-07-29 15:44:36 PDT
Attachment 316729 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 75 Filip Pizlo 2017-07-29 17:23:06 PDT
Created attachment 316730 [details]
maybe this will do it
Comment 76 Build Bot 2017-07-29 17:26:55 PDT
Attachment 316730 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:372:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 93 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 77 JF Bastien 2017-07-31 10:19:16 PDT
Comment on attachment 316730 [details]
maybe this will do it

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

Only looked at WebAssembly for now.

The WebAssembly fast memory pre-allocation was critical in getting a fair number of pages when in constrained memory, otherwise our address space was too fragmented and there weren't holes big enough. It looks like gigacage makes that a non-issue because it gets its own huge slab of virtual memory?

This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:146
> +    MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes)

This name is misleading because it doesn't actually allocate anything. It checks whether the allocation is an acceptable size, and then accounts for those bytes.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:282
>          return nullptr;

Here, I initially thought this actually allocated! It just checks whether it's likely to work.

This doesn't take fast memory into account though: it only logs the initial bytes, and doesn't take virtual memory pressure into account. Is that on purpose, since gigacage just has virtual space and it's not a resource worth accounting for anymore?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:296
> +        OSAllocator::commit(fastMemory, initialBytes, true, false);

Can you make those not bools but enums while you're changing code?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:301
> +        }

Why is protection of the tail needed here? It should already be PROT_NONE.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:303
> +        memset(fastMemory, 0, initialBytes);

Why memset? It should already be zero. Is it to force early commit?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:313
> +    void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes);

The previous code first tried allocating maximum if it was provided, and only if that failed did it allocate initial. That's kind of the expectation when providing a maximum. Why drop this?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:318
> +    memset(slowMemory, 0, initialBytes);

Ditto, is this needed?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:334
> +        }

I guess memset is needed because gigacage doesn't guarantee zero pages? Old code used to zero out stuff here.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:379
> +        void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize);

Can you realloc first?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:380
> +        memcpy(newMemory, m_memory, m_size);

newMemory can be nullptr here.
Comment 78 JF Bastien 2017-07-31 10:20:05 PDT
Comment on attachment 316730 [details]
maybe this will do it

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

Only looked at WebAssembly for now.

The WebAssembly fast memory pre-allocation was critical in getting a fair number of pages when in constrained memory, otherwise our address space was too fragmented and there weren't holes big enough. It looks like gigacage makes that a non-issue because it gets its own huge slab of virtual memory?

This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:146
> +    MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes)

This name is misleading because it doesn't actually allocate anything. It checks whether the allocation is an acceptable size, and then accounts for those bytes.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:282
>          return nullptr;

Here, I initially thought this actually allocated! It just checks whether it's likely to work.

This doesn't take fast memory into account though: it only logs the initial bytes, and doesn't take virtual memory pressure into account. Is that on purpose, since gigacage just has virtual space and it's not a resource worth accounting for anymore?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:296
> +        OSAllocator::commit(fastMemory, initialBytes, true, false);

Can you make those not bools but enums while you're changing code?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:301
> +        }

Why is protection of the tail needed here? It should already be PROT_NONE.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:303
> +        memset(fastMemory, 0, initialBytes);

Why memset? It should already be zero. Is it to force early commit?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:313
> +    void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes);

The previous code first tried allocating maximum if it was provided, and only if that failed did it allocate initial. That's kind of the expectation when providing a maximum. Why drop this?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:318
> +    memset(slowMemory, 0, initialBytes);

Ditto, is this needed?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:334
> +        }

I guess memset is needed because gigacage doesn't guarantee zero pages? Old code used to zero out stuff here.

> Source/JavaScriptCore/wasm/WasmMemory.cpp:379
> +        void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize);

Can you realloc first?

> Source/JavaScriptCore/wasm/WasmMemory.cpp:380
> +        memcpy(newMemory, m_memory, m_size);

newMemory can be nullptr here.
Comment 79 Filip Pizlo 2017-07-31 12:51:44 PDT
(In reply to JF Bastien from comment #77)
> Comment on attachment 316730 [details]
> maybe this will do it
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316730&action=review
> 
> Only looked at WebAssembly for now.
> 
> The WebAssembly fast memory pre-allocation was critical in getting a fair
> number of pages when in constrained memory, otherwise our address space was
> too fragmented and there weren't holes big enough. It looks like gigacage
> makes that a non-issue because it gets its own huge slab of virtual memory?

Yeah.

> 
> This loses VM_TAG_FOR_WEBASSEMBLY_MEMORY. Is that desirable?

It's not desirable. :-(  But, if we're in the Gigacage then the memory is already tagged.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:146
> > +    MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes)
> 
> This name is misleading because it doesn't actually allocate anything. It
> checks whether the allocation is an acceptable size, and then accounts for
> those bytes.

Yeah.  Do you have a better name?

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:282
> >          return nullptr;
> 
> Here, I initially thought this actually allocated! It just checks whether
> it's likely to work.
> 
> This doesn't take fast memory into account though: it only logs the initial
> bytes, and doesn't take virtual memory pressure into account. Is that on
> purpose, since gigacage just has virtual space and it's not a resource worth
> accounting for anymore?

We're failing because we know that there isn't enough physical memory for the allocation.

If we succeed here, we may still fail when allocating virtual memory.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:296
> > +        OSAllocator::commit(fastMemory, initialBytes, true, false);
> 
> Can you make those not bools but enums while you're changing code?

I'm not changing OSAllocator. :-)

But I'll name the booleans here.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:301
> > +        }
> 
> Why is protection of the tail needed here? It should already be PROT_NONE.

Nope.  Bmalloc keeps unused memory around as PROT_READ|PROT_WRITE but decommitted.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:303
> > +        memset(fastMemory, 0, initialBytes);
> 
> Why memset? It should already be zero. Is it to force early commit?

Nope.  Bmalloc won't necessarily zero memory you return to it.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:313
> > +    void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes);
> 
> The previous code first tried allocating maximum if it was provided, and
> only if that failed did it allocate initial. That's kind of the expectation
> when providing a maximum. Why drop this?

No evidence that we needed it.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:318
> > +    memset(slowMemory, 0, initialBytes);
> 
> Ditto, is this needed?

Yeah.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:334
> > +        }
> 
> I guess memset is needed because gigacage doesn't guarantee zero pages? Old
> code used to zero out stuff here.

Yeah.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:379
> > +        void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize);
> 
> Can you realloc first?

Bmalloc doesn't have realloc.

> 
> > Source/JavaScriptCore/wasm/WasmMemory.cpp:380
> > +        memcpy(newMemory, m_memory, m_size);
> 
> newMemory can be nullptr here.

Right!  Oops!  Fixed.
Comment 80 JF Bastien 2017-07-31 12:59:24 PDT
> > > Source/JavaScriptCore/wasm/WasmMemory.cpp:146
> > > +    MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes)
> > 
> > This name is misleading because it doesn't actually allocate anything. It
> > checks whether the allocation is an acceptable size, and then accounts for
> > those bytes.
> 
> Yeah.  Do you have a better name?

tryAddingPhysicalMemoryToBookkeeping ?
willThisFitAndIfSoPutItOnTheBooks ?

> > 
> > > Source/JavaScriptCore/wasm/WasmMemory.cpp:282
> > >          return nullptr;
> > 
> > Here, I initially thought this actually allocated! It just checks whether
> > it's likely to work.
> > 
> > This doesn't take fast memory into account though: it only logs the initial
> > bytes, and doesn't take virtual memory pressure into account. Is that on
> > purpose, since gigacage just has virtual space and it's not a resource worth
> > accounting for anymore?
> 
> We're failing because we know that there isn't enough physical memory for
> the allocation.
> 
> If we succeed here, we may still fail when allocating virtual memory.

What I'm saying is: old code used to account for actually allocated memory, as well as virtual allocation. IIUC all the "dead" pages that we allocate just for PROT_NONE never get counted in the updated code.

> > > Source/JavaScriptCore/wasm/WasmMemory.cpp:313
> > > +    void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes);
> > 
> > The previous code first tried allocating maximum if it was provided, and
> > only if that failed did it allocate initial. That's kind of the expectation
> > when providing a maximum. Why drop this?
> 
> No evidence that we needed it.
> 
> > > Source/JavaScriptCore/wasm/WasmMemory.cpp:379
> > > +        void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize);
> > 
> > Can you realloc first?
> 
> Bmalloc doesn't have realloc.

The above two things are potential regressions if users use max, or if they don't but just grow. We indeed have zero data that people do it, but I'm wary of regressing this. I guess we have time to pick u on such cases though.
Comment 81 Mark Lam 2017-07-31 17:22:05 PDT
Comment on attachment 316730 [details]
maybe this will do it

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

Some comments for now (I'm still reading and digesting the patch but need to stop for a bit).

> Source/JavaScriptCore/jsc.cpp:2315
> +    JSObject* result = JSUint8Array::create(exec->vm(), structure, WTFMove(content));

vm is already available.  No need to use exec->vm().

> Source/JavaScriptCore/API/JSTypedArray.cpp:307
> -
> +    

Please undo.

> Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:124
> -
> +        

Please undo.

> Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:49
> -
> +    

Please undo.

> Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:77
> -
> +    

Please undo.

> Source/bmalloc/ChangeLog:8
> +        This adds a mechanism for managing multiple isolated heaps in bmalloc. For now, these isoheaps have a

Can you define the term "isoheaps"?  Is it short for "isolated heaps"?  Is this a term of art?  In scientific parlance, "iso" typically means "same" or "equal".  The first time I read "isoheap", I was thinking same heap (as in no isolation).  But I think you mean the opposite here i.e. isolated heaps.

> Source/bmalloc/bmalloc/Allocator.cpp:188
> -
> +    

Please undo.

> Source/bmalloc/bmalloc/Heap.cpp:267
> -
> +    

Please undo.

> Source/bmalloc/bmalloc/Heap.cpp:526
> -
> +        

Please undo.

> Source/bmalloc/bmalloc/HeapKind.h:35
> +static const unsigned numHeaps = 2;

Let's make this a constexpr.  Sometimes, lldb may decide that static const actually needs linkage to memory storage (and I don't always understand why).

Also, I think in webkit-style, we typically prefer to have this spelled out as numberOfHeaps.

> Source/bmalloc/bmalloc/PerHeapKind.h:49
> +        return reinterpret_cast<T*>(&m_memory)[i];

I think this should be "*reinterpret_cast<T*>(&m_memory[i])".  I looked at http://en.cppreference.com/w/cpp/types/aligned_storage, and I don't see anything that says that it guarantees that sizeof(Memory) == sizeof(T) i.e. it may be that sizeof(Memory) >= sizeof(T).  I presume we used std::aligned_storage in the first place because we want each element to be properly aligned.  Hence, using &m_memory[i] is the right thing to do.

> Source/bmalloc/bmalloc/PerHeapKind.h:54
> +        return reinterpret_cast<T*>(&m_memory)[i];

Ditto.  Same as above.
Comment 82 Filip Pizlo 2017-08-01 10:59:01 PDT
(In reply to JF Bastien from comment #80)
> > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:146
> > > > +    MemoryResult::Kind tryAllocatePhysicalBytes(size_t bytes)
> > > 
> > > This name is misleading because it doesn't actually allocate anything. It
> > > checks whether the allocation is an acceptable size, and then accounts for
> > > those bytes.
> > 
> > Yeah.  Do you have a better name?
> 
> tryAddingPhysicalMemoryToBookkeeping ?
> willThisFitAndIfSoPutItOnTheBooks ?

Meh... those twist my tongue too much.

> 
> > > 
> > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:282
> > > >          return nullptr;
> > > 
> > > Here, I initially thought this actually allocated! It just checks whether
> > > it's likely to work.
> > > 
> > > This doesn't take fast memory into account though: it only logs the initial
> > > bytes, and doesn't take virtual memory pressure into account. Is that on
> > > purpose, since gigacage just has virtual space and it's not a resource worth
> > > accounting for anymore?
> > 
> > We're failing because we know that there isn't enough physical memory for
> > the allocation.
> > 
> > If we succeed here, we may still fail when allocating virtual memory.
> 
> What I'm saying is: old code used to account for actually allocated memory,
> as well as virtual allocation. IIUC all the "dead" pages that we allocate
> just for PROT_NONE never get counted in the updated code.

This code accounts for both also.

> 
> > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:313
> > > > +    void* slowMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), initialBytes);
> > > 
> > > The previous code first tried allocating maximum if it was provided, and
> > > only if that failed did it allocate initial. That's kind of the expectation
> > > when providing a maximum. Why drop this?
> > 
> > No evidence that we needed it.
> > 
> > > > Source/JavaScriptCore/wasm/WasmMemory.cpp:379
> > > > +        void* newMemory = Gigacage::tryAlignedMalloc(WTF::pageSize(), desiredSize);
> > > 
> > > Can you realloc first?
> > 
> > Bmalloc doesn't have realloc.
> 
> The above two things are potential regressions if users use max, or if they
> don't but just grow. We indeed have zero data that people do it, but I'm
> wary of regressing this. I guess we have time to pick u on such cases though.

We can fix it if that becomes a problem. :-)
Comment 83 Filip Pizlo 2017-08-01 11:02:25 PDT
(In reply to Mark Lam from comment #81)
> Comment on attachment 316730 [details]
> maybe this will do it
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316730&action=review
> 
> Some comments for now (I'm still reading and digesting the patch but need to
> stop for a bit).
> 
> > Source/JavaScriptCore/jsc.cpp:2315
> > +    JSObject* result = JSUint8Array::create(exec->vm(), structure, WTFMove(content));
> 
> vm is already available.  No need to use exec->vm().

Fixed.

> 
> > Source/JavaScriptCore/API/JSTypedArray.cpp:307
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/JavaScriptCore/wasm/WasmFaultSignalHandler.cpp:124
> > -
> > +        
> 
> Please undo.

Fixed.

> 
> > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:49
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/JavaScriptCore/wasm/js/JSWebAssemblyMemory.cpp:77
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/bmalloc/ChangeLog:8
> > +        This adds a mechanism for managing multiple isolated heaps in bmalloc. For now, these isoheaps have a
> 
> Can you define the term "isoheaps"?  Is it short for "isolated heaps"?  Is
> this a term of art?  In scientific parlance, "iso" typically means "same" or
> "equal".  The first time I read "isoheap", I was thinking same heap (as in
> no isolation).  But I think you mean the opposite here i.e. isolated heaps.

Fixed.  It's "isolated heaps".

> 
> > Source/bmalloc/bmalloc/Allocator.cpp:188
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/bmalloc/bmalloc/Heap.cpp:267
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/bmalloc/bmalloc/Heap.cpp:526
> > -
> > +        
> 
> Please undo.

Fixed.

> 
> > Source/bmalloc/bmalloc/HeapKind.h:35
> > +static const unsigned numHeaps = 2;
> 
> Let's make this a constexpr.  Sometimes, lldb may decide that static const
> actually needs linkage to memory storage (and I don't always understand why).
> 
> Also, I think in webkit-style, we typically prefer to have this spelled out
> as numberOfHeaps.

Fixed.

> 
> > Source/bmalloc/bmalloc/PerHeapKind.h:49
> > +        return reinterpret_cast<T*>(&m_memory)[i];
> 
> I think this should be "*reinterpret_cast<T*>(&m_memory[i])".  I looked at
> http://en.cppreference.com/w/cpp/types/aligned_storage, and I don't see
> anything that says that it guarantees that sizeof(Memory) == sizeof(T) i.e.
> it may be that sizeof(Memory) >= sizeof(T).  I presume we used
> std::aligned_storage in the first place because we want each element to be
> properly aligned.  Hence, using &m_memory[i] is the right thing to do.
> 
> > Source/bmalloc/bmalloc/PerHeapKind.h:54
> > +        return reinterpret_cast<T*>(&m_memory)[i];
> 
> Ditto.  Same as above.

Fixed and fixed.
Comment 84 Radar WebKit Bug Importer 2017-08-01 11:08:15 PDT
<rdar://problem/33657752>
Comment 85 Filip Pizlo 2017-08-01 12:16:36 PDT
Created attachment 316879 [details]
rebased

Also applied more review feedback.
Comment 86 Build Bot 2017-08-01 12:19:12 PDT
Attachment 316879 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:374:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 94 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 87 Mark Lam 2017-08-01 12:22:46 PDT
Comment on attachment 316730 [details]
maybe this will do it

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

I see that you've rebased with new code.  I'll just post my additional comments here, and then resume reviewing on the new patch.

> Source/bmalloc/bmalloc/Gigacage.cpp:45
> +    Callback()
> +    {
> +    }

nit: you can just put this on 1 line: Callback() { } or Callback() = default;

> Source/bmalloc/bmalloc/Gigacage.h:35
> +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024)

Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"?  Isn't it always intended to be the same size?

> Source/bmalloc/bmalloc/Heap.cpp:59
> +        if (m_kind == HeapKind::Gigacage)
> +            Gigacage::ensureGigacage();

You can put this snippit in the #if GIGACAGE_ENABLED below.  It would have been a call to an empty function anyway.

> Source/bmalloc/bmalloc/Heap.cpp:138
> +    pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0);

You can use PerProcess<Scavenger>::getFastCase() here because the Heap constructor already ensured that the Scavenger is created (line 69 above).

> Source/bmalloc/bmalloc/Heap.cpp:-542
> -    scheduleScavenger(size);

This is a change in logic.  I think Michael did a lot of memory work to discover that this call is needed (in https://trac.webkit.org/r217456).  Unless you have a good reason for removing it, I suggest retaining the call.

However, given that we can deallocate different AllocationKinds, maybe it only make sense to scavenge if allocationKind == Physical?

> Source/bmalloc/bmalloc/Heap.h:119
> +    HeapKind m_kind;

nit: not a big deal, but you can move m_kind down to next to m_isGrowing for better compaction.

> Source/bmalloc/bmalloc/bmalloc.h:68
> +// Returns null of failure

"Returns null for failure"?
Comment 88 Filip Pizlo 2017-08-01 12:31:00 PDT
(In reply to Mark Lam from comment #87)
> Comment on attachment 316730 [details]
> maybe this will do it
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316730&action=review
> 
> I see that you've rebased with new code.  I'll just post my additional
> comments here, and then resume reviewing on the new patch.
> 
> > Source/bmalloc/bmalloc/Gigacage.cpp:45
> > +    Callback()
> > +    {
> > +    }
> 
> nit: you can just put this on 1 line: Callback() { } or Callback() = default;

Fixed.

> 
> > Source/bmalloc/bmalloc/Gigacage.h:35
> > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024)
> 
> Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"?  Isn't it always
> intended to be the same size?

They're not the same size.  GIGACAGE_RUNWAY is a function of array element size (8 bytes) and maximum index size (usually 2^31).

GIGACAGE_SIZE is supposed to be 64GB, not 16GB.  But I just realized that in this patch it's actually 1024GB.  I'll fix!!

> 
> > Source/bmalloc/bmalloc/Heap.cpp:59
> > +        if (m_kind == HeapKind::Gigacage)
> > +            Gigacage::ensureGigacage();
> 
> You can put this snippit in the #if GIGACAGE_ENABLED below.  It would have
> been a call to an empty function anyway.

Fixed.

> 
> > Source/bmalloc/bmalloc/Heap.cpp:138
> > +    pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0);
> 
> You can use PerProcess<Scavenger>::getFastCase() here because the Heap
> constructor already ensured that the Scavenger is created (line 69 above).

Fixed.

> 
> > Source/bmalloc/bmalloc/Heap.cpp:-542
> > -    scheduleScavenger(size);
> 
> This is a change in logic.  I think Michael did a lot of memory work to
> discover that this call is needed (in https://trac.webkit.org/r217456). 
> Unless you have a good reason for removing it, I suggest retaining the call.
> 
> However, given that we can deallocate different AllocationKinds, maybe it
> only make sense to scavenge if allocationKind == Physical?

You're right, I didn't mean to remove the call to scheduleScavenger().  In fact, Geoff had specifically suggested keeping the scheduleScavenger() call in place, even for Physical.  I'll do that!

> 
> > Source/bmalloc/bmalloc/Heap.h:119
> > +    HeapKind m_kind;
> 
> nit: not a big deal, but you can move m_kind down to next to m_isGrowing for
> better compaction.

I'll keep it here for now, just 'cause it makes most logical sense to come first.

> 
> > Source/bmalloc/bmalloc/bmalloc.h:68
> > +// Returns null of failure
> 
> "Returns null for failure"?

Fixed.
Comment 89 Filip Pizlo 2017-08-01 13:01:24 PDT
(In reply to Filip Pizlo from comment #88)
> (In reply to Mark Lam from comment #87)
> > Comment on attachment 316730 [details]
> > maybe this will do it
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=316730&action=review
> > 
> > I see that you've rebased with new code.  I'll just post my additional
> > comments here, and then resume reviewing on the new patch.
> > 
> > > Source/bmalloc/bmalloc/Gigacage.cpp:45
> > > +    Callback()
> > > +    {
> > > +    }
> > 
> > nit: you can just put this on 1 line: Callback() { } or Callback() = default;
> 
> Fixed.
> 
> > 
> > > Source/bmalloc/bmalloc/Gigacage.h:35
> > > +#define GIGACAGE_RUNWAY (16lu * 1024 * 1024 * 1024)
> > 
> > Why not just say "#define GIGACAGE_RUNWAY GIGACAGE_SIZE"?  Isn't it always
> > intended to be the same size?
> 
> They're not the same size.  GIGACAGE_RUNWAY is a function of array element
> size (8 bytes) and maximum index size (usually 2^31).
> 
> GIGACAGE_SIZE is supposed to be 64GB, not 16GB.  But I just realized that in
> this patch it's actually 1024GB.  I'll fix!!
> 
> > 
> > > Source/bmalloc/bmalloc/Heap.cpp:59
> > > +        if (m_kind == HeapKind::Gigacage)
> > > +            Gigacage::ensureGigacage();
> > 
> > You can put this snippit in the #if GIGACAGE_ENABLED below.  It would have
> > been a call to an empty function anyway.
> 
> Fixed.

Turns out that's wrong.  ensureGigacage() is needed for usingGigacage() to return the correct value.

> 
> > 
> > > Source/bmalloc/bmalloc/Heap.cpp:138
> > > +    pthread_set_qos_class_self_np(PerProcess<Scavenger>::get()->requestedScavengerThreadQOSClass(), 0);
> > 
> > You can use PerProcess<Scavenger>::getFastCase() here because the Heap
> > constructor already ensured that the Scavenger is created (line 69 above).
> 
> Fixed.
> 
> > 
> > > Source/bmalloc/bmalloc/Heap.cpp:-542
> > > -    scheduleScavenger(size);
> > 
> > This is a change in logic.  I think Michael did a lot of memory work to
> > discover that this call is needed (in https://trac.webkit.org/r217456). 
> > Unless you have a good reason for removing it, I suggest retaining the call.
> > 
> > However, given that we can deallocate different AllocationKinds, maybe it
> > only make sense to scavenge if allocationKind == Physical?
> 
> You're right, I didn't mean to remove the call to scheduleScavenger().  In
> fact, Geoff had specifically suggested keeping the scheduleScavenger() call
> in place, even for Physical.  I'll do that!
> 
> > 
> > > Source/bmalloc/bmalloc/Heap.h:119
> > > +    HeapKind m_kind;
> > 
> > nit: not a big deal, but you can move m_kind down to next to m_isGrowing for
> > better compaction.
> 
> I'll keep it here for now, just 'cause it makes most logical sense to come
> first.
> 
> > 
> > > Source/bmalloc/bmalloc/bmalloc.h:68
> > > +// Returns null of failure
> > 
> > "Returns null for failure"?
> 
> Fixed.
Comment 90 Filip Pizlo 2017-08-01 13:13:07 PDT
Created attachment 316887 [details]
with more fixes

Fixed the GIGACAGE_MASK so that it's 64GB for reals.
Comment 91 Build Bot 2017-08-01 13:15:27 PDT
Attachment 316887 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:281:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/JavaScriptCore/wasm/WasmMemory.cpp:374:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/bmalloc/bmalloc/Scavenger.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:119:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/JavaScriptCore/runtime/ArrayBuffer.cpp:190:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
ERROR: Source/bmalloc/bmalloc/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/JavaScriptCore/heap/Subspace.h:63:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.cpp:30:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/WTF/wtf/Gigacage.cpp:36:  g_gigacageBasePtr is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/graphics/cocoa/GPUBufferMetal.mm:52:  More than one command on the same line  [whitespace/newline] [4]
ERROR: Source/WTF/wtf/Gigacage.h:53:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:56:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:57:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:65:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:70:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WTF/wtf/Gigacage.h:71:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/bmalloc/bmalloc/Heap.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
ERROR: Source/JavaScriptCore/runtime/VM.cpp:425:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/heap/GigacageSubspace.h:40:  The parameter name "size" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 20 in 94 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 92 Mark Lam 2017-08-01 13:31:33 PDT
Comment on attachment 316879 [details]
rebased

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

I think some of the style checker complaints are valid.  Would be nice to fix the valid (especially trivial low risk) ones before landing.

I have a few more files to review, but just want to post what I have so far first.

> Source/JavaScriptCore/heap/SubspaceInlines.h:57
> +            allocator.forEachBlock(func);

You've changed this from forEachNotEmptyBlock to forEachBlock.  Is this intentional, or is this a copy-paste error?

> Source/JavaScriptCore/runtime/ArrayBuffer.cpp:221
> -    contents.tryAllocate(byteLength, 1, ArrayBufferContents::ZeroInitialize);
> +    contents.tryAllocate(byteLength, 1, ArrayBufferContents::DontInitialize);

I think you did this intentionally because createInternal() will full the bits with data below.  But just in case, I thought I'd flag it.

> Source/JavaScriptCore/runtime/VM.h:761
> -
> +    

Please undo.

> Source/bmalloc/bmalloc/Gigacage.cpp:94
> +    Callbacks& callbacks = *PerProcess<Callbacks>::get();
> +    std::unique_lock<StaticMutex> lock(PerProcess<Callbacks>::mutex());
> +    for (Callback& callback : callbacks.callbacks)
> +        callback.function(callback.argument);
> +    callbacks.callbacks.shrink(0);

I don't see the part where you actually disable the gigacage and clear g_gigacageBasePtr.
Comment 93 Filip Pizlo 2017-08-01 13:35:10 PDT
(In reply to Mark Lam from comment #92)
> Comment on attachment 316879 [details]
> rebased
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316879&action=review
> 
> I think some of the style checker complaints are valid.  Would be nice to
> fix the valid (especially trivial low risk) ones before landing.
> 
> I have a few more files to review, but just want to post what I have so far
> first.
> 
> > Source/JavaScriptCore/heap/SubspaceInlines.h:57
> > +            allocator.forEachBlock(func);
> 
> You've changed this from forEachNotEmptyBlock to forEachBlock.  Is this
> intentional, or is this a copy-paste error?

It's an error.  Fixed.

It would have been correctness-neutral (it's always OK to also visit empty blocks) but it might have perf implications.

> 
> > Source/JavaScriptCore/runtime/ArrayBuffer.cpp:221
> > -    contents.tryAllocate(byteLength, 1, ArrayBufferContents::ZeroInitialize);
> > +    contents.tryAllocate(byteLength, 1, ArrayBufferContents::DontInitialize);
> 
> I think you did this intentionally because createInternal() will full the
> bits with data below.  But just in case, I thought I'd flag it.

Yeah, I think this is right.

> 
> > Source/JavaScriptCore/runtime/VM.h:761
> > -
> > +    
> 
> Please undo.

Fixed.

> 
> > Source/bmalloc/bmalloc/Gigacage.cpp:94
> > +    Callbacks& callbacks = *PerProcess<Callbacks>::get();
> > +    std::unique_lock<StaticMutex> lock(PerProcess<Callbacks>::mutex());
> > +    for (Callback& callback : callbacks.callbacks)
> > +        callback.function(callback.argument);
> > +    callbacks.callbacks.shrink(0);
> 
> I don't see the part where you actually disable the gigacage and clear
> g_gigacageBasePtr.

Fixed.
Comment 94 Build Bot 2017-08-01 14:59:31 PDT
Comment on attachment 316887 [details]
with more fixes

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

New failing tests:
imported/blink/fast/dom/HTMLImageElement/image-sizes-viewport-with-template-parent-and-empty-sizes.html
Comment 95 Build Bot 2017-08-01 14:59:33 PDT
Created attachment 316900 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 96 Build Bot 2017-08-01 15:02:59 PDT
Comment on attachment 316887 [details]
with more fixes

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

New failing tests:
fast/dom/Window/HTMLBodyElement-window-eventListener-attributes.html
Comment 97 Build Bot 2017-08-01 15:03:01 PDT
Created attachment 316901 [details]
Archive of layout-test-results from ews117 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 98 Filip Pizlo 2017-08-01 17:22:05 PDT
(In reply to Build Bot from comment #97)
> Created attachment 316901 [details]
> Archive of layout-test-results from ews117 for mac-elcapitan
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-debug-ews.
> Bot: ews117  Port: mac-elcapitan  Platform: Mac OS X 10.11.6

These crashes look unrelated to my change.  Looks like this is a regression in trunk.
Comment 99 Saam Barati 2017-08-01 17:34:03 PDT
Comment on attachment 316887 [details]
with more fixes

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

> Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:63
> +                    case GetByOffset:
> +                    case PutByOffset:
> +                    case GetGetterSetterByOffset:
> +                    case GetArrayLength:
> +                    case GetVectorLength:

I'm not sure I understand why we can whitelist these. What happens if some attacker overwrote the butterfly pointer in the JSObject. This would just trust the value that's there?

> Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:76
> +                        if (!readsOverlap(m_graph, node, Heap))
> +                            break;

I'm not sure this matters in practice given how Clobberize is written today, but I'd argue that this should also consider writes into the Heap, not just reads.
Comment 100 Mark Lam 2017-08-01 17:35:46 PDT
Comment on attachment 316887 [details]
with more fixes

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

I don't understand how !readsOverlap(m_graph, node, Heap) means that we don't need to cage the read of the butterfly pointer.  Can you explain?

Everything else looks good to me.

> Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58
> +                        // Check if this is needed:

This comment is misaligned.

> Source/JavaScriptCore/heap/MarkedAllocator.cpp:108
> +            RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace));
> +            RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace()));

Do these really need to be RELEASE_ASSERTs?  All we're doing here is confirming that no one broke findEmptyBlockToSteal().  I feel that a debug ASSERT would do.
Comment 101 Filip Pizlo 2017-08-01 17:42:35 PDT
(In reply to Mark Lam from comment #100)
> Comment on attachment 316887 [details]
> with more fixes
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=316887&action=review
> 
> I don't understand how !readsOverlap(m_graph, node, Heap) means that we
> don't need to cage the read of the butterfly pointer.  Can you explain?

We only need to do caging if we do indexes access to the butterfly.

If a node does not read from the heap then it does not do indexed accesses to the butterfly.

> 
> Everything else looks good to me.
> 
> > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58
> > +                        // Check if this is needed:
> 
> This comment is misaligned.

I fixed it and turned it into a proper FIXME.

> 
> > Source/JavaScriptCore/heap/MarkedAllocator.cpp:108
> > +            RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace));
> > +            RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace()));
> 
> Do these really need to be RELEASE_ASSERTs?  All we're doing here is
> confirming that no one broke findEmptyBlockToSteal().  I feel that a debug
> ASSERT would do.

If we get this wrong then bmalloc craps itself in horrible ways.  Like, it often just deadlocks.

So, I was being extra paranoid.
Comment 102 Filip Pizlo 2017-08-01 17:49:58 PDT
(In reply to Filip Pizlo from comment #101)
> (In reply to Mark Lam from comment #100)
> > Comment on attachment 316887 [details]
> > with more fixes
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=316887&action=review
> > 
> > I don't understand how !readsOverlap(m_graph, node, Heap) means that we
> > don't need to cage the read of the butterfly pointer.  Can you explain?
> 
> We only need to do caging if we do indexes access to the butterfly.
> 
> If a node does not read from the heap then it does not do indexed accesses
> to the butterfly.

But what if it writes instead of reads.

I'm going to change this to accessesOverlap.

> 
> > 
> > Everything else looks good to me.
> > 
> > > Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:58
> > > +                        // Check if this is needed:
> > 
> > This comment is misaligned.
> 
> I fixed it and turned it into a proper FIXME.
> 
> > 
> > > Source/JavaScriptCore/heap/MarkedAllocator.cpp:108
> > > +            RELEASE_ASSERT(block->subspace()->canTradeBlocksWith(m_subspace));
> > > +            RELEASE_ASSERT(m_subspace->canTradeBlocksWith(block->subspace()));
> > 
> > Do these really need to be RELEASE_ASSERTs?  All we're doing here is
> > confirming that no one broke findEmptyBlockToSteal().  I feel that a debug
> > ASSERT would do.
> 
> If we get this wrong then bmalloc craps itself in horrible ways.  Like, it
> often just deadlocks.
> 
> So, I was being extra paranoid.
Comment 103 Mark Lam 2017-08-01 18:29:48 PDT
Comment on attachment 316887 [details]
with more fixes

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

r=me with remaining issues resolved.

> Source/JavaScriptCore/dfg/DFGFixedButterflyAccessUncagingPhase.cpp:75
> +                        if (!readsOverlap(m_graph, node, Heap))

We discussed offline that this should be accessOverlaps instead.
Comment 104 Filip Pizlo 2017-08-01 18:50:58 PDT
Landed in https://trac.webkit.org/changeset/220118/webkit
Comment 105 Saam Barati 2017-08-04 18:56:32 PDT
This patch regressed ARES-6 by 2% on Mac.
Comment 106 Saam Barati 2017-08-04 19:05:48 PDT
(In reply to Saam Barati from comment #105)
> This patch regressed ARES-6 by 2% on Mac.

I filed:
https://bugs.webkit.org/show_bug.cgi?id=175234