Skip to content

Add support for Error.captureStackTrace#16790

Merged
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
kmiller68:eng/Add-support-for-Error-captureStackTrace
Aug 17, 2023
Merged

Add support for Error.captureStackTrace#16790
webkit-commit-queue merged 1 commit into
WebKit:mainfrom
kmiller68:eng/Add-support-for-Error-captureStackTrace

Conversation

@kmiller68

@kmiller68 kmiller68 commented Aug 17, 2023

Copy link
Copy Markdown
Contributor

997e074

Add support for Error.captureStackTrace
https://bugs.webkit.org/show_bug.cgi?id=260326
rdar://113767788

Reviewed by Yusuke Suzuki.

This change adds support for the Error.captureStackTrace extension. We mostly want it because web tooling benchamrk
uses it in the hot path of the chai-wtb test, where having native Error.captureStackTrace is a 5% progression over the polyfill.

Error.captureStackTrace(obj[, caller]) sets the "stack" property on obj with a string of the current callstack. If caller is provided
then any frames above (textually, below in the machine stack) or at the first reference to caller are omitted from the trace.

Our implementation should behave the same as v8's with two exceptions:
1) The stack trace is formatted the same as our Error.stack, not v8's.
2) v8 makes the stack property a getter/setter whereas we make it a data property, like our Error object.

* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::Interpreter::getStackTrace):
* Source/JavaScriptCore/interpreter/Interpreter.h:
* Source/JavaScriptCore/runtime/CommonIdentifiers.h:
* Source/JavaScriptCore/runtime/Error.cpp:
(JSC::getStackTrace):
* Source/JavaScriptCore/runtime/ErrorConstructor.cpp:
(JSC::ErrorConstructor::finishCreation):
(JSC::JSC_DEFINE_HOST_FUNCTION):

Canonical link: https://commits.webkit.org/267010@main

a23a602

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🛠 gtk
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🧪 gtk-wk2
✅ 🛠 🧪 jsc ✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ✅ 🧪 jsc-armv7-tests
✅ 🛠 🧪 merge ✅ 🛠 watch ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@kmiller68 kmiller68 requested a review from a team as a code owner August 17, 2023 14:58
@kmiller68 kmiller68 self-assigned this Aug 17, 2023
@kmiller68 kmiller68 added the JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. label Aug 17, 2023
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 17, 2023
@kmiller68 kmiller68 removed the merging-blocked Applied to prevent a change from being merged label Aug 17, 2023
@kmiller68 kmiller68 force-pushed the eng/Add-support-for-Error-captureStackTrace branch from 448a461 to e125c84 Compare August 17, 2023 16:43

@Constellation Constellation left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r=me

@kmiller68 kmiller68 force-pushed the eng/Add-support-for-Error-captureStackTrace branch from e125c84 to a23a602 Compare August 17, 2023 17:04
@kmiller68 kmiller68 added the merge-queue Applied to send a pull request to merge-queue label Aug 17, 2023
https://bugs.webkit.org/show_bug.cgi?id=260326
rdar://113767788

Reviewed by Yusuke Suzuki.

This change adds support for the Error.captureStackTrace extension. We mostly want it because web tooling benchamrk
uses it in the hot path of the chai-wtb test, where having native Error.captureStackTrace is a 5% progression over the polyfill.

Error.captureStackTrace(obj[, caller]) sets the "stack" property on obj with a string of the current callstack. If caller is provided
then any frames above (textually, below in the machine stack) or at the first reference to caller are omitted from the trace.

Our implementation should behave the same as v8's with two exceptions:
1) The stack trace is formatted the same as our Error.stack, not v8's.
2) v8 makes the stack property a getter/setter whereas we make it a data property, like our Error object.

* Source/JavaScriptCore/interpreter/Interpreter.cpp:
(JSC::Interpreter::getStackTrace):
* Source/JavaScriptCore/interpreter/Interpreter.h:
* Source/JavaScriptCore/runtime/CommonIdentifiers.h:
* Source/JavaScriptCore/runtime/Error.cpp:
(JSC::getStackTrace):
* Source/JavaScriptCore/runtime/ErrorConstructor.cpp:
(JSC::ErrorConstructor::finishCreation):
(JSC::JSC_DEFINE_HOST_FUNCTION):

Canonical link: https://commits.webkit.org/267010@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Add-support-for-Error-captureStackTrace branch from a23a602 to 997e074 Compare August 17, 2023 20:05
@webkit-commit-queue webkit-commit-queue merged commit 997e074 into WebKit:main Aug 17, 2023
@webkit-commit-queue

Copy link
Copy Markdown
Collaborator

Committed 267010@main (997e074): https://commits.webkit.org/267010@main

Reviewed commits have been landed. Closing PR #16790 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Aug 17, 2023
@Jarred-Sumner

Copy link
Copy Markdown

There's a second argument along with Error.prepareStackTrace which allows custom formatting. More complicated though

@kmiller68

Copy link
Copy Markdown
Contributor Author

Interesting! I missed Error.prepareStackTrace when I was reading about v8 errors. Do you think Error.prepareStackTrace is important for compatibility?

@Jarred-Sumner

Jarred-Sumner commented Aug 17, 2023 via email

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants