Re: [patch] ia64: Fix breakpoints memory shadow

[Joel Brobecker <brobecker@adacore.com>]

> 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