This is the mail archive of the
gdb@sources.redhat.com
mailing list for the GDB project.
Re: [RFC] Unified watchpoints for x86 platforms
- To: Eli Zaretskii <eliz at is dot elta dot co dot il>
- Subject: Re: [RFC] Unified watchpoints for x86 platforms
- From: Mark Kettenis <kettenis at wins dot uva dot nl>
- Date: 15 Feb 2001 17:17:17 +0100
- Cc: gdb at sources dot redhat dot com
- References: <200009070855.EAA00749@albacore> <200009070855.EAA00749@albacore> <200009071500.LAA07756@indy.delorie.com> <200009081529.e88FTjx15960@debye.wins.uva.nl> <200102101533.KAA10417@indy.delorie.com> <200102151146.NAA28431@is.elta.co.il>
Eli Zaretskii <eliz@is.elta.co.il> writes:
> Ping!
>
> No one posted any approvals or disprovals of this design. Do I take
> the silence as a sign of agreement and start coding?
Sorry for not responding earlier. In general, your proposal looks
fine, but I think it is best to export functions similar to GDB's
target_* functions:
int i386_remove_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type);
int i386_insert_watchpoint (CORE_ADDR addr, int len,
enum target_hw_bp_type type);
int i386_insert_hw_breakpoint (CORE_ADDR addr, void *shadow);
int i386_remove_hw_breakpoint (CORE_ADDR addr, void *shadow);
etc.
Of course you can implement those on top of the functions mentioned below.
> > I started working on the unified support for hardware-assisted
> > breakpoints and watchpoints on x86 platforms (see TODO). Since I
> > don't feel I know enough about all the aspects of this on any platform
> > but DJGPP, I thought I'd better get the framework agreed to before I
> > start coding.
> >
> > Here's the API I suggest for use by higher-level GDB code:
> >
> > (Note: I'm not good at inventing names, so please suggest better
> > ones if you want.)
> >
> > int i386_hwbp_insert (int pid, CORE_ADDR addr, int len, int kind);
Is there any particular reason why you need the PID argument? AFAICS
it will always be equal to INFERIOR_PID, so I think we can do without
it. This is also true for the other i386_hwbp_* functions you're
proposing.
> >
> > This function inserts a breakpoint or watchpoint to watch memory
> > region starting at address ADDR whose length is LEN bytes. The
> > watchpoint will watch said memory region for accesses whose type
> > is defined by KIND:
> >
> > HW_READ break if the region is accessed for reading[1]
> > HW_WRITE break if the region is accessed for writing
> > HW_ACCESS break if the region is accessed for either
> > reading or writing
> > HW_IO_ACCESS same as HW_ACCESS type, but for I/O read/write
> > access[2]
> > HW_EXECUTE instruction execution breakpoint
Please consider using an enum instead of an int. It looks as if GDB's
convention is to use lower-case names for enum values.
> > The function returns 0 upon success, else -1.
> >
> > Notes:
> > [1] Since x86 doesn't support read data watchpoints, HW_READ will
> > actually be implemented as a read/write watchpoint, and relies
> > on higher-level GDB code to distinguish between reads and
> > writes. The infrastructure to support this is already in place
> > in breakpoint.c, since GDB 5.0.
Sounds OK.
> > [2] I/O watchpoints are not currently supported (AFAIK) by GDB on
> > any x86 platform. I can provide the code to handle it, but do
> > people think we should define a command to access this feature?
> > If so, should we provide separate read, write, and access types
> > of watchpoints, or a single access type (the only one supported
> > by x86's debug registers) is enough?
I think this can be added later if people want it.
> > Note that I/O watchpoints require that the DE (debug extensions)
> > flag in the CR4 register be set. I don't know what platforms
> > set it and under what circumstances.
I don't think you can do this in any of the x86 Unixoid systems.
> >
> > int i386_hwbp_remove (int pid, CORE_ADDR addr, int len, int kind);
> >
> > This function removes a breakpoint of watchpoint at address ADDR
> > which watches a memory region of LEN bytes and whose type is given
> > by KIND. It returns 0 upon success, else -1.
> >
> > int i386_hwbp_region_ok (CORE_ADDR addr, int len);
> >
> > This function tests whether a memory region of LEN bytes starting at
> > ADDR can be watched with debug registers. It returns 1 if the
> > region can be watched, 0 otherwise.
> >
> > int i386_hwbp_stopped_by_watchpoint (int pid);
> >
> > This function returns the address of a breakpoint or watchpoint
> > which could have stopped the debuggee. If no watchpoint triggered,
> > it returns 0.
> >
> > To actually insert and remove breakpoints and watchpoints, I need
> > low-level system-dependent functions. Here's the API I suggest for
> > this low-levwl layer. (These are macros so that targets could define
> > them on their nm-*.h files. On a typical Unix or GNU/Linux system,
> > each of these macros will call `ptrace' with suitable arguments.)
> >
> > void HWBP_SET_ADDR (int pid, int dr_num, CORE_ADDR addr);
> >
> > This macro sets the debug register DR_NUM in the inferior to watch
> > the address ADDR. DR_NUM can be in the range [0..3].
> >
> > void HWBP_SET_CONTROL (int pid, unsigned int val);
> >
> > This macro sets the DR7 debug control register in the inferior to
> > the value VAL.
> >
> > unsigned int HWBP_GET_STATUS (int pid);
> >
> > This macro returns the value of the DR6 debug status register from
> > the inferior.
Why not have simply long I386_GET_DR(int regnum) and I386_SET_DR(int
regnum, long value) and let the system-dependent decide how to map the
debug register number ([0..7], as used in the Intel documentation)
into whatever is needed?
> > In the discussion we had back in September, Mark said that the
> > status register should be per thread. Does that mean that we need
> > an additional argument (int tid?) to pass to HWBP_GET_STATUS? If
> > so, how will this argument get into the i386_hwbp_* functions which
> > will call these macros?
I don't think an additional argument is needed. When calling
HWBP_GET_STATUS, it is the current thread that has encountered a trap,
and INFERIOR_PID should be set appropriately.
> > Or maybe the target end can figure out the thread id by itself with
> > some TIDGET(pid) magic?
Indeed.
Mark