This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA] Problem after hitting breakpoint on Windows (with GDBserver)
On 03/13/2012 09:59 PM, Joel Brobecker wrote:
>>> + if (val == 0)
>>> + memcpy (bp_tgt->shadow_contents, readbuf, bp_tgt->placed_size);
>>>
>>> /* Write the breakpoint. */
>>> if (val == 0)
>>
>> Merge?
>
> I actually started that way, with the two blocks merged. But I felt
> that it was breaking the separation between the two steps. With the
> comments clearly separating the two steps, I didn't want to break
> that unless asked. So now I changed it.
If that's a concern, we can still keep it, like e.g.:
/* Fetch the memory contents "under" the breakpoint, and save it in
the shadow_contents buffer. */
readbuf = alloca (bp_tgt->placed_size);
val = target_read_memory (bp_tgt->placed_address, readbuf,
bp_tgt->placed_size);
if (val == 0)
{
/* Success, save it. */
bp_tgt->shadow_len = bp_tgt->placed_size;
memcpy (bp_tgt->shadow_contents, readbuf, bp_tgt->placed_size);
/* Now write the breakpoint instruction. */
val = target_write_raw_memory (bp_tgt->placed_address, bp,
bp_tgt->placed_size);
}
>
>>> + As a limitation, MYADDR must not be the shadow_contents buffer of one
>>
>> I wouldn't call it a limitation; it's more a design choice thing, like
>> memcpy doesn't handle overlapping buffers.
>
> OK - I just removed the "As a limitation" from the comments.
>
>> Otherwise this is fine with me.
>
> Thanks! Attached is a new version of the patch. The only changes
> should be the changes you pointed out.
>
>> An assertion in breakpoint_xfer_memory to catch that READBUF or
>> WRITEBUF doesn't overlap bp->target_info.shadow_contents would be
>> nice.
>
> I thought about that, but decided to look at that separately, since
> it doesn't help correctness, and can potentially be a little expensive
> (at least compared to just allocating a buffer on the heap - I think!).
Eh, it's meant to insure correctness. :-) Certainly a heap allocation on every
read is more expensive than a simple range check, and more so one that only
triggers when we have breakpoints in the range we're reading.
>
> But I don't mind writing a patch - probably a function in breakpoint.c
> and a gdb_assert calling that breakpoint?
Oh, I was only thinking of something along the lines of what Jan did on
gdbserver. That is, something like:
--- c/gdb/breakpoint.c
+++ w/gdb/breakpoint.c
@@ -1446,6 +1446,10 @@ breakpoint_xfer_memory (gdb_byte *readbuf, gdb_byte *writebuf,
if (readbuf != NULL)
{
+ gdb_assert (bl->target_info.shadow_contents >= readbuf + len
+ || readbuf >= (bl->target_info.shadow_contents
+ + bl->target_info.shadow_len));
+
/* Update the read buffer with this inserted breakpoint's
shadow. */
memcpy (readbuf + bp_addr - memaddr,
--
Pedro Alves