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.

Sign up for GitHub

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

Jump to bottom

Wasm error stack traces are too similar to JS traces #9205

Closed
ghost opened this issue Jan 21, 2021 · 7 comments
Closed

Wasm error stack traces are too similar to JS traces #9205

ghost opened this issue Jan 21, 2021 · 7 comments
Labels
stale

Comments

@ghost
Copy link

ghost commented Jan 21, 2021 •
edited by ghost
Loading

Example script:

const { loop } = (await WebAssembly.instantiate(
	BigUint64Array.of(
		0x16D736100n,
		0x203000060010401n,
		0x6F6C040108070001n,
		0x401060A0000706Fn,
		0x6E040A000B001000n,
		0x10302656D61n
	)
)).instance.exports;

loop();

loop is a function that just recurses into itself, so we can get a RangeError and a stack trace.

Chrome gives this stack trace:

RangeError: Maximum call stack size exceeded
  at loop (<anonymous>:wasm-function[0]:0x20)
  at loop (<anonymous>:wasm-function[0]:0x21)
  ...
  at loop (<anonymous>:wasm-function[0]:0x21)

This tells me that the first function in my Wasm module threw the error, at byte 0x21, and that it was called via the JS identifier loop.

Here's Deno's stack trace:

error: Uncaught RangeError: Maximum call stack size exceeded
  at loop (<anonymous>:0:32)
  at loop (<anonymous>:0:33)
    ...
  at loop (<anonymous>:0:33)

Personally, I'd say that this is very misleading, binary blobs don't exactly have columns and rows (0:33).
I would consider that part to be a bug.

Otherwise, imo, it would be great to have the common wasm-function[N] that FF and Chromium show in the stack trace, for debugging.

Also, I believe that double <anonymous> here is unnecessary:

WebAssembly.instantiate(
	Uint8Array.of(
		0x0, 0x61, 0x73, 0x6D,
		0x1, 0x0, 0x0, 0x0,
		0x1, 0x4, 0x1, 0x60,
		0x0, 0x0, 0x3, 0x2,
		0x1, 0x0, 0x8, 0x1,
		0x0, 0xA, 0x6, 0x1,
		0x4, 0x0, 0x10, 0x0,
		0xB, 0x0, 0xA, 0x4,
		0x6E, 0x61, 0x6D, 0x65,
		0x2, 0x3, 0x1, 0x0,
		0x0
	)
);

Deno:

error: Uncaught (in promise) RangeError: Maximum call stack size exceeded
  at <anonymous> (<anonymous>:0:25)
  at <anonymous> (<anonymous>:0:26)
  ...
  at <anonymous> (<anonymous>:0:26)

Chromium:

RangeError: Maximum call stack size exceeded
  at <anonymous>:wasm-function[0]:0x19
  at <anonymous>:wasm-function[0]:0x1a
  ...
  at <anonymous>:wasm-function[0]:0x1a

what was the reasoning for overriding Error.prepareStackTrace in the first place?
@bnoordhuis
Copy link
Contributor

what was the reasoning for overriding Error.prepareStackTrace in the first place?

#7644 explains it and why it's not ideal. The idea is to RIIR.

V8's CallSite API that deno's prepareStackTrace hook uses doesn't expose the relevant information so that's one more reason to expedite #7644. The C API doesn't expose everything either, however, and that's the current blocker.

(That's partially on me - I submitted a CL to V8 last year that I promptly forgot about. I just checked and it looks like I never got notified of the review feedback.)

@ghost
Copy link
Author

ghost commented Jan 21, 2021

@bnoordhuis Yes, I know that it's in the plans to move it over to the Rust, but V8 already provides the stack trace, and formats it, so why does Deno cover that up with another set of formatting?

Also, do you have any suggestions regarding the issue raised?
Should the formatting be changed?
I would've tried to do this myself, but it seems like CallSite doesn't expose whether or not a function is JS or Wasm.

@stale
Copy link

stale bot commented Mar 23, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 23, 2021
@ghost
Copy link
Author

ghost commented Mar 23, 2021

@bnoordhuis Sorry to potentially annoy you, but are you aware of whether the JS or C APIs provide whether the source function was ECMAScript or WebAssembly source code? If so, which one(s)?

@stale stale bot removed the stale label Mar 23, 2021
@bnoordhuis
Copy link
Contributor

@crimsoncodes0 In C there's v8::StackFrame::IsWasm(), called v8::StackFrame::is_wasm() in rusty_v8.

With a reference to a v8::Function, I think you can call function->GetScriptOrigin().Options().IsWasm() but there are no rusty_v8 bindings for those yet.

@ghost
Copy link
Author

ghost commented Mar 23, 2021

@bnoordhuis Thank you for that information, when the stack trace is handled in Rust, I'll come back around to this, that is, if the core team is interested in my suggestion at all.

@stale
Copy link

stale bot commented May 22, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 22, 2021
@stale stale bot closed this as completed May 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bnoordhuis