Bug 188577

Summary:

Fix exception throwing code so that topCallFrame and topEntryFrame stay true to their names.

Product:

WebKit

Reporter:

Mark Lam <mark.lam>

Component:

JavaScriptCore

Assignee:

Mark Lam <mark.lam>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ews-watchlist, fpizlo, keith_miller, msaboff, rmorisset, rniwa, saam, tzagallo, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch.	mark.lam: review-, ews-watchlist: commit-queue-
x86_64 benchmark results on a 13" MBP.	none
Archive of layout-test-results from ews102 for mac-sierra	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
Archive of layout-test-results from ews116 for mac-sierra	none
patch for EWS testing.	ews-watchlist: commit-queue-
Archive of layout-test-results from ews101 for mac-sierra	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
candidate patch for EWS testing.	ews-watchlist: commit-queue-
Archive of layout-test-results from ews117 for mac-sierra	none
patch for EWS testing.	ews-watchlist: commit-queue-
Archive of layout-test-results from ews116 for mac-sierra	none
patch for EWS testing.	none
proposed patch.	saam: review+, ews-watchlist: commit-queue-
Archive of layout-test-results from ews107 for mac-sierra-wk2	none

Mark Lam

Reported 2018-08-14 14:02:58 PDT

Details to come.

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
Show Obsolete (13) View All Add attachment proposed patch, testcase, etc.

Mark Lam

Comment 1 2018-08-16 14:58:00 PDT

<rdar://problem/42985684>

Mark Lam

Comment 2 2018-08-16 16:36:57 PDT

Created attachment 347320 [details] proposed patch.

Mark Lam

Comment 3 2018-08-16 16:37:41 PDT

Created attachment 347321 [details] x86_64 benchmark results on a 13" MBP.

Mark Lam

Comment 4 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.

EWS Watchlist

Comment 5 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

EWS Watchlist

Comment 6 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

EWS Watchlist

Comment 7 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

EWS Watchlist

Comment 8 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

EWS Watchlist

Comment 9 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

EWS Watchlist

Comment 10 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

EWS Watchlist

Comment 11 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

Mark Lam

Comment 12 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.

Mark Lam

Comment 13 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.

EWS Watchlist

Comment 14 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

EWS Watchlist

Comment 15 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

EWS Watchlist

Comment 16 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

EWS Watchlist

Comment 17 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

EWS Watchlist

Comment 18 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

Mark Lam

Comment 19 2018-08-20 12:06:47 PDT

Created attachment 347523 [details] candidate patch for EWS testing.

EWS Watchlist

Comment 20 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.

EWS Watchlist

Comment 21 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

EWS Watchlist

Comment 22 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

EWS Watchlist

Comment 23 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

Mark Lam

Comment 24 2018-08-20 16:57:57 PDT

Created attachment 347574 [details] patch for EWS testing.

EWS Watchlist

Comment 25 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.

EWS Watchlist

Comment 26 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

EWS Watchlist

Comment 27 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

EWS Watchlist

Comment 28 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

Mark Lam

Comment 29 2018-08-24 10:33:29 PDT

Created attachment 348021 [details] patch for EWS testing.

Mark Lam

Comment 30 2018-08-24 15:33:55 PDT

Created attachment 348047 [details] proposed patch. Let's get some EWS testing first.

EWS Watchlist

Comment 31 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

EWS Watchlist

Comment 32 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

Saam Barati

Comment 33 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.

Mark Lam

Comment 34 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>.

Note You need to log in before you can comment on or make changes to this bug.