Re: [patch] ia64: Fix breakpoints memory shadow

Joel Brobecker
2008-11-22T06:38:40Z

> gdb/doc/
> 2008-11-20 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdbint.texinfo (Target Conditionals): Extend the
> gdbarch_breakpoint_from_pc description.

This part needs to be reviewed by Eli.

> One can reproduce it more easily by simple `b *0':
> echo 'int main (void) { return 0; }' >mainx.c; gcc -o mainx mainx.c -Wall -ggdb2; ./gdb -nx -ex start -ex 'b *0' ./mainx

Ah, yes - if you place a breakpoint at an invalid address (whether it
was invalid right from the start as in your case, or if it became
invalid later on)...

> > I would have just said that we need to be able to read the original memory
> > location to produce the breakpoint sequence.
>
> It currently depends on `set breakpoint always-inserted'. In the
> default mode it broke the testsuite. Found `return NULL' as a minimal
> regression-free solution.

Right. I wasn't questioning the code, I just wasn't sure about
the comment.

> + /* It may be currently unreachable breakpoint which is never permanent, it
> + may happen for `b *0' and `set breakpoint always-inserted off'. */

Maybe just an English suggestion, and a suggestion to make your comment
more general:

/* The memory might be unreachable. This can happen, for instance,
when the user inserts a breakpoint at an invalid address. */

You'll notice that I removed the reference to permanent breakpoints,
because I still don't understand why it's relevant. I suggest you
checkin your patch with the comment as I suggested (once the doco
part is approved by Eli), and then we can discuss together just
that one comment (reviewing the whole patch at each iteration is
a little time-consuming).

Thanks,
--
Joel