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

Add debuginfo flags for msvc assemblers #742

Merged
merged 1 commit into from Nov 4, 2022

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.

@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
Contributor

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

Copy link
Contributor

@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.

@ChrisDenton ChrisDenton merged commit 66005c8 into rust-lang:main Nov 4, 2022
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.

None yet

3 participants