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

Change memory and stack dumping to be more readable #527

Merged
merged 3 commits into from Apr 14, 2022
Merged

Change memory and stack dumping to be more readable #527

merged 3 commits into from Apr 14, 2022

Conversation

marti4d
Copy link
Contributor

@marti4d marti4d commented Apr 4, 2022

Fixes #375

@Gankra
Copy link
Collaborator

Gankra commented Apr 5, 2022

cc @jrmuizel, what do you think of this format?

@Gankra
Copy link
Collaborator

Gankra commented Apr 5, 2022

possible notes:

  • do we want to use more horizontal space here? 3 or 4 pointers per line..?
  • what does this mean for endianess? do we show our endianess? the crashing platform's endianess..? is purely grouping the bytes as seen without messing with endianess?

@jrmuizel
Copy link
Contributor

jrmuizel commented Apr 5, 2022

This format is fine. I think one pointer per line is easier to scan than multiple pointers per line.

The idea here is to be able to easily read return addresses so it should use the crashing platform's endianess and pointer size

@marti4d
Use enum for pointer width instead of integer
91bb757 
Allowing for an arbitrary number of bits in a word means we may have to
write code that is general enough to handle any number of bits, which can
get pretty complicated, and is generally not needed.

Case-in-point: Even though this function has only been used twice, we can
already see it being matched against "Some(4)" so it can be casted to a u32.
@marti4d marti4d force-pushed the main branch 3 times, most recently from 2b9d3f0 to 331af94 Compare April 6, 2022 20:52
@marti4d
Copy link
Contributor Author

marti4d commented Apr 6, 2022

@Gankra @jrmuizel: Alrighty - The stack should now be dumping with target-system-sized words. I tested it with a 32-bit dump, and it seems to do everything correctly.

@marti4d marti4d changed the title Support dumping the stack as 64bit words Change memory and stack dumping to be more readable Apr 6, 2022
@marti4d
Copy link
Contributor Author

marti4d commented Apr 6, 2022 •
edited

Fixes #528

@marti4d marti4d force-pushed the main branch 2 times, most recently from f4cc6da to 5302253 Compare April 7, 2022 19:01
Gankra
Gankra approved these changes Apr 14, 2022
View reviewed changes
Copy link
Collaborator

@Gankra Gankra left a comment

Choose a reason for hiding this comment

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

hoooooly heck this is incredible work.

I genuinely have no comments, this is just GOOD.

@Gankra Gankra merged commit 4c6fb12 into rust-minidump:main Apr 14, 2022
@Gankra Gankra mentioned this pull request Jul 28, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Gankra Gankra Gankra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

minidump_dump: Support dumping the stack as 32/64bit words
3 participants
@marti4d @Gankra @jrmuizel