Bug 188577 - Fix exception throwing code so that topCallFrame and topEntryFrame stay true to their names.
Summary: Fix exception throwing code so that topCallFrame and topEntryFrame stay true ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-14 14:02 PDT by Mark Lam
Modified: 2018-08-27 22:03 PDT (History)
10 users (show)

See Also:


Attachments
proposed patch. (63.62 KB, patch)
2018-08-16 16:36 PDT, Mark Lam
mark.lam: review-
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
x86_64 benchmark results on a 13" MBP. (95.80 KB, text/plain)
2018-08-16 16:37 PDT, Mark Lam
no flags Details
Archive of layout-test-results from ews102 for mac-sierra (2.35 MB, application/zip)
2018-08-16 17:55 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.82 MB, application/zip)
2018-08-16 18:05 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews116 for mac-sierra (3.08 MB, application/zip)
2018-08-16 18:31 PDT, EWS Watchlist
no flags Details
patch for EWS testing. (63.62 KB, patch)
2018-08-17 10:03 PDT, Mark Lam
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.36 MB, application/zip)
2018-08-17 11:23 PDT, EWS Watchlist
no flags Details
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.95 MB, application/zip)
2018-08-17 11:33 PDT, EWS Watchlist
no flags Details
candidate patch for EWS testing. (60.93 KB, patch)
2018-08-20 12:06 PDT, Mark Lam
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews117 for mac-sierra (3.22 MB, application/zip)
2018-08-20 14:26 PDT, EWS Watchlist
no flags Details
patch for EWS testing. (98.62 KB, patch)
2018-08-20 16:57 PDT, Mark Lam
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews116 for mac-sierra (3.19 MB, application/zip)
2018-08-20 20:10 PDT, EWS Watchlist
no flags Details
patch for EWS testing. (66.85 KB, patch)
2018-08-24 10:33 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (65.72 KB, patch)
2018-08-24 15:33 PDT, Mark Lam
saam: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (3.08 MB, application/zip)
2018-08-24 16:51 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2018-08-14 14:02:58 PDT
Details to come.
Comment 1 Mark Lam 2018-08-16 14:58:00 PDT
<rdar://problem/42985684>
Comment 2 Mark Lam 2018-08-16 16:36:57 PDT
Created attachment 347320 [details]
proposed patch.
Comment 3 Mark Lam 2018-08-16 16:37:41 PDT
Created attachment 347321 [details]
x86_64 benchmark results on a 13" MBP.
Comment 4 Mark Lam 2018-08-16 16:47:37 PDT
I forgot to say: see the ChangeLog in the patch for details of what change is being proposed and why.
Comment 5 EWS Watchlist 2018-08-16 17:55:18 PDT
Comment on attachment 347320 [details]
proposed patch.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 6 EWS Watchlist 2018-08-16 17:55:20 PDT
Created attachment 347329 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 7 EWS Watchlist 2018-08-16 17:57:56 PDT
Comment on attachment 347320 [details]
proposed patch.

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

New failing tests:
stress/regress-188577.js.no-ftl
stress/regress-188577.js.dfg-eager-no-cjit-validate
stress/regress-188577.js.ftl-eager-no-cjit
stress/regress-188577.js.ftl-eager-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-small-pool
stress/regress-188577.js.default
stress/regress-188577.js.no-cjit-validate-phases
stress/regress-188577.js.ftl-no-cjit-no-inline-validate
stress/regress-188577.js.dfg-eager
stress/regress-188577.js.no-cjit-collect-continuously
stress/regress-188577.js.ftl-no-cjit-no-put-stack-validate
stress/regress-188577.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-188577.js.ftl-eager
stress/regress-188577.js.dfg-maximal-flush-validate-no-cjit
apiTests
Comment 8 EWS Watchlist 2018-08-16 18:05:40 PDT
Comment on attachment 347320 [details]
proposed patch.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 9 EWS Watchlist 2018-08-16 18:05:42 PDT
Created attachment 347330 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 10 EWS Watchlist 2018-08-16 18:31:32 PDT
Comment on attachment 347320 [details]
proposed patch.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 11 EWS Watchlist 2018-08-16 18:31:33 PDT
Created attachment 347332 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 12 Mark Lam 2018-08-17 09:53:03 PDT
Comment on attachment 347320 [details]
proposed patch.

Need to investigate test failures to see what bug crept in.
Comment 13 Mark Lam 2018-08-17 10:03:53 PDT
Created attachment 347364 [details]
patch for EWS testing.

I can't repro the JSC test failure locally.  Let's try a test tweak on the EWS.
Comment 14 EWS Watchlist 2018-08-17 11:23:29 PDT
Comment on attachment 347364 [details]
patch for EWS testing.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 15 EWS Watchlist 2018-08-17 11:23:31 PDT
Created attachment 347370 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 16 EWS Watchlist 2018-08-17 11:30:32 PDT
Comment on attachment 347364 [details]
patch for EWS testing.

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

New failing tests:
stress/regress-188577.js.no-ftl
stress/regress-188577.js.dfg-eager-no-cjit-validate
stress/regress-188577.js.ftl-eager-no-cjit
stress/regress-188577.js.ftl-eager-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-small-pool
stress/regress-188577.js.default
stress/regress-188577.js.no-cjit-validate-phases
stress/regress-188577.js.ftl-no-cjit-no-inline-validate
stress/regress-188577.js.dfg-eager
stress/regress-188577.js.no-cjit-collect-continuously
stress/regress-188577.js.ftl-no-cjit-no-put-stack-validate
stress/regress-188577.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-188577.js.ftl-eager
stress/regress-188577.js.dfg-maximal-flush-validate-no-cjit
apiTests
Comment 17 EWS Watchlist 2018-08-17 11:33:04 PDT
Comment on attachment 347364 [details]
patch for EWS testing.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 18 EWS Watchlist 2018-08-17 11:33:06 PDT
Created attachment 347372 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 19 Mark Lam 2018-08-20 12:06:47 PDT
Created attachment 347523 [details]
candidate patch for EWS testing.
Comment 20 EWS Watchlist 2018-08-20 12:09:55 PDT
Attachment 347523 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/interpreter/CallFrame.h:266:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
ERROR: Source/JavaScriptCore/runtime/VM.h:302:  The parameter name "callFrame" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 32 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 EWS Watchlist 2018-08-20 13:28:36 PDT
Comment on attachment 347523 [details]
candidate patch for EWS testing.

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

New failing tests:
stress/regress-188577.js.no-ftl
stress/regress-188577.js.dfg-eager-no-cjit-validate
stress/regress-188577.js.ftl-eager-no-cjit
stress/regress-188577.js.ftl-eager-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-small-pool
stress/regress-188577.js.default
stress/regress-188577.js.no-cjit-validate-phases
stress/regress-188577.js.ftl-no-cjit-no-inline-validate
stress/regress-188577.js.dfg-eager
stress/regress-188577.js.no-cjit-collect-continuously
stress/regress-188577.js.ftl-no-cjit-no-put-stack-validate
stress/regress-188577.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-188577.js.ftl-eager
stress/regress-188577.js.dfg-maximal-flush-validate-no-cjit
apiTests
Comment 22 EWS Watchlist 2018-08-20 14:26:43 PDT
Comment on attachment 347523 [details]
candidate patch for EWS testing.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 23 EWS Watchlist 2018-08-20 14:26:45 PDT
Created attachment 347542 [details]
Archive of layout-test-results from ews117 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews117  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 24 Mark Lam 2018-08-20 16:57:57 PDT
Created attachment 347574 [details]
patch for EWS testing.
Comment 25 EWS Watchlist 2018-08-20 18:27:30 PDT
Attachment 347574 [details] did not pass style-queue:


ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:190:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:191:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:203:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:205:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:206:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/JavaScriptCore/interpreter/CallFrame.cpp:216:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 6 in 52 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 26 EWS Watchlist 2018-08-20 20:07:06 PDT
Comment on attachment 347574 [details]
patch for EWS testing.

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

New failing tests:
stress/regress-188577.js.no-ftl
stress/regress-188577.js.dfg-eager-no-cjit-validate
stress/regress-188577.js.ftl-eager-no-cjit
stress/regress-188577.js.ftl-eager-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-b3o1
stress/regress-188577.js.ftl-no-cjit-small-pool
stress/regress-188577.js.default
stress/regress-188577.js.no-cjit-validate-phases
stress/regress-188577.js.ftl-no-cjit-no-inline-validate
stress/regress-188577.js.dfg-eager
stress/regress-188577.js.no-cjit-collect-continuously
stress/regress-188577.js.ftl-no-cjit-no-put-stack-validate
stress/regress-188577.js.ftl-no-cjit-validate-sampling-profiler
stress/regress-188577.js.ftl-eager
stress/regress-188577.js.dfg-maximal-flush-validate-no-cjit
apiTests
Comment 27 EWS Watchlist 2018-08-20 20:10:49 PDT
Comment on attachment 347574 [details]
patch for EWS testing.

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

New failing tests:
http/tests/misc/large-js-program.php
Comment 28 EWS Watchlist 2018-08-20 20:10:51 PDT
Created attachment 347604 [details]
Archive of layout-test-results from ews116 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 29 Mark Lam 2018-08-24 10:33:29 PDT
Created attachment 348021 [details]
patch for EWS testing.
Comment 30 Mark Lam 2018-08-24 15:33:55 PDT
Created attachment 348047 [details]
proposed patch.

Let's get some EWS testing first.
Comment 31 EWS Watchlist 2018-08-24 16:51:54 PDT
Comment on attachment 348047 [details]
proposed patch.

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

New failing tests:
css3/filters/backdrop/add-remove-add-backdrop-filter.html
Comment 32 EWS Watchlist 2018-08-24 16:51:56 PDT
Created attachment 348059 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6
Comment 33 Saam Barati 2018-08-27 18:03:45 PDT
Comment on attachment 348047 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:314
>      loadp VM::callFrameForCatch[t3], cfr

this LOC can now be removed.
Comment 34 Mark Lam 2018-08-27 22:03:52 PDT
Thanks for the review.

(In reply to Build Bot from comment #31)
> New failing tests:
> css3/filters/backdrop/add-remove-add-backdrop-filter.html

FYI, this test failure is unrelated.  It's an image diff which has nothing to do with this patch.

(In reply to Saam Barati from comment #33)
> > Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:314
> >      loadp VM::callFrameForCatch[t3], cfr
> 
> this LOC can now be removed.

I've removed this dead code both in LowLevelInterpreter32_64.asm and LowLevelInterpreter64.asm.

Landed in r235419: <http://trac.webkit.org/r235419>.