Re: [patch 2/2] Disable epilogue unwinders on recent GCCs [Re: Regression: Re: [PATCH] Fix some i386 unwinder inconcistencies]

Mark Kettenis
2011-06-27T09:39:12Z

> Date: Sun, 26 Jun 2011 10:41:41 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
>
> On Mon, 13 Jun 2011 12:49:11 +0200, Jan Kratochvil wrote:
> > On all the tested platforms Fedora-{13,14,15,Rawhide} for {i686,x86_64-m32}
> > (but not for x86_64):
> > -PASS: gdb.base/watchpoint-cond-gone.exp: Catch the no longer valid watchpoint
> > +FAIL: gdb.base/watchpoint-cond-gone.exp: Catch the no longer valid watchpoint
> > -XFAIL: gdb.mi/mi-watch.exp: sw: watchpoint trigger (stopped at wrong place)
> > +XFAIL: gdb.mi/mi-watch.exp: sw: watchpoint trigger (unknown output after running)
> > -XFAIL: gdb.mi/mi2-watch.exp: sw: watchpoint trigger (stopped at wrong place)
> > +XFAIL: gdb.mi/mi2-watch.exp: sw: watchpoint trigger (unknown output after running)
>
> On recent GCCs:
> http://gcc.gnu.org/gcc-4.5/changes.html
> GCC now generates unwind info also for epilogues.
>
> The epilogue unwinders are therefore no longer useful in common cases.

Good news! The epilogue unwinders will still be useful for debugging
code without debug information.

> Moreover as they precede the DWARF unwinder they prevent
> archer-jankratochvil-entryval (to be submitted these days) functionality at
> those return instructions as entryval can provide more constructed parameters
> debug info even at those PCs. It is a bit offtopic now but it causes:
>
>
> (gdb) bt
> #0 d (i=70) at tailcall.c:6
> ^^^^ value provided by archer-jankratochvil-entryval
> #1 0x00000000004004ba in c (i=7) at tailcall.c:8
> ^^^^^^^^^^^^^^^^^^^^^^^ frame provided by archer-jankratochvil-entryval
> #2 0x00000000004004d8 in b (i=5) at tailcall.c:12
> ^^^^^^^^^^^^^^^^^^^^^^^ frame provided by archer-jankratochvil-entryval
> #3 0x0000000000400384 in main () at tailcall.c:15
> (gdb) disass
> Dump of assembler code for function d:
> 0x0000000000400490 <+0>: callq 0x400480 <e>
> 0x0000000000400495 <+5>: mov 0x20046d(%rip),%edi # 0x600908 <v>
> 0x000000000040049b <+11>: callq 0x400480 <e>
> => 0x00000000004004a0 <+16>: xor %eax,%eax
> 0x00000000004004a2 <+18>: retq
> End of assembler dump.
> (gdb) bt
> #0 d (i=70) at tailcall.c:6
> #1 0x00000000004004ba in c (i=7) at tailcall.c:8
> #2 0x00000000004004d8 in b (i=5) at tailcall.c:12
> #3 0x0000000000400384 in main () at tailcall.c:15

Interesting. Don't see how you can reliably do that, but I'm looking
forward to seeing your patch. Tail-call optimizations often confuse
GDB users.

> I find questionable this detection uses DWARF DW_AT_producer.
> Therefore on binaries with .eh_frame but no (DWARF) debug info the
> detection fails and epilogue unwinders will get into affect.

I think it is an acceptable compromise. People should expect some
lost functionality when they strip their binaries.

> 1441 /* This restriction could be lifted if other unwinders are known to
> 1442 compute the frame base in a way compatible with the DWARF
> 1443 unwinder. */
> 1444 if (! frame_unwinder_is (this_frame, &dwarf2_frame_unwind))
> 1445 error (_("can't compute CFA for this frame"));

I still think, this code should be removed. Tom, since you added that
bit, what's your take on that?

> No regressions on {x86_64,x86_64-m32,i686}-fedora15-linux-gnu, it fixes the
> regression reported above. I will check it in some time if no comments
> appear.

Implementation makes sense to me. So go ahead once 1/2 is approved
(not really in my area).

> gdb/
> 2011-06-26 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> Disable epilogue unwinders on recent GCCs.
> * amd64-tdep.c (amd64_in_function_epilogue_p): New variable symtab,
> initialize it, return 0 on EPILOGUE_UNWIND_VALID.
> * dwarf2read.c (process_full_comp_unit): Initialize
> EPILOGUE_UNWIND_VALID.
> * i386-tdep.c (i386_in_function_epilogue_p): New variable symtab,
> initialize it, return 0 on EPILOGUE_UNWIND_VALID.
> * symtab.h (struct symtab): New field epilogue_unwind_valid.
>
> --- a/gdb/amd64-tdep.c
> +++ b/gdb/amd64-tdep.c
> @@ -2219,6 +2219,11 @@ static int
> amd64_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> {
> gdb_byte insn;
> + struct symtab *symtab;
> +
> + symtab = find_pc_symtab (pc);
> + if (symtab && symtab->epilogue_unwind_valid)
> + return 0;
>
> if (target_read_memory (pc, &insn, 1))
> return 0; /* Can't read memory at pc. */
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -4751,6 +4751,9 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu)
> */
> if (cu->has_loclist && gcc_4_minor >= 0)
> symtab->locations_valid = 1;
> +
> + if (gcc_4_minor >= 5)
> + symtab->epilogue_unwind_valid = 1;
> }
>
> if (dwarf2_per_objfile->using_index)
> --- a/gdb/i386-tdep.c
> +++ b/gdb/i386-tdep.c
> @@ -1885,6 +1885,11 @@ static int
> i386_in_function_epilogue_p (struct gdbarch *gdbarch, CORE_ADDR pc)
> {
> gdb_byte insn;
> + struct symtab *symtab;
> +
> + symtab = find_pc_symtab (pc);
> + if (symtab && symtab->epilogue_unwind_valid)
> + return 0;
>
> if (target_read_memory (pc, &insn, 1))
> return 0; /* Can't read memory at pc. */
> --- a/gdb/symtab.h
> +++ b/gdb/symtab.h
> @@ -779,6 +779,11 @@ struct symtab
>
> unsigned int locations_valid : 1;
>
> + /* DWARF unwinder for this CU is valid even for epilogues (PC at the return
> + instruction). This is supported by GCC since 4.5.0. */
> +
> + unsigned int epilogue_unwind_valid : 1;
> +
> /* The macro table for this symtab. Like the blockvector, this
> may be shared between different symtabs --- and normally is for
> all the symtabs in a given compilation unit. */
>