[RFA] Unified watchpoints for x86 platforms
Eli Zaretskii
eliz@is.elta.co.il
Sun Mar 11 03:19:00 GMT 2001
On Fri, 9 Mar 2001, Mark Kettenis wrote:
> IMHO, since currently only native debugging uses watchpoints, it
> shouldn't matter one way or the other. In the long run, when the code is
> multi-arched, Someone(tm) will have to figure out how to do that so that
> non-native targets would be able to benefit from this code. If and when
> they do, they will want to move the code back to i386-tdep.c. So why not
> leave it there in the first place?
>
> Simply because I'd like i386-tdep.c to contain only code that's
> multi-arch ready.
Why is this goal so important that it justifies preventing
non-multiarched targets from using watchpoints, and creating a
separate file on top of that?
> If someone really wants to use the code in non-native targets he/she
> should address the multi-arch problems.
IMHO, this means we are being too harsh to target maintainers.
Anyway, since you insist on moving the code to a separate file, I'll
do that. I just wish I understood the motivation for that better than
I do now.
> I will remove debugreg.h if no one objects. As for ptrace.h, is it wise
> to remove that as well? I'd imagine that just about every target will
> want to include it anyway.
>
> Yes, but the actual implementation of the I386_DR_LOW_* will live in
> an entirely different file.
That's one possibility. What I had in mind was something different;
for example:
#define I386_DR_LOW_SET_ADDR(dr,addr) \
ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[dr]),(addr))
#define I386_DR_LOW_SET_CONTROL(val) \
ptrace (6, inferior_pid, offsetof (struct user, u_debugreg[DR_CONTROL]),(val))
I thought that when the macros are defined like this, ptrace.h would
be most useful.
While writing the code, I tried to make it very easy for the targets
to start using it. As written, all they need to do is define a small
number of macros in tm-*.h header and say "./configure; make".
> It was in the go32-nat.c code which served as a prototype. IIRC, EBUSY
> is used in an error message printed by GDB when a breakpoint cannot be
> inserted. Perhaps I'm mistaken.
>
> I think you are.
Here's the relevant snippet from breakpoint.c:insert_breakpoints (with
some of the code removed for brevity):
if (b->type == bp_hardware_breakpoint)
val = target_insert_hw_breakpoint (b->address, b->shadow_contents);
else
{
...
val = target_insert_breakpoint (b->address, b->shadow_contents);
}
if (val)
{
/* Can't set the breakpoint. */
#if defined (DISABLE_UNSETTABLE_BREAK)
...
#endif
{
target_terminal_ours_for_output ();
warning ("Cannot insert breakpoint %d:", b->number);
#ifdef ONE_PROCESS_WRITETEXT
warning ("The same program may be running in another process.");
#endif
memory_error (val, b->address); /* which bombs us out */
}
}
As you see, it calls memory_error with the value returned by
target_insert_hw_breakpoint. memory_error then interprets this arg
as a value of errno and prints the text returned by safe_strerror for
it.
So the question is: what do we want GDB to print when it fails to
insert a breakpoint, hardware or otherwise?
More information about the Gdb-patches
mailing list