Skip to content

feat: support dSYM symbolication in native_stack_traces #48767

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
Closed
vaind wants to merge 1 commit into from
Closed

feat: support dSYM symbolication in native_stack_traces #48767

vaind wants to merge 1 commit into from

Conversation

@vaind
Copy link

@vaind vaind commented Apr 8, 2022 •
edited
Loading

This is part of #43612 - enabling Flutter users to split-debug-info into dSYM and symbolize it back, using flutter symbolize or directly using the CLI tool in native_stack_traces.

The general approach taken was proposed by @mraleph in this comment: #43612 (comment)

There are quite a few open questions here as well as in the Flutter tooling, but I'd like to get the feedback on this. There are two main pain points in this PR

  1. the current implementation in dwarf.dart only supports ELF format - that doesn't match the dwarf file contained in a dSYM container and it is beyond me to implement the dwarf parsing so the proposed solution uses a CLI tool atos get symbol names for individual addresses. This, however, loses the option to load the dwarf from a Uint8List, which is currently done by the flutter tool to allow unit testing.
  2. So far I couldn't figure out a way to extract the following addresses from dSYM and they're required to properly call atos... any ideas? Currently they're parsed from the given stack trace (the second line already was before this PR). 
    isolate_dso_base: 116904000, vm_dso_base: 116904000
isolate_instructions: 11690f6d0, vm_instructions: 11690aee0

Additionally, I haven't seen any tests for the existing CLI tool. Hopefully, the CI will give me some feedback here.

@vaind vaind mentioned this pull request Apr 8, 2022
Merged
13 tasks
@mraleph
Copy link
Member

mraleph commented Apr 13, 2022

(just wanted to leave a note that I am on Easter break - I will be back next Tuesday)

@mraleph
Copy link
Member

mraleph commented Apr 20, 2022

I started to take an initial look at this.

  1. the current implementation in dwarf.dart only supports ELF format - that doesn't match the dwarf file contained in a dSYM container and it is beyond me to implement the dwarf parsing so the proposed solution uses a CLI tool atos get symbol names for individual addresses.

We are in luck here. We have actually recently added a Mach-O parser which might be good enough for our purposes here. It has somewhat different API from Elf/Dwarf but I think we can contort things enough to make them work.

I am trying to prototype something up.

  1. So far I couldn't figure out a way to extract the following addresses from dSYM and they're required to properly call atos... any ideas? Currently they're parsed from the given stack trace (the second line already was before this PR).

You can't extract these values from dSYM because they are runtime values. (The same Mach-O file can be loaded into different places in memory).

Though I think this question is moot if we start reading DWARF natively from dSYM instead of relying on atos.

@vaind
Copy link
Author

vaind commented Apr 20, 2022

We are in luck here. We have actually recently added a Mach-O parser which might be good enough for our purposes here. It has somewhat different API from Elf/Dwarf but I think we can contort things enough to make them work.

Cool, I'll have a look.

You can't extract these values from dSYM because they are runtime values. (The same Mach-O file can be loaded into different places in memory).

Yeah, that was confusing to me because the original dwarf symbolization somehow reads them, e.g.

sdk/pkg/native_stack_traces/lib/src/dwarf.dart

Line 1265 in 84497e4

final vmStartAddress = vmStartSymbol.value;

@marandaneto marandaneto mentioned this pull request Apr 22, 2022
Closed
@fzyzcjy
Copy link
Contributor

fzyzcjy commented Apr 22, 2022

Looking forward to the PR!

@mraleph
Copy link
Member

mraleph commented Apr 22, 2022

I hacked some stuff together to verify that we can actually read DWARF from Mach-O. See this CL: https://dart-review.googlesource.com/c/sdk/+/242108

Seems to work like a charm in my limited testing.

This also uncovered some bugs in our implementation of DWARF reading.

I am planning to take some time next week to clean this CL up.

@mkustermann
Copy link
Member

mkustermann commented Apr 27, 2022

@sstrickl has kindly agreed to take this work over from @mraleph .

@vaind
Copy link
Author

vaind commented Apr 29, 2022 •
edited
Loading

@sstrickl has kindly agreed to take this work over from @mraleph .

awesome. Please let me know if there's anything I can do to help land those changes.

@sstrickl
Copy link
Contributor

sstrickl commented May 18, 2022

Hi @vaind, sorry about the delay, but if you could take a look at https://dart-review.googlesource.com/c/sdk/+/240645 and address the comments there (it's just about the TODOs), I think otherwise we can go with this initial cut as is and then I'll work on addressing any remaining open questions, for example, the TODOs you've written plus those issues noticed by Slava during his work on his CL.

@mraleph
Copy link
Member

mraleph commented May 18, 2022

@sstrickl I am not sure we need @vaind's CL if we cleanup and land some form of dart-review.googlesource.com/c/sdk/+/242108? No need to shell out to atos, etc.

@sstrickl
Copy link
Contributor

sstrickl commented May 18, 2022

Ah. Gotcha, I'll take a look at working on your CL specifically instead then. Sorry for the misunderstanding.

@vaind
Copy link
Author

vaind commented May 18, 2022

Ah. Gotcha, I'll take a look at working on your CL specifically instead then. Sorry for the misunderstanding.

Great, let me know if there's anything I should do @sstrickl

@scott-the-brewer
Copy link

scott-the-brewer commented Jun 7, 2022

@sstrickl is there a CL I can follow for this feature? Is it https://dart-review.googlesource.com/c/sdk/+/242108? Just noticed there hasn't been any update on it for weeks.

@lrhn lrhn added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Jun 14, 2022
@vaind
Copy link
Author

vaind commented Jul 5, 2022

replaced by https://dart-review.googlesource.com/c/sdk/+/250381

@vaind vaind closed this Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@vaind @mraleph @fzyzcjy @mkustermann @sstrickl @scott-the-brewer @lrhn