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

wasm stack frames are stripped out #1858

Closed
saintmac-google opened this issue May 11, 2023 · 10 comments · Fixed by dart-lang/stack_trace#161
Closed

wasm stack frames are stripped out #1858

saintmac-google opened this issue May 11, 2023 · 10 comments · Fixed by dart-lang/stack_trace#161

Comments

@saintmac-google
Copy link

When building a web application using Flutter and WebAssembly, we will often have stack frames in callstacks that don't have the typical javascript format of somefile.js:123:45, but instead have an hex address

ex:

at util::engine::SomeClass::SomeFunction() (https://some.url.com/some/path/some_app.wasm:wasm-function[66334]:0x12c28ad)

This makes it difficult to capture callstacks for crashes.

Pointers:
Frame.ParseV8 does not match a location generated by wasm:
https://github.com/dart-lang/stack_trace/blob/master/lib/src/frame.dart#L183

source_map_stack_trace filters out frames that are "Unparsed"
https://github.com/dart-lang/source_map_stack_trace/blob/master/lib/source_map_stack_trace.dart#L37

@kevmoo
Copy link
Member

kevmoo commented May 11, 2023

CC @natebosch

@natebosch
Copy link
Member

This might need a fairly invasive fix - if we could parse out something as a line number we might be able to fix this by tweaking the regex, but without a line we might need to make changes in source_map_stack_trace to handle these.

@natebosch
Copy link
Member

@sigmundch - do you know how we should evaluate changes to package:source_map_stack_trace? If we start including frames that have no line information I'm not sure what the impact would be, and I don't know much about the infrastructure that relies on the mapped stack traces. Do you know if package:source_map_stack_trace is involved in our internal crash report tooling?

This behavior has existed since the introduction of package:source_map_stack_trace

@natebosch
Copy link
Member

I don't see any test impact when I start including the frames with missing line information from source_map_stack_trace. I don't know how to predict the impact of that change on the tools which may be sensitive to the difference...

@sigmundch
Copy link
Member

@natebosch - sorry for the delayed reply. We don't directly use source_map_stack_trace or even Frame.parseV8 in our internal crash deobfuscation tools for web clients. So I believe there would be no impact there (I'm less familiar with the native crash reporting tools, not sure if they have a separate dependency there).

For the web - we have a Java and a Dart implementation. Clients internally use the Java implementation, which has its own parsing logic partially shared across other tools. The Dart implementation is there to provide the same functionality externally and for our own testing purposes. There we use similar logic to parse stack traces and deobfuscate them, but we don't actually use the source_map_stack_trace package.

You can find that Dart implementation here https://github.com/dart-lang/sdk/blob/fef81857caed60ee8315355c42f23811a302d682/pkg/dart2js_tools/lib/deobfuscate_stack_trace.dart (note that there is even a very old TODO there indicating that we could use the parse logic in package:stack_trace, but we currently don't).

Do I understand correctly that if we start adding frames with no-line number (but for example a new property to present the binary-offset), we could be backwards compatible to what is happening today when we can't parse these frames altogether?

@natebosch
Copy link
Member

we could be backwards compatible to what is happening today

We can be statically backwards compatible by including these frames opaquely, but the behavior change could break anything that reads the frames and (intentionally or unintentionally) expects to not see these frames.

@saintmac-google

This makes it difficult to capture callstacks for crashes.

Is this an internal system capturing callstack? If we make a change in package:source_map_stack_trace would you be able to confirm the change is helpful?

@saintmac-google
Copy link
Author

we could be backwards compatible to what is happening today

We can be statically backwards compatible by including these frames opaquely, but the behavior change could break anything that reads the frames and (intentionally or unintentionally) expects to not see these frames.

@saintmac-google

This makes it difficult to capture callstacks for crashes.

Is this an internal system capturing callstack? If we make a change in package:source_map_stack_trace would you be able to confirm the change is helpful?

Yes
(sorry, I thought I responded earlier)

@art926
Copy link

art926 commented Aug 3, 2023

Hi!
Was there any work done on it yet?

@natebosch
Copy link
Member

b/286929124 has some internal discussion from June. I think the latest update was a request to @yjbanov about making the flutter frame parsing looser. I'm not sure if this is the same frame parsing that was just adjusted in flutter/flutter#131786

After that I think we found that the issue was caused by a DDC copy of this code which may need an update. @nshahan @sigmundch - have we tried making the same patch in the DDC copy?

I would be happy to land a change in package:source_map_stack_trace, but it wouldn't make a difference in the cases we've seen.

osa1 referenced this issue in osa1/dart_stack_trace Sep 19, 2024
Handle URIs other than `wasm://`.

Fixes #131.
@osa1
Copy link
Member

osa1 commented Sep 19, 2024

I fixed this in dart-lang/stack_trace#161.

This might need a fairly invasive fix - if we could parse out something as a line number we might be able to fix this by tweaking the regex, but without a line we might need to make changes in source_map_stack_trace to handle these.

When mapping Wasm byte offsets to sources the convention is that the .wasm file is considered as a single line line where bytes are columns.

I've updated this package to parse Wasm frames, in dart-lang/stack_trace#159. With this change source_map_stack_trace package can map Wasm locations to sources without any changes.

osa1 referenced this issue in dart-lang/stack_trace Sep 19, 2024
Handle URIs other than `wasm://`.

Fixes #131.
mosuem referenced this issue Dec 10, 2024
Handle URIs other than `wasm://`.

Fixes dart-lang/stack_trace#131.
@mosuem mosuem transferred this issue from dart-lang/stack_trace Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants