Skip to content

feat(wasm): Unconditionally parse instruction addresses#13655

Merged
lforst merged 1 commit intodevelopfrom
lforst-wasm-stackframe-parsing-without-images
Sep 13, 2024
Merged

feat(wasm): Unconditionally parse instruction addresses#13655
lforst merged 1 commit intodevelopfrom
lforst-wasm-stackframe-parsing-without-images

Conversation

@lforst
Copy link
Copy Markdown
Contributor

@lforst lforst commented Sep 10, 2024

Currently we are only setting the platform and instruction_addr for a wasm frame if we find a matching debug image. This is not always the case and unconditionally parsing the instruction address is almost always desirable.

@lforst lforst requested a review from andreiborza September 10, 2024 15:31
@lforst
Copy link
Copy Markdown
Contributor Author

lforst commented Sep 10, 2024

That's the one wasm PR for the year. We can pack it up.

AbhiPrasad
AbhiPrasad reviewed Sep 10, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

What motivated this change?

@lforst
Copy link
Copy Markdown
Contributor Author

lforst commented Sep 10, 2024

What motivated this change?

@AbhiPrasad A user wrote in via sales that the stack frames for an unsymbolicated wasm are all collapsed in the event. This is because for unsymbolicated wasm we parse out the module, but don't include the address:

  [{
    "module": "WASM/redacted/redacted",
    "filename": "/WASM/redacted/redacted.wasm:wasm-function[127458]:0x3695e9b",
    "abs_path": "https://redacted.com:3000/WASM/redacted/redacted.wasm:wasm-function[127458]:0x3695e9b",
    "in_app": true
  },
  {
    "module": "WASM/redacted/redacted",
    "filename": "/WASM/redacted/redacted.wasm:wasm-function[2275]:0x2b880f",
    "abs_path": "https://redacted.com:3000/WASM/redacted/redacted.wasm:wasm-function[2275]:0x2b880f",
    "in_app": true
  },
  {
    "module": "WASM/redacted/redacted",
    "filename": "/WASM/redacted/redacted.wasm:wasm-function[79547]:0x21df74d",
    "abs_path": "https://redacted.com:3000/WASM/redacted/redacted.wasm:wasm-function[79547]:0x21df74d",
    "in_app": true
  }]

The product will show the stack frames all collapsed because it takes only the module (which in normal circumstances is a some .js and also has line & col). We are basically missing the address as a discriminating factor.

@lforst lforst requested a review from AbhiPrasad September 13, 2024 08:24
@lforst lforst merged commit cbb9475 into develop Sep 13, 2024
@lforst lforst deleted the lforst-wasm-stackframe-parsing-without-images branch September 13, 2024 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreiborza andreiborza Awaiting requested review from andreiborza

1 more reviewer

@AbhiPrasad AbhiPrasad AbhiPrasad approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lforst @AbhiPrasad