-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
omitting frame pointers breaks retrieving DWARF debug info for parameter values #11906
Comments
Updating title to include the keyword "DWARF" |
Triage: no change that I'm aware of. This bug is quite old, however, so I might have missed it. |
This makes Rust's generated code look worse than it actually is on http://rust.godbolt.org, because -g is required for output colorization, but it also deactivates frame pointer elimination. Compiler Explorer Issue: |
See also #9641. We still cannot omit frame pointers. Taking test
|
Is there another way to get the line-level information in the asm output, without passing -debug? Compiler Explorer runs Sadly the The equivalent flags for C++ compilation don't have the same effect. |
|
That doesn't appear to be the case. (some lines removed from the below for clarity)
Note how adding the |
Apologies; you did say "less of an impact" - and maybe it does. I was ideally looking for "no code impact" - but I'll change to use this at least! Thanks for the reply; any further suggestions welcomed! |
I really hope we can get rid of these debuginfo only frame pointers. Some platforms cough 64bit windows cough have function tables such that unwinding and debugging can be done just fine without frame pointers so this would definitely help to squeeze out a bit more size and speed. |
This seems largely solved by #44573 and #45380 , because by now we're actually using the incoming arguments and don't shadow them with locals by the same name (the test likely broke because it accessed the local instead of the actual argument). So the unoptimized assembly for the function above now is (I already compiled with debug info + frame pointer omission here):
And the result is as expected:
The only thing that still breaks right now seems to be code that uses pattern matching for function arguments. I'll try to look into that. |
Awesome, thank you @dotdash! |
I disagree with this, but if others agree, we're done here once a scoping bug that @eddyb identified has been resolved. |
"we're done" meaning that we should be fine to omit frame pointers in debug builds. |
The bug in question is likely (cc @arielb1 8c7500f#diff-ef9c98f22bf95e3bda12c849d5550cedR198) rust/src/librustc_mir/build/matches/mod.rs Lines 218 to 221 in 16212b9
Which appears to set the visibility scope way earlier than the intended "after the rust/src/librustc_mir/build/block.rs Lines 134 to 137 in 16212b9
It appears that we do generate nested visibility scopes for binding arguments, just like rust/src/librustc_mir/build/mod.rs Lines 611 to 614 in df8dfde
So once EDIT: #46896 should be fixing this bug. |
cc @pcwalton who might have opinions about whether we should skip frame pointers or not. |
Given that #46896 has been merged now, what's the latest? Was garbage seen in the debugger before, and has that changed? |
Given this program: fn foo(a: i64, (b, c): (i64, i64)) {
}
fn main() {
foo(1, (2, 3));
} If you break on With the current nightly, If you add some code into fn bar() {
}
fn foo(a: i64, (b, c): (i64, i64)) {
bar();
}
fn main() {
foo(1, (2, 3));
} In stable you now get to see I'll build a rustc without the forced framepointers to see whether that still makes any difference at all. I'd still argue that if you break on |
FWIW I would expect debuggers to show |
lldb seems to handle anonymous arguments better than gdb. GDB:
LLDB:
But as with gdb, |
Should we give the anonymous arguments a non-empty name so they show up better? |
Maybe we could simply go with |
We apparently used to generate bad/incomplete debug info causing debuggers not to find symbols of stack allocated variables. This was somehow worked around by having frame pointers. With the current codegen, this seems no longer necessary, so we can remove the code that force-enables frame pointers whenever debug info is requested. Since certain situations, like profiling code profit from having frame pointers, we add a -Cforce-frame-pointers flag to always enable frame pointers. Fixes rust-lang#11906
I filed a gdb bug for this: https://sourceware.org/bugzilla/show_bug.cgi?id=22940 |
We apparently used to generate bad/incomplete debug info causing debuggers not to find symbols of stack allocated variables. This was somehow worked around by having frame pointers. With the current codegen, this seems no longer necessary, so we can remove the code that force-enables frame pointers whenever debug info is requested. Since certain situations, like profiling code profit from having frame pointers, we add a -Cforce-frame-pointers flag to always enable frame pointers. Fixes rust-lang#11906
Add force-frame-pointer flag to allow control of frame pointer ommision Rebase of rust-lang#47152 plus some changes suggested by rust-lang#48785. Fixes rust-lang#11906 r? @nikomatsakis
We apparently used to generate bad/incomplete debug info causing debuggers not to find symbols of stack allocated variables. This was somehow worked around by having frame pointers. With the current codegen, this seems no longer necessary, so we can remove the code that force-enables frame pointers whenever debug info is requested. Since certain situations, like profiling code profit from having frame pointers, we add a -Cforce-frame-pointers flag to always enable frame pointers. Fixes rust-lang#11906
Add force-frame-pointer flag to allow control of frame pointer ommision Rebase of #47152 plus some changes suggested by #48785. Fixes #11906 r? @nikomatsakis
Quoting @alexcrichton: > the fp elim here comes from the code contents of the patch: > > ```rust > // FIXME: rust-lang#11906: Omitting frame pointers breaks retrieving the value of a parameter. > // FIXME: rust-lang#11954: mac64 unwinding may not work with fp elim > let no_fp_elim = (sess.opts.debuginfo != NoDebugInfo) || > (sess.targ_cfg.os == abi::OsMacos && > sess.targ_cfg.arch == abi::X86_64); > ``` > > > which points to rust-lang#11954 which > I believe was [incorrectly closed][] (only references i686, not > x86_64). > > This sounds vaguely familiar about how it's related to > unwinding. This also is the definition of something lost to time > which we unfortunately lost track of :(. > > [incorrectly closed]: rust-lang#11954 (comment)
Turn let-else statements into let and match Fixes rust-lang#11906.
Frame pointers should not be required for debugging, because the DWARF debug information can contain everything necessary. However, it seems we do not generate the expected location list for parameters. This causes the
debuginfo/function-prologue-stepping-no-split-stack.rs
test to break.The text was updated successfully, but these errors were encountered: