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

omitting frame pointers breaks retrieving DWARF debug info for parameter values #11906

Closed
thestinger opened this issue Jan 29, 2014 · 23 comments
Closed
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@thestinger
Copy link
Contributor

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.

@nikomatsakis
Copy link
Contributor

Updating title to include the keyword "DWARF"

@huonw
Copy link
Member

huonw commented Aug 11, 2014

cc @michaelwoerister

@steveklabnik
Copy link
Member

Triage: no change that I'm aware of. This bug is quite old, however, so I might have missed it.

@CryZe
Copy link
Contributor

CryZe commented Oct 16, 2016

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.

ASM Output

Compiler Explorer Issue:
compiler-explorer/compiler-explorer#79

@nagisa
Copy link
Member

nagisa commented Nov 3, 2016

See also #9641.

We still cannot omit frame pointers. Taking test src/test/debuginfo/function-arg-initialization.rs, the disassembly looks like this:

   0x00005555555599c0 <+0>: push   %rbp
   0x00005555555599c1 <+1>: mov    %rsp,%rbp
   0x00005555555599c4 <+4>: sub    $0x140,%rsp                 # breakpoint with `rbreak non_immediate` with frame pointer elimination enabled
   0x00005555555599cb <+11>:    mov    $0x40,%eax
   0x00005555555599d0 <+16>:    mov    %eax,%ecx
   0x00005555555599d2 <+18>:    lea    -0x80(%rbp),%rdx
   0x00005555555599d6 <+22>:    lea    -0x40(%rbp),%r8
   0x00005555555599da <+26>:    mov    %rdi,-0x110(%rbp)
   0x00005555555599e1 <+33>:    mov    %r8,%rdi
   0x00005555555599e4 <+36>:    mov    -0x110(%rbp),%r8
   0x00005555555599eb <+43>:    mov    %rsi,-0x118(%rbp)
   0x00005555555599f2 <+50>:    mov    %r8,%rsi
   0x00005555555599f5 <+53>:    mov    %rdx,-0x120(%rbp)
   0x00005555555599fc <+60>:    mov    %rcx,%rdx
   0x00005555555599ff <+63>:    mov    %rcx,-0x128(%rbp)
   0x0000555555559a06 <+70>:    callq  0x5555555595c0 <memcpy@plt> # argument a gets initialized
   0x0000555555559a0b <+75>:    mov    -0x118(%rbp),%rcx
   0x0000555555559a12 <+82>:    mov    -0x120(%rbp),%rdx
   0x0000555555559a19 <+89>:    mov    %rdx,%rdi
   0x0000555555559a1c <+92>:    mov    %rcx,%rsi
   0x0000555555559a1f <+95>:    mov    -0x128(%rbp),%rdx
   0x0000555555559a26 <+102>:   callq  0x5555555595c0 <memcpy@plt> # argument b gets initialized
   0x0000555555559a2b <+107>:   mov    $0x40,%eax
   0x0000555555559a30 <+112>:   mov    %eax,%ecx
   0x0000555555559a32 <+114>:   lea    -0x108(%rbp),%rdx
   0x0000555555559a39 <+121>:   lea    -0x80(%rbp),%rsi
   0x0000555555559a3d <+125>:   lea    -0xc8(%rbp),%rdi
   0x0000555555559a44 <+132>:   lea    -0x40(%rbp),%r8
   0x0000555555559a48 <+136>:   mov    %rsi,-0x130(%rbp)           # breakpoint with `rbreak non_immediate` with frame pointer elimination disabled
   0x0000555555559a4f <+143>:   mov    %r8,%rsi
   0x0000555555559a52 <+146>:   mov    %rdx,-0x138(%rbp)
   0x0000555555559a59 <+153>:   mov    %rcx,%rdx
   0x0000555555559a5c <+156>:   mov    %rcx,-0x140(%rbp)
   0x0000555555559a63 <+163>:   callq  0x5555555595c0 <memcpy@plt>
   0x0000555555559a68 <+168>:   mov    -0x130(%rbp),%rcx
   0x0000555555559a6f <+175>:   mov    -0x138(%rbp),%rdx
   0x0000555555559a76 <+182>:   mov    %rdx,%rdi
   0x0000555555559a79 <+185>:   mov    %rcx,%rsi
   0x0000555555559a7c <+188>:   mov    -0x140(%rbp),%rdx
   0x0000555555559a83 <+195>:   callq  0x5555555595c0 <memcpy@plt>
   0x0000555555559a88 <+200>:   callq  0x55555555a020 <function_arg_initialization::zzz>
   0x0000555555559a8d <+205>:   add    $0x140,%rsp
   0x0000555555559a94 <+212>:   pop    %rbp
   0x0000555555559a95 <+213>:   retq   

@mattgodbolt
Copy link

Is there another way to get the line-level information in the asm output, without passing -debug? Compiler Explorer runs rustc -g --emit asm to get the assembly, and then parses the .loc directive to marry input code with output code.

Sadly the -g changes the code generation, which makes the output less useful.

The equivalent flags for C++ compilation don't have the same effect.

@michaelwoerister
Copy link
Member

-g is equivalent to -C debuginfo=2, meaning "full debuginfo". But there is also -C debuginfo=1 which means "just line-tables" and should have less of an impact on code generation.

@mattgodbolt
Copy link

That doesn't appear to be the case.

(some lines removed from the below for clarity)

/t/r $ cat test.rs
pub fn square(num: i32) -> i32 {
  num * num
}
/t/r $ rustc --version
rustc 1.14.0 (e8a012324 2016-12-16)
/t/r $ rustc -O -C debuginfo=0 --emit asm test.rs --crate-type rlib
/t/r $ cat test.s
...
	.cfi_startproc
	imull	%edi, %edi
	movl	%edi, %eax
	retq
.Lfunc_end0:
	.size	_ZN4test6square17h3872c4fac15ac3c4E, .Lfunc_end0-_ZN4test6square17h3872c4fac15ac3c4E
	.cfi_endproc
...

/t/r $ rustc -O -C debuginfo=1 --emit asm test.rs --crate-type rlib
...
	.cfi_startproc
	pushq	%rbp
.Ltmp0:
	.cfi_def_cfa_offset 16
.Ltmp1:
	.cfi_offset %rbp, -16
	movq	%rsp, %rbp
.Ltmp2:
	.cfi_def_cfa_register %rbp
.Ltmp3:
	.loc	1 2 0 prologue_end
	imull	%edi, %edi
	.loc	1 3 0
	movl	%edi, %eax
	popq	%rbp
	retq
.Ltmp4:
.Lfunc_end0:
	.size	_ZN4test6square17h3872c4fac15ac3c4E, .Lfunc_end0-_ZN4test6square17h3872c4fac15ac3c4E
	.cfi_endproc
...

Note how adding the debuginfo=1 has caused code generation differences.

@mattgodbolt
Copy link

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!

@retep998
Copy link
Member

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.

@Mark-Simulacrum Mark-Simulacrum added the C-enhancement Category: An issue proposing an enhancement or a PR with one. label Jul 20, 2017
@dotdash
Copy link
Contributor

dotdash commented Dec 19, 2017

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):

Dump of assembler code for function function_arg_initialization::non_immediate_args:
=> 0x0000555555559ff0 <+0>:	sub    $0x18,%rsp
   0x0000555555559ff4 <+4>:	mov    %rdi,0x10(%rsp)
   0x0000555555559ff9 <+9>:	mov    %rsi,0x8(%rsp)
   0x0000555555559ffe <+14>:	callq  0x55555555a4e0 <function_arg_initialization::zzz>
   0x000055555555a003 <+19>:	add    $0x18,%rsp
   0x000055555555a007 <+23>:	retq   

And the result is as expected:

(gdb) info args
a = function_arg_initialization::BigStruct {a: 3, b: 4, c: 5, d: 6, e: 7, f: 8, g: 9, h: 10}
b = function_arg_initialization::BigStruct {a: 11, b: 12, c: 13, d: 14, e: 15, f: 16, g: 17, h: 18}
(gdb) info locals
No locals.
(gdb) print a
$1 = function_arg_initialization::BigStruct {a: 3, b: 4, c: 5, d: 6, e: 7, f: 8, g: 9, h: 10}
(gdb) print b
$2 = function_arg_initialization::BigStruct {a: 11, b: 12, c: 13, d: 14, e: 15, f: 16, g: 17, h: 18}

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.

@michaelwoerister
Copy link
Member

Awesome, thank you @dotdash!

@dotdash
Copy link
Contributor

dotdash commented Dec 20, 2017

<doener> eddyb: so you're saying that when I set a breakpoint 
         on a function foo((a, b): (i64, 64)), it would be fine to not 
         have anything in scope at the breakpoint?
<eddyb> doener: yeah sure

I disagree with this, but if others agree, we're done here once a scoping bug that @eddyb identified has been resolved.

@dotdash
Copy link
Contributor

dotdash commented Dec 20, 2017

"we're done" meaning that we should be fine to omit frame pointers in debug builds.

@eddyb
Copy link
Member

eddyb commented Dec 20, 2017

The bug in question is likely (cc @arielb1 8c7500f#diff-ef9c98f22bf95e3bda12c849d5550cedR198)

// Pop any scope we created for the locals.
if let Some(var_scope) = var_scope {
self.visibility_scope = var_scope;
}

Which appears to set the visibility scope way earlier than the intended "after the let":

// Enter the visibility scope, after evaluating the initializer.
if let Some(visibility_scope) = scope {
this.visibility_scope = visibility_scope;
}

It appears that we do generate nested visibility scopes for binding arguments, just like let:

// Enter the argument pattern bindings visibility scope, if it exists.
if let Some(visibility_scope) = scope {
self.visibility_scope = visibility_scope;
}

So once declare_bindings is fixed, arguments should be at least hidden during their destructuring.

EDIT: #46896 should be fixing this bug.

@eddyb
Copy link
Member

eddyb commented Dec 20, 2017

cc @pcwalton who might have opinions about whether we should skip frame pointers or not.

@eddyb
Copy link
Member

eddyb commented Dec 31, 2017

Given that #46896 has been merged now, what's the latest? Was garbage seen in the debugger before, and has that changed?

@dotdash
Copy link
Contributor

dotdash commented Jan 2, 2018

Given this program:

fn foo(a: i64, (b, c): (i64, i64)) {
}

fn main() {
    foo(1, (2, 3));
}

If you break on foo, in stable you'd see a as an argument with the correct data and a, b and c as uninitialized locals. Stepping over machine instructions (using ni) you see a and b getting initialized, but c falls out of scope at the same time as it gets initialized, so you never get to see it with its correct value.

With the current nightly, a only shows up as an initialized argument (no additional local), and b and c never get into scope at all.

If you add some code into foo like this:

fn bar() {
}

fn foo(a: i64, (b, c): (i64, i64)) {
    bar();
}

fn main() {
    foo(1, (2, 3));
}

In stable you now get to see c getting initialized as well. And in nightly, you still start out with b and c not being in scope at all, but they get into scope as initialized locals once you arrive at the call to bar().

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 foo, you should get a break point that puts you at the call to bar() in the last example, not somewhere in the prologue where the function "arguments" get initialized. I'm putting the quotes there, because I fell that those variables should be marked as arguments rather than locals, but both of these points are kind of unrelated to the issue at hand.

@eddyb
Copy link
Member

eddyb commented Jan 2, 2018

FWIW I would expect debuggers to show foo(1, (2, 3)) in the backtrace, i.e. two arguments, the second of which is unnamed - is that not the case? Also, is lldb different?

@dotdash
Copy link
Contributor

dotdash commented Jan 2, 2018

lldb seems to handle anonymous arguments better than gdb.

GDB:

(gdb) bt
#0  match::foo::h96fa2745b04f623a (a=1) at match.rs:4
#1  0x0000555555559e2c in match::main::h27865b1a408d32d7 () at match.rs:9
#2  0x0000555555559ef0 in std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::h82de5b69f22f8abb () at /home/bs/src/rust/src/libstd/rt.rs:74

LLDB:

* thread #1, name = 'match', stop reason = breakpoint 1.1
  * frame #0: match`match::foo::h96fa2745b04f623a(a=1, (null)=(__0 = 2, __1 = 3)) at match.rs:4
    frame #1: match`match::main::h27865b1a408d32d7 at match.rs:9

(lldb) frame v
(long) a = 1
((i64, i64))  = (__0 = 2, __1 = 3)

But as with gdb, b and c only show up once you stepped to the call to bar().

@eddyb
Copy link
Member

eddyb commented Jan 2, 2018

Should we give the anonymous arguments a non-empty name so they show up better?

@dotdash
Copy link
Contributor

dotdash commented Jan 2, 2018

Maybe we could simply go with _ to avoid name clashes? Do we need to mark them as artificial as well?

dotdash added a commit to dotdash/rust that referenced this issue Feb 8, 2018
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
@tromey
Copy link
Contributor

tromey commented Mar 8, 2018

lldb seems to handle anonymous arguments better than gdb.

I filed a gdb bug for this: https://sourceware.org/bugzilla/show_bug.cgi?id=22940

nagisa pushed a commit to nagisa/rust that referenced this issue Mar 15, 2018
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
kennytm added a commit to kennytm/rust that referenced this issue Mar 21, 2018
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
nagisa pushed a commit to nagisa/rust that referenced this issue May 1, 2018
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
bors added a commit that referenced this issue May 1, 2018
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
shepmaster added a commit to shepmaster/rust that referenced this issue Jul 21, 2020
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)
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 25, 2022
Turn let-else statements into let and match

Fixes rust-lang#11906.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-debuginfo Area: Debugging information in compiled programs (DWARF, PDB, etc.) C-enhancement Category: An issue proposing an enhancement or a PR with one. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests