Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error message for O.p.toString on revoked proxy #3424

Closed
wants to merge 1 commit into from

Conversation

serakeri
Copy link
Contributor

@serakeri serakeri commented Aug 17, 2022

https://bugs.webkit.org/show_bug.cgi?id=242342

Reviewed by NOBODY (OOPS!).

* JSTests/stress/array-isarray-error-message.js: Added.
* Source/JavaScriptCore/runtime/ArrayConstructor.cpp:
* Source/JavaScriptCore/runtime/Intrinsic.cpp:
* Source/JavaScriptCore/runtime/Intrinsic.h:
* Source/JavaScriptCore/runtime/JSGlobalObject.cpp:
@serakeri serakeri requested a review from a team as a code owner August 17, 2022 21:02
@serakeri serakeri self-assigned this Aug 17, 2022
@serakeri serakeri added JavaScriptCore For bugs in JavaScriptCore, the JS engine used by WebKit, other than kxmlcore issues. Safari 15 labels Aug 17, 2022
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 17, 2022
@caitp
Copy link
Contributor

caitp commented Aug 18, 2022

So there are a few ways this could be done:

  • Have a generic not-throwing version of Array.isArray which returns a status enum that the caller can map to an error message, and have the caller throw the error
  • Get the actual JS code used to invoke the function from a SourceProvider for the appropriate frame, and use that to output a message (I don't think anything does this for runtime errors in JSC?)
  • What is being done here (getting the CallFrame preceding whatever invoked IsArray, and testing the function --- Using an Intrinsic ID is a novel way to do this), and using that to render a canonical version of the caller function.

I'm not sure which is the best, they all seem to have pros and cons in terms of robustness, complexity and message clarity. This seems like a happy medium

@serakeri
Copy link
Contributor Author

serakeri commented Sep 1, 2022

Moved to #3923

@serakeri serakeri closed this Sep 1, 2022
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. merging-blocked Applied to prevent a change from being merged
Projects
None yet
4 participants