Skip to content

Conversation

@nagisa
Copy link
Member

@nagisa nagisa commented Oct 31, 2022

When code is being built with debuginfo, we need to instruct the assemblers accordingly.

When code is being built with debuginfo, we need to instruct the
assemblers accordingly.
Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

This matches the documentation on the arm assemblers https://learn.microsoft.com/en-us/cpp/assembler/arm/arm-assembler-command-line-reference?view=msvc-170

The x86 arg seems a bit odd tho https://learn.microsoft.com/en-us/cpp/assembler/masm/ml-and-ml64-command-line-reference?view=msvc-170, but maybe it's just a lack of familiarity with the term "CodeView" for a debuginfo format.

I don't have a good way to test this at the moment, so I might punt to @ChrisDenton in case he's slightly more familiar than I.

@thomcc thomcc requested a review from ChrisDenton October 31, 2022 22:55
@nagisa
Copy link
Member Author

nagisa commented Nov 1, 2022

FWIW my approach to testing this was basically just slapping a cmd.arg("-Zi") in my build.rs and running my stuffs in a debugger.

Two things to keep in mind when interpreting this change for x86: this flag is somewhat shared between the ml assembler and the c/c++ compiler (cl), though it seems to be closer to /Z7: https://learn.microsoft.com/en-us/cpp/build/reference/z7-zi-zi-debug-information-format?view=msvc-170

And / is interchangeable with -. Since cc-rs appears to be using -, I stuck to that convention as well.

@ChrisDenton
Copy link
Member

This does look correct to me but I'll test when I have a moment.

Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

Ok, I tested in WinDbg. With this patch I get an interactive code view when debugging, which is a very nice enhancement!

Btw, it'd be good if CI were able to test debugging info in a similar way to the main rust repo. But setting that up is way beyond the scope of this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants