From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark Kettenis To: Eli Zaretskii Cc: gdb@sources.redhat.com Subject: Re: [RFC] Unified watchpoints for x86 platforms Date: Thu, 15 Feb 2001 08:17:00 -0000 Message-id: References: <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> X-SW-Source: 2001-02/msg00175.html Eli Zaretskii 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