Re: Regression: Re: [PATCH] Fix some i386 unwinder inconcistencies

[Mark Kettenis <mark.kettenis@xs4all.nl>]

> Date: Mon, 13 Jun 2011 12:49:11 +0200
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Sun, 12 Jun 2011 22:57:30 +0200, Mark Kettenis wrote:
> > This diff fixes a few issues with the epilogue and stack tramp unwinders.
> > 
> > Committed.
> > 
> > 2011-06-12  Mark Kettenis  <kettenis@gnu.org>
> >  
> > 	* i386-tdep.c (i386_epilogue_frame_cache): Simplify code.  Call
> > 	get_frame_func instead of get_frame_pc to determine the code
> > 	address used to construct the frame ID.
> > 	(i386_epilogue_frame_unwind_stop_reason): Fix coding style.
> > 	(i386_epilogue_frame_this_id): Likewise.
> > 	(i386_epilogue_frame_prev_register): New function.
> > 	(i386_epilogue_frame_unwind): Use i386_epilogue_frame_prev_register.
> > 	(i386_stack_tramp_frame_sniffer): Fix coding style.
> > 	(i386_stack_tramp_frame_unwind): Use i386_epilogue_frame_prev_register.
> > 	(i386_gdbarch_init): Fix comments.
> 
> 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

Odd, that tests still passes for me on i386-unknown-openbsd4.9.

PASS: gdb.base/watchpoint-cond-gone.exp: Catch the no longer valid watchpoint

Something did change though.  Before my change:

  Run till exit from #0  func () at ../../../src/gdb/testsuite/gdb.base/watchpoint-cond-gone.c:26

  Watchpoint 3 deleted because the program has left the block in
  which its expression is valid.
  0x1c0006d0 in func () at ../../../src/gdb/testsuite/gdb.base/watchpoint-cond-gone.c:28
  28      }

So the watchpoint went out of scope before the function returned.  Whereas after my change:

  Run till exit from #0  func () at ../../../src/gdb/testsuite/gdb.base/watchpoint-cond-gone.c:26

  Watchpoint 3 deleted because the program has left the block in
  which its expression is valid.
  0x1c000707 in jumper ()

the watchpoint went out of scope when the function returned, as I believe it should.

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

Ok, I'm seeing these as well.  Didn't classify these as a regression
since they went from XFAIL to XFAIL.  They seem to be related to the
fact that I changed get_frame_pc into get_frame_func.  That change is
correct though.  The frame ID returned by the epilogue unwinder should
be identical to the one returned by the "normal", so the code address
needs to be the start of the function and not the address of the 'ret'
instruction.

> The regressions are all just instances of:
> 
>  finish
>  Run till exit from #0  func () at ./gdb.base/watchpoint-cond-gone.c:26
> -
> -Watchpoint 3 deleted because the program has left the block in
> -which its expression is valid.
> +Error evaluating expression for watchpoint 3
> +can't compute CFA for this frame
> +Watchpoint 3 deleted.

I think this can be avoided by implementing the
in_function_epilogue_p() gdbarch method for i386/amd64.  In fact, that
method already seems to be implemented.  It just isn't registered.