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

allow overriding frame pointer defaults #441

Merged
merged 1 commit into from
Sep 13, 2019

Conversation

ratmice
Copy link
Contributor

@ratmice ratmice commented Sep 7, 2019

When using a Cargo.toml such containing:

debug = true

cc-rs will automatically add -fno-omit-frame-pointer which affects code generation,
in my opinion there should be a way to enable debug info, without affecting the resulting code generation.

This patch attempts to do so, by only adding -fno-omit-frame-pointer if the target profile is debug
rather than if debuginfo is enabled. I imagine it's somewhat debatable the right way to make this possible.

@alexcrichton
Copy link
Member

I've personally used this before because frame pointers assist in profiling by making stack traces easier for some tools (like perf). It's true that it's unfortunate that it affects runtime performance as well, but I think that we'd probably want an explicit option for this rather than inference based on existing settings.

@ratmice
Copy link
Contributor Author

ratmice commented Sep 9, 2019

I was actually comparing a rust program in perf, to the equivalent system executable. On some systems frame pointers are required, but others can be done with just dwarf and the .eh_frame section.

After sending the request I noticed rustc now has: -C force-frame-pointers=yes|no
Would it be better to rework this based upon the rustc flags/RUSTFLAGS environment variable,
leaving the current behavior unless force-frame-pointers=no? Or is there some value in keeping the cc flags and rust flags distinct?

@alexcrichton
Copy link
Member

Yeah if we could infer from RUSTFLAGS that would be great, although I'm not sure how to best do that myself

@ratmice
Copy link
Contributor Author

ratmice commented Sep 10, 2019

I did have a bit of a look at this, the only thing that would inhibit this from being straight forward argument parsing is the unstable feature Implement argsfile, depending on the compiler version we may have to chase the options around the filesystem.

In many ways, it seems like an ugly hack of using serde in the compiler to serialize some struct containing all of the options, and setting that in an environment variable would be really nice.
But i'm not certain if that is a viable option due to adding depending on serde in rustc.

@alexcrichton
Copy link
Member

Failing auto-inference of an option, having a separate config for this crate seems reasonable. Not a great UI, but gets the job done.

@ratmice ratmice changed the title Only add -fno-omit-frame-pointer if using debug profile. allow overriding frame pointer defaults Sep 12, 2019
@ratmice
Copy link
Contributor Author

ratmice commented Sep 12, 2019

argh, it seems like renaming the issue for some reason canceled all the jobs?,
seems to be something with rustup.

This probably needs some work in specifying the option for other compilers besides gcc/clang.

@ratmice ratmice closed this Sep 12, 2019
@ratmice ratmice reopened this Sep 12, 2019
@alexcrichton
Copy link
Member

Looks great to me, no need to worry about the CI errors, thanks!

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.

2 participants