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

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Mon, 13 Jun 2011 21:10:28 +0200, Mark Kettenis wrote:
> > In both cases on Fedora it stops at the same place:
> > 0x080483e0 in func () at ./gdb.base/watchpoint-cond-gone.c:28^M
> 
> Are you sure?  If that's the case, there must be debug info that tells
> GDB that the watchpoint goes out of scope.

It stops because the original frame changes and in watchpoint_check
within_current_scope becomes 0.  This is because code_addr is different but
your patch fixed code_addr so it no longer gets trapped.

You should also have provided a testcase showing a PASS->FAIL on the epilogue
code_addr fix.


> Smells like there is a
> flaw in the watchpoint code where it notices that the watchpoint goes
> out of scope, but still tries to evaluate the watchpoint condition.

BTW I do not say how many flaws are in GDB, there are many.  But so far the
functionality worked and now it does not.  So either we find a simple fix soon
enough or one should revert the patch.  Cross-comparison of various known
regressions get complicated.


> There should be debug info to tell us exactly when a certain variable
> goes out of scope, and the breakpoint/watchpoint code should use it.

For -O0 -g code the debug info is not perfect per instruction - this is why
for example the prologues need to be skipped.


> In absence of that debug info, assuming that the watchpoint goes out
> of scope when the function returns, combined with the
> in_function_epilogue_p() check will have to do the job.

Yes but you broke a functionality depending on existing bugs so you should
have also fixed these associated problems not visible before.


> > Also for the epilogue unwinder you would need to somehow fix:
> > 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"));
> 
> All unwinders are supposed to return a frame base that is "compatible"
> amongst unwinders, including the DWARF one.  Now that may be tricky if
> compilers don't agree on what the frame base (CFA) is.  But we should
> get this right for GCC, and that's all I care about.  If you'd ask me,
> that check should be removed.

I agree, CFA is computed for the same address in all unwinders I have seen so
far.


Thanks,
Jan