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

Jan Kratochvil
2011-06-13T20:46:43Z

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