From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17724 invoked by alias); 16 Apr 2009 17:08:46 -0000 Received: (qmail 17388 invoked by uid 22791); 16 Apr 2009 17:08:40 -0000 X-SWARE-Spam-Status: No, hits=-1.6 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_33,J_CHICKENPOX_37,J_CHICKENPOX_44 X-Spam-Check-By: sourceware.org Received: from mail3.caviumnetworks.com (HELO mail3.caviumnetworks.com) (12.108.191.235) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 16 Apr 2009 17:08:33 +0000 Received: from exch4.caveonetworks.com (Not Verified[192.168.16.23]) by mail3.caviumnetworks.com with MailMarshal (v6,2,2,3503) id ; Thu, 16 Apr 2009 13:08:27 -0400 Received: from exch4.caveonetworks.com ([192.168.16.23]) by exch4.caveonetworks.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 16 Apr 2009 10:08:07 -0700 Received: from dd1.caveonetworks.com ([64.169.86.201]) by exch4.caveonetworks.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 16 Apr 2009 10:08:07 -0700 Message-ID: <49E765F7.1030201@caviumnetworks.com> Date: Thu, 16 Apr 2009 17:08:00 -0000 From: David Daney User-Agent: Thunderbird 2.0.0.21 (X11/20090320) MIME-Version: 1.0 To: Pedro Alves , gdb-patches@sourceware.org Subject: Re: [Patch 2/2] MIPS hardware watchpoint support. References: <49D9AECB.7040302@gmail.com> <200904111817.36236.pedro@codesourcery.com> In-Reply-To: <200904111817.36236.pedro@codesourcery.com> Content-Type: multipart/mixed; boundary="------------080905020602080206010207" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-04/txt/msg00366.txt.bz2 This is a multi-part message in MIME format. --------------080905020602080206010207 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-length: 15190 Pedro Alves wrote: > On Monday 06 April 2009 08:27:07, David Daney wrote: >> Support for hardware watchpoints for mips-linux was merged into the >> Linux kernel for version 2.6.28, with some bug fixes in 2.6.29. This >> patch adds the corresponding gdb support. It has been tested on both >> mipsel-linux (32-bit kernel) and mips64-linux (64-bit kernel). > > Thanks! I'll pretend to know a thing about mips from here on, and > hope that it doesn't show too much that I don't. > > In general, this looks very good to me, although I didn't > make an effort to understand all the bit juggling going on. Minor > comments to the code below. Then comments to the tests follow that. > >> 2009-04-05 David Daney > >> (get_irw_mask, get_reg_mask, get_num_valid, get_watchlo, >> set_watchlo, get_watchhi, set_watchhi, mips_show_dr, > > Please split these context lines like so instead, as per the > coding conventions: > >> (get_irw_mask, get_reg_mask, get_num_valid, get_watchlo) >> (set_watchlo, get_watchhi, set_watchhi, mips_show_dr) > > Done. All these years and there is still more to learn about the coding conventions. >> +/* Assuming usable watch registers, return the irw_mask. */ >> +static uint32_t > > Please add an empty line between comment and function. Here > and elsewhere. Done. > >> +get_irw_mask (struct pt_watch_regs *regs, int set) >> +{ >> + switch (regs->style) >> + { >> + case pt_watch_style_mips32: >> + return regs->mips32.watch_masks[set] & IRW_MASK; >> + case pt_watch_style_mips64: >> + return regs->mips64.watch_masks[set] & IRW_MASK; >> + default: >> + gdb_assert (0); > > Please don't write 'gdb_assert (0)', instead make that a > more bit more friendly `internal_error'. Here and > elsewhere. > Done. >> + printf_unfiltered (" (addr=%lx, len=%d, type=%s)", >> + /* This code is for mips, so casting CORE_ADDR >> + to unsigned long should be okay. */ >> + (unsigned long)addr, len, > > Why bother to explain that instead of using `paddr'? If there's > a reason, then it would be nice if it was spelled out in the > comment. Comment deleted, and types casted to (unsigned long long). >> + for (i = 0; i < MAX_DEBUG_REGISTER; i++) >> + { > > Unneeded braces. You have a couple more in the patch. Fixed. > >> + printf_unfiltered ("\tDR%d: lo=0x%s, hi=0x%s\n", >> + i, paddr (get_watchlo (&watch_mirror, i)), >> + paddr (get_watchhi (&watch_mirror, i))); >> + } >> +} >> + >> + { >> + perror_with_name (_("Couldn't write debug register")); >> + return -1; > > perror doesn't return, no need for `return -1'. Fixed. >> + >> + watch_mirror = watch_readback; >> + populate_regs_from_watches (&watch_mirror); >> + >> + retval = write_watchpoint_regs(); > > ^ missing space. All instances fixed. >> + deprecated_add_set_cmd ("show-debug-regs", class_maintenance, >> + var_boolean, (char *) &maint_show_dr, _("\ >> +Set whether to show variables that mirror the mips debug registers.\n\ >> +Use \"on\" to enable, \"off\" to disable.\n\ >> +If enabled, the debug registers values are shown when GDB inserts\n\ >> +or removes a hardware breakpoint or watchpoint, and when the inferior\n\ >> +triggers a breakpoint or watchpoint."), >> + &maintenancelist); > > Argh. I've posted a patch that removes deprecated_add_set_cmd, but > never got around to committing it. > > http://sourceware.org/ml/gdb-patches/2009-01/msg00119.html > > When I get around to finish off that patch, I'll adjust this > file too --- I assume that is OK with you. > > However, reusing the x86 command does avoid needing to document > it, but, could you remove the reference to x86 in the current > docs? > > @kindex maint show-debug-regs > @cindex x86 hardware debug registers > @item maint show-debug-regs > Control whether to show variables that mirror the x86 hardware debug > registers. Use @code{ON} to enable, @code{OFF} to disable. If > enabled, the debug registers values are shown when @value{GDBN} inserts or > removes a hardware breakpoint or watchpoint, and when the inferior > triggers a hardware-assisted breakpoint or watchpoint. > > ... since it now applies to mips as well. Done. >> For this submission I tested on a mipsel-linux system with these results >> (with comments): >> >> --- ./gdb/testsuite/gdb.sum 2009-04-05 20:53:20.000000000 -0700 >> +++ gdb.sum 2009-04-05 17:53:05.000000000 -0700 >> @@ -1,4 +1,4 @@ >> -Test Run By root on Sun Apr 5 18:08:05 2009 >> +Test Run By root on Sun Apr 5 14:45:02 2009 >> Native configuration is mipsel-unknown-linux-gnu >> >> === gdb tests === >> @@ -5717,15 +5717,15 @@ >> PASS: gdb.base/recurse.exp: continue to recurse (a = 5) >> PASS: gdb.base/recurse.exp: next over b = 0 in second instance >> PASS: gdb.base/recurse.exp: set second instance watchpoint >> -PASS: gdb.base/recurse.exp: continue to second instance watchpoint, >> first time >> -PASS: gdb.base/recurse.exp: continue to recurse (a = 4) >> -PASS: gdb.base/recurse.exp: continue to recurse (a = 3) >> -PASS: gdb.base/recurse.exp: continue to recurse (a = 2) >> -PASS: gdb.base/recurse.exp: continue to recurse (a = 1) >> -PASS: gdb.base/recurse.exp: continue to second instance watchpoint, >> second time >> -PASS: gdb.base/recurse.exp: second instance watchpoint deleted when >> leaving scope >> -PASS: gdb.base/recurse.exp: continue to first instance watchpoint, >> second time >> -PASS: gdb.base/recurse.exp: first instance watchpoint deleted when >> leaving scope >> +FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, >> first time >> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 4) >> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 3) >> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 2) >> +FAIL: gdb.base/recurse.exp: continue to recurse (a = 1) >> +FAIL: gdb.base/recurse.exp: continue to second instance watchpoint, >> second time >> +FAIL: gdb.base/recurse.exp: second instance watchpoint deleted when >> leaving scope >> +FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, >> second time >> +FAIL: gdb.base/recurse.exp: first instance watchpoint deleted when >> leaving scope >> >> These failures are caused by the fact that the test assumes more than a >> the single watch register supported by the test system. Although this >> patch supports up to eight registers, the test platform only has one. >> We return true from mips_linux_can_use_hw_breakpoint for multiple >> requests as it is not possible to know how many watchpoints are >> enabled. Later gdb tries to insert more than one watchpoint and all but >> the first fail. > > Could we detect that, and fail the rest of the test gracefully? > What error message do you get? > I slightly modified the patch so that it makes more conservative estimates about how many hardware watches it can handle. This fixes these failures. > >> -PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger >> +FAIL: gdb.mi/mi-watch.exp: hw: watchpoint trigger (unknown output after >> running) >> >> Reported instruction location of the watchpoint trigger is one >> instruction later and one line later. > > Why is that? I was mistaken, it is earlier, not later. With hardware watch points, $pc points to the faulting instruction, with software watch points $pc points to the following instruction. In a couple of tests, this results in the reported line number being different than the expected value. Here is the revised patch and its test results: --- pre/gdb.sum 2009-04-15 12:12:33.000000000 -0700 +++ post/gdb.sum 2009-04-15 10:22:37.000000000 -0700 @@ -1,4 +1,4 @@ -Test Run By root on Wed Apr 15 10:42:35 2009 +Test Run By root on Tue Apr 14 16:28:52 2009 Native configuration is mips64-unknown-linux-gnu === gdb tests === @@ -5684,7 +5684,7 @@ PASS: gdb.base/recurse.exp: continue to recurse (a = 1) PASS: gdb.base/recurse.exp: continue to second instance watchpoint, second time PASS: gdb.base/recurse.exp: second instance watchpoint deleted when leaving scope -PASS: gdb.base/recurse.exp: continue to first instance watchpoint, second time +FAIL: gdb.base/recurse.exp: continue to first instance watchpoint, second time Different line number as explained above. PASS: gdb.base/recurse.exp: first instance watchpoint deleted when leaving scope Running ../../../src/gdb/testsuite/gdb.base/regs.exp ... Running ../../../src/gdb/testsuite/gdb.base/relational.exp ... @@ -8272,26 +8272,26 @@ PASS: gdb.base/watch_thread_num.exp: Disable breakpoint 2 PASS: gdb.base/watch_thread_num.exp: Watchpoint on shared variable PASS: gdb.base/watch_thread_num.exp: info breakpoint 3 -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 1 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 1 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 2 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 2 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 3 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 3 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 4 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 4 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 5 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 5 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 6 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 6 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 7 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 7 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 8 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 8 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 9 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 9 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 10 (timeout) -FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 10 (timeout) +PASS: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 1 +PASS: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 1 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 2 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 2 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 3 +PASS: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 3 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 4 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 4 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 5 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 5 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 6 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 6 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 7 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 7 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 8 +PASS: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 8 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 9 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 9 +FAIL: gdb.base/watch_thread_num.exp: Watchpoint triggered iteration 10 +FAIL: gdb.base/watch_thread_num.exp: Check thread that triggered iteration 10 Running ../../../src/gdb/testsuite/gdb.base/watchpoint-hw.exp ... Running ../../../src/gdb/testsuite/gdb.base/watchpoint-solib.exp ... PASS: gdb.base/watchpoint-solib.exp: set pending breakpoint @@ -8476,7 +8476,7 @@ PASS: gdb.cp/annota2.exp: breakpoint at main PASS: gdb.cp/annota2.exp: run until main breakpoint PASS: gdb.cp/annota2.exp: set watch on a.x -PASS: gdb.cp/annota2.exp: watch triggered on a.x +FAIL: gdb.cp/annota2.exp: watch triggered on a.x PASS: gdb.cp/annota2.exp: annotate-quit Running ../../../src/gdb/testsuite/gdb.cp/annota3.exp ... PASS: gdb.cp/annota3.exp: breakpoint main @@ -8488,7 +8488,7 @@ PASS: gdb.cp/annota3.exp: break at main PASS: gdb.cp/annota3.exp: second run until main breakpoint PASS: gdb.cp/annota3.exp: set watch on a.x -PASS: gdb.cp/annota3.exp: watch triggered on a.x +FAIL: gdb.cp/annota3.exp: watch triggered on a.x The two annotation failures are explained in the PASS: gdb.cp/annota3.exp: annotate-quit Running ../../../src/gdb/testsuite/gdb.cp/anon-union.exp ... PASS: gdb.cp/anon-union.exp: next 1 @@ -11976,7 +11976,7 @@ PASS: gdb.mi/mi-watch.exp: hw: mi runto callee4 PASS: gdb.mi/mi-watch.exp: hw: break-watch operation PASS: gdb.mi/mi-watch.exp: hw: list of watchpoints -PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger +FAIL: gdb.mi/mi-watch.exp: hw: watchpoint trigger (unknown output after running) Different line number as explained above. PASS: gdb.mi/mi-watch.exp: hw: watchpoint trigger Running ../../../src/gdb/testsuite/gdb.mi/mi2-basics.exp ... PASS: gdb.mi/mi2-basics.exp: acceptance of MI operations @@ -12609,7 +12609,7 @@ PASS: gdb.mi/mi2-watch.exp: hw: mi runto callee4 PASS: gdb.mi/mi2-watch.exp: hw: break-watch operation PASS: gdb.mi/mi2-watch.exp: hw: list of watchpoints -PASS: gdb.mi/mi2-watch.exp: hw: watchpoint trigger +FAIL: gdb.mi/mi2-watch.exp: hw: watchpoint trigger (unknown output after running) Different line number as explained above. PASS: gdb.mi/mi2-watch.exp: hw: watchpoint trigger Running ../../../src/gdb/testsuite/gdb.modula2/unbounded-array.exp ... PASS: gdb.modula2/unbounded-array.exp: switch to modula-2 @@ -13351,10 +13351,10 @@ PASS: gdb.threads/tls.exp: info address a_thread_local Running ../../../src/gdb/testsuite/gdb.threads/watchthreads.exp ... PASS: gdb.threads/watchthreads.exp: successfully compiled posix threads test case -FAIL: gdb.threads/watchthreads.exp: watch args[0] +PASS: gdb.threads/watchthreads.exp: watch args[0] FAIL: gdb.threads/watchthreads.exp: watch args[1] FAIL: gdb.threads/watchthreads.exp: threaded watch loop -FAIL: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit +PASS: gdb.threads/watchthreads.exp: first watchpoint on args[0] hit FAIL: gdb.threads/watchthreads.exp: first watchpoint on args[1] hit FAIL: gdb.threads/watchthreads.exp: watchpoint on args[0] hit in thread FAIL: gdb.threads/watchthreads.exp: watchpoint on args[1] hit in thread OK to commit? --------------080905020602080206010207 Content-Type: text/plain; name="mips-watch.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="mips-watch.patch" Content-length: 22095 gdb/ 2009-04-16 David Daney * mips-linux-nat.c (command.h, gdbcmd.h, gdb_assert.h): New #includes. (maint_show_dr, super_close): New variables. (super_fetch_registers, super_store_registers): Make static. (PTRACE_GET_WATCH_REGS, PTRACE_SET_WATCH_REGS, W_BIT, R_BIT, I_BIT) (W_MASK, R_MASK, I_MASK, IRW_MASK, MAX_DEBUG_REGISTER): Define. (pt_watch_style): Define new enum. (mips32_watch_regs, mips64_watch_regs, pt_watch_regs, mips_watchpoint): Define new structs. (watch_readback_valid, watch_readback, current_watches, watch_mirror): New variables. (get_irw_mask, get_reg_mask, get_num_valid, get_watchlo) (set_watchlo, get_watchhi, set_watchhi, mips_show_dr) (mips_linux_read_watch_registers, mips_linux_can_use_hw_breakpoint) (mips_linux_stopped_by_watchpoint, mips_linux_stopped_data_address) (type_to_irw, fill_mask, try_one_watch) (mips_linux_region_ok_for_hw_watchpoint, write_watchpoint_regs) (mips_linux_new_thread, populate_regs_from_watches) (mips_linux_insert_watchpoint, mips_linux_remove_watchpoint) (mips_linux_close): New functions. (_initialize_mips_linux_nat): Register watchpoint functions with the target_ops. Add show-debug-regs maintenance command. gdb/doc/ 2009-04-16 David Daney * gdb.texinfo (maint show-debug-regs): Remove mention of x86. Index: mips-linux-nat.c =================================================================== RCS file: /cvs/src/src/gdb/mips-linux-nat.c,v retrieving revision 1.32 diff -u -p -r1.32 mips-linux-nat.c --- mips-linux-nat.c 26 Feb 2009 19:44:39 -0000 1.32 +++ mips-linux-nat.c 16 Apr 2009 16:34:56 -0000 @@ -19,6 +19,9 @@ along with this program. If not, see . */ #include "defs.h" +#include "command.h" +#include "gdbcmd.h" +#include "gdb_assert.h" #include "inferior.h" #include "mips-tdep.h" #include "target.h" @@ -44,11 +47,19 @@ we'll clear this and use PTRACE_PEEKUSER instead. */ static int have_ptrace_regsets = 1; +/* Whether or not to print the mirrored debug registers. */ + +static int maint_show_dr; + /* Saved function pointers to fetch and store a single register using PTRACE_PEEKUSER and PTRACE_POKEUSER. */ -void (*super_fetch_registers) (struct target_ops *, struct regcache *, int); -void (*super_store_registers) (struct target_ops *, struct regcache *, int); +static void (*super_fetch_registers) (struct target_ops *, + struct regcache *, int); +static void (*super_store_registers) (struct target_ops *, + struct regcache *, int); + +static void (*super_close) (int); /* Map gdb internal register number to ptrace ``address''. These ``addresses'' are normally defined in . @@ -357,12 +368,697 @@ mips_linux_read_description (struct targ return tdesc_mips64_linux; } +#ifndef PTRACE_GET_WATCH_REGS +# define PTRACE_GET_WATCH_REGS 0xd0 +#endif + +#ifndef PTRACE_SET_WATCH_REGS +# define PTRACE_SET_WATCH_REGS 0xd1 +#endif + +#define W_BIT 0 +#define R_BIT 1 +#define I_BIT 2 + +#define W_MASK (1 << W_BIT) +#define R_MASK (1 << R_BIT) +#define I_MASK (1 << I_BIT) + +#define IRW_MASK (I_MASK | R_MASK | W_MASK) + +enum pt_watch_style { + pt_watch_style_mips32, + pt_watch_style_mips64 +}; + +#define MAX_DEBUG_REGISTER 8 + +/* A value of zero in a watchlo indicates that it is available. */ + +struct mips32_watch_regs +{ + uint32_t watchlo[MAX_DEBUG_REGISTER]; + /* Lower 16 bits of watchhi. */ + uint16_t watchhi[MAX_DEBUG_REGISTER]; + /* Valid mask and I R W bits. + * bit 0 -- 1 if W bit is usable. + * bit 1 -- 1 if R bit is usable. + * bit 2 -- 1 if I bit is usable. + * bits 3 - 11 -- Valid watchhi mask bits. + */ + uint16_t watch_masks[MAX_DEBUG_REGISTER]; + /* The number of valid watch register pairs. */ + uint32_t num_valid; + /* There is confusion across gcc versions about structure alignment, + so we force 8 byte alignment for these structures so they match + the kernel even if it was build with a different gcc version. */ +} __attribute__ ((aligned (8))); + +struct mips64_watch_regs +{ + uint64_t watchlo[MAX_DEBUG_REGISTER]; + uint16_t watchhi[MAX_DEBUG_REGISTER]; + uint16_t watch_masks[MAX_DEBUG_REGISTER]; + uint32_t num_valid; +} __attribute__ ((aligned (8))); + +struct pt_watch_regs +{ + enum pt_watch_style style; + union + { + struct mips32_watch_regs mips32; + struct mips64_watch_regs mips64; + }; +}; + +/* -1 if the kernel and/or CPU do not support watch registers. + 1 if watch_readback is valid and we can read style, num_valid + and the masks. + 0 if we need to read the watch_readback. */ + +static int watch_readback_valid; + +/* Cached watch register read values. */ + +static struct pt_watch_regs watch_readback; + +/* We keep list of all watchpoints we should install and calculate the + watch register values each time the list changes. This allows for + easy sharing of watch registers for more than one watchpoint. */ + +struct mips_watchpoint +{ + CORE_ADDR addr; + int len; + int type; + struct mips_watchpoint *next; +}; + +static struct mips_watchpoint *current_watches; + +/* The current set of watch register values for writing the + registers. */ + +static struct pt_watch_regs watch_mirror; + +/* Assuming usable watch registers, return the irw_mask. */ + +static uint32_t +get_irw_mask (struct pt_watch_regs *regs, int set) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + return regs->mips32.watch_masks[set] & IRW_MASK; + case pt_watch_style_mips64: + return regs->mips64.watch_masks[set] & IRW_MASK; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, return the reg_mask. */ + +static uint32_t +get_reg_mask (struct pt_watch_regs *regs, int set) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + return regs->mips32.watch_masks[set] & ~IRW_MASK; + case pt_watch_style_mips64: + return regs->mips64.watch_masks[set] & ~IRW_MASK; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, return the num_valid. */ + +static uint32_t +get_num_valid (struct pt_watch_regs *regs) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + return regs->mips32.num_valid; + case pt_watch_style_mips64: + return regs->mips64.num_valid; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, return the watchlo. */ + +static CORE_ADDR +get_watchlo (struct pt_watch_regs *regs, int set) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + return regs->mips32.watchlo[set]; + case pt_watch_style_mips64: + return regs->mips64.watchlo[set]; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, set a watchlo value. */ + +static void +set_watchlo (struct pt_watch_regs *regs, int set, CORE_ADDR value) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + /* The cast will never throw away bits as 64 bit addresses can + never be used on a 32 bit kernel. */ + regs->mips32.watchlo[set] = (uint32_t)value; + break; + case pt_watch_style_mips64: + regs->mips64.watchlo[set] = value; + break; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, return the watchhi. */ + +static uint32_t +get_watchhi (struct pt_watch_regs *regs, int n) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + return regs->mips32.watchhi[n]; + case pt_watch_style_mips64: + return regs->mips64.watchhi[n]; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +/* Assuming usable watch registers, set a watchhi value. */ + +static void +set_watchhi (struct pt_watch_regs *regs, int n, uint16_t value) +{ + switch (regs->style) + { + case pt_watch_style_mips32: + regs->mips32.watchhi[n] = value; + break; + case pt_watch_style_mips64: + regs->mips64.watchhi[n] = value; + break; + default: + internal_error (__FILE__, __LINE__, + _("Unrecognized watch register style")); + } +} + +static void +mips_show_dr (const char *func, CORE_ADDR addr, + int len, enum target_hw_bp_type type) +{ + int i; + + puts_unfiltered (func); + if (addr || len) + printf_unfiltered (" (addr=%llx, len=%d, type=%s)", + (unsigned long long)addr, len, + type == hw_write ? "data-write" + : (type == hw_read ? "data-read" + : (type == hw_access ? "data-read/write" + : (type == hw_execute ? "instruction-execute" + : "??unknown??")))); + puts_unfiltered (":\n"); + + for (i = 0; i < MAX_DEBUG_REGISTER; i++) + printf_unfiltered ("\tDR%d: lo=0x%s, hi=0x%s\n", + i, paddr (get_watchlo (&watch_mirror, i)), + paddr (get_watchhi (&watch_mirror, i))); +} + +/* Return 1 if watch registers are usable. Cached information is used + unless force is true. */ + +static int +mips_linux_read_watch_registers (int force) +{ + int tid; + + if (force || watch_readback_valid == 0) + { + tid = ptid_get_lwp (inferior_ptid); + if (ptrace (PTRACE_GET_WATCH_REGS, tid, &watch_readback) == -1) + { + watch_readback_valid = -1; + return 0; + } + switch (watch_readback.style) + { + case pt_watch_style_mips32: + if (watch_readback.mips32.num_valid == 0) + { + watch_readback_valid = -1; + return 0; + } + break; + case pt_watch_style_mips64: + if (watch_readback.mips64.num_valid == 0) + { + watch_readback_valid = -1; + return 0; + } + break; + default: + watch_readback_valid = -1; + return 0; + } + /* Watch registers appear to be usable. */ + watch_readback_valid = 1; + } + return (watch_readback_valid == 1) ? 1 : 0; +} + +/* Convert GDB's type to an IRW mask. */ + +static unsigned +type_to_irw (int type) +{ + switch (type) + { + case hw_write: + return W_MASK; + case hw_read: + return R_MASK; + case hw_access: + return (W_MASK | R_MASK); + default: + return 0; + } +} + +/* Target to_can_use_hw_breakpoint implementation. Return 1 if we can + handle the specified watch type. */ + +static int +mips_linux_can_use_hw_breakpoint (int type, int cnt, int ot) +{ + int i; + uint32_t wanted_mask, irw_mask; + + if (!mips_linux_read_watch_registers (0)) + return 0; + + switch (type) + { + case bp_hardware_watchpoint: + wanted_mask = W_MASK; + break; + case bp_read_watchpoint: + wanted_mask = R_MASK; + break; + case bp_access_watchpoint: + wanted_mask = R_MASK | W_MASK; + break; + default: + return 0; + } + + for (i = 0; i < get_num_valid (&watch_readback) && cnt; i++) + { + irw_mask = get_irw_mask (&watch_readback, i); + if ((irw_mask & wanted_mask) == wanted_mask) + cnt--; + } + return (cnt == 0) ? 1 : 0; +} + +/* Target to_stopped_by_watchpoint implementation. Return 1 if + stopped by watchpoint. The watchhi R and W bits indicate the watch + register triggered. */ + +static int +mips_linux_stopped_by_watchpoint (void) +{ + int n; + int num_valid; + + if (!mips_linux_read_watch_registers (1)) + return 0; + + num_valid = get_num_valid (&watch_readback); + + for (n = 0; n < MAX_DEBUG_REGISTER && n < num_valid; n++) + if (get_watchhi (&watch_readback, n) & (R_MASK | W_MASK)) + return 1; + + return 0; +} + +/* Target to_stopped_data_address implementation. Set the address + where the watch triggered (if known). Return 1 if the address was + known. */ + +static int +mips_linux_stopped_data_address (struct target_ops *t, CORE_ADDR *paddr) +{ + /* On mips we don't know the low order 3 bits of the data address, + so we must return false. */ + return 0; +} + +/* Set any low order bits in mask that are not set. */ + +static CORE_ADDR +fill_mask (CORE_ADDR mask) +{ + CORE_ADDR f = 1; + while (f && f < mask) + { + mask |= f; + f <<= 1; + } + return mask; +} + +/* Try to add a single watch to the specified registers. Return 1 on + success, 0 on failure. */ + +static int +try_one_watch (struct pt_watch_regs *regs, CORE_ADDR addr, + int len, unsigned irw) +{ + CORE_ADDR base_addr, last_byte, break_addr, segment_len; + CORE_ADDR mask_bits, t_low, t_low_end; + uint16_t t_hi; + int i, free_watches; + struct pt_watch_regs regs_copy; + + if (len <= 0) + return 0; + + last_byte = addr + len - 1; + mask_bits = fill_mask (addr ^ last_byte) | IRW_MASK; + base_addr = addr & ~mask_bits; + + /* Check to see if it is covered by current registers. */ + for (i = 0; i < get_num_valid (regs); i++) + { + t_low = get_watchlo (regs, i); + if (t_low != 0 && irw == ((unsigned)t_low & irw)) + { + t_hi = get_watchhi (regs, i) | IRW_MASK; + t_low &= ~(CORE_ADDR)t_hi; + if (addr >= t_low && last_byte <= (t_low + t_hi)) + return 1; + } + } + /* Try to find an empty register. */ + free_watches = 0; + for (i = 0; i < get_num_valid (regs); i++) + { + t_low = get_watchlo (regs, i); + if (t_low == 0 && irw == (get_irw_mask (regs, i) & irw)) + { + if (mask_bits <= (get_reg_mask (regs, i) | IRW_MASK)) + { + /* It fits, we'll take it. */ + set_watchlo (regs, i, base_addr | irw); + set_watchhi (regs, i, mask_bits & ~IRW_MASK); + return 1; + } + else + { + /* It doesn't fit, but has the proper IRW capabilities. */ + free_watches++; + } + } + } + if (free_watches > 1) + { + /* Try to split it across several registers. */ + regs_copy = *regs; + for (i = 0; i < get_num_valid (®s_copy); i++) + { + t_low = get_watchlo (®s_copy, i); + t_hi = get_reg_mask (®s_copy, i) | IRW_MASK; + if (t_low == 0 && irw == (t_hi & irw)) + { + t_low = addr & ~(CORE_ADDR)t_hi; + break_addr = t_low + t_hi + 1; + if (break_addr >= addr + len) + segment_len = len; + else + segment_len = break_addr - addr; + mask_bits = fill_mask (addr ^ (addr + segment_len - 1)); + set_watchlo (®s_copy, i, (addr & ~mask_bits) | irw); + set_watchhi (®s_copy, i, mask_bits & ~IRW_MASK); + if (break_addr >= addr + len) + { + *regs = regs_copy; + return 1; + } + len = addr + len - break_addr; + addr = break_addr; + } + } + } + /* It didn't fit anywhere, we failed. */ + return 0; +} + +/* Target to_region_ok_for_hw_watchpoint implementation. Return 1 if + the specified region can be covered by the watch registers. */ + +static int +mips_linux_region_ok_for_hw_watchpoint (CORE_ADDR addr, int len) +{ + struct pt_watch_regs dummy_regs; + int i; + + if (!mips_linux_read_watch_registers (0)) + return 0; + + dummy_regs = watch_readback; + /* Clear them out. */ + for (i = 0; i < get_num_valid (&dummy_regs); i++) + set_watchlo (&dummy_regs, i, 0); + return try_one_watch (&dummy_regs, addr, len, 0); +} + + +/* Write the mirrored watch register values for each thread. */ + +static int +write_watchpoint_regs (void) +{ + struct lwp_info *lp; + ptid_t ptid; + int tid; + + ALL_LWPS (lp, ptid) + { + tid = ptid_get_lwp (ptid); + if (ptrace (PTRACE_SET_WATCH_REGS, tid, &watch_mirror) == -1) + perror_with_name (_("Couldn't write debug register")); + } + return 0; +} + +/* linux_nat new_thread implementation. Write the mirrored watch + register values for the new thread. */ + +static void +mips_linux_new_thread (ptid_t ptid) +{ + int tid; + + if (!mips_linux_read_watch_registers (0)) + return; + + tid = ptid_get_lwp (ptid); + if (ptrace (PTRACE_SET_WATCH_REGS, tid, &watch_mirror) == -1) + perror_with_name (_("Couldn't write debug register")); +} + +/* Fill in the watch registers with the currently cached watches. */ + +static void +populate_regs_from_watches (struct pt_watch_regs *regs) +{ + struct mips_watchpoint *w; + int i; + + /* Clear them out. */ + for (i = 0; i < get_num_valid (regs); i++) + { + set_watchlo (regs, i, 0); + set_watchhi (regs, i, 0); + } + + w = current_watches; + while (w) + { + i = try_one_watch (regs, w->addr, w->len, type_to_irw (w->type)); + /* They must all fit, because we previously calculated that they + would. */ + gdb_assert (i); + w = w->next; + } +} + +/* Target to_insert_watchpoint implementation. Try to insert a new + watch. Return zero on success. */ + +static int +mips_linux_insert_watchpoint (CORE_ADDR addr, int len, int type) +{ + struct pt_watch_regs regs; + struct mips_watchpoint *new_watch; + struct mips_watchpoint **pw; + + int i; + int retval; + + if (!mips_linux_read_watch_registers (0)) + return -1; + + if (len <= 0) + return -1; + + regs = watch_readback; + /* Add the current watches. */ + populate_regs_from_watches (®s); + + /* Now try to add the new watch. */ + if (!try_one_watch (®s, addr, len, type_to_irw (type))) + return -1; + + /* It fit. Stick it on the end of the list. */ + new_watch = (struct mips_watchpoint *) + xmalloc (sizeof (struct mips_watchpoint)); + new_watch->addr = addr; + new_watch->len = len; + new_watch->type = type; + new_watch->next = NULL; + + pw = ¤t_watches; + while (*pw != NULL) + pw = &(*pw)->next; + *pw = new_watch; + + watch_mirror = regs; + retval = write_watchpoint_regs (); + + if (maint_show_dr) + mips_show_dr ("insert_watchpoint", addr, len, type); + + return retval; +} + +/* Target to_remove_watchpoint implementation. Try to remove a watch. + Return zero on success. */ + +static int +mips_linux_remove_watchpoint (CORE_ADDR addr, int len, int type) +{ + int retval; + int deleted_one; + + struct mips_watchpoint **pw; + struct mips_watchpoint *w; + + /* Search for a known watch that matches. Then unlink and free + it. */ + deleted_one = 0; + pw = ¤t_watches; + while ((w = *pw)) + { + if (w->addr == addr && w->len == len && w->type == type) + { + *pw = w->next; + xfree (w); + deleted_one = 1; + break; + } + pw = &(w->next); + } + + if (!deleted_one) + return -1; /* We don't know about it, fail doing nothing. */ + + /* At this point watch_readback is known to be valid because we + could not have added the watch without reading it. */ + gdb_assert (watch_readback_valid == 1); + + watch_mirror = watch_readback; + populate_regs_from_watches (&watch_mirror); + + retval = write_watchpoint_regs (); + + if (maint_show_dr) + mips_show_dr ("remove_watchpoint", addr, len, type); + + return retval; +} + +/* Target to_close implementation. Free any watches and call the + super implementation. */ + +static void +mips_linux_close (int quitting) +{ + struct mips_watchpoint *w; + struct mips_watchpoint *nw; + + /* Clean out the current_watches list. */ + w = current_watches; + while (w) + { + nw = w->next; + xfree (w); + w = nw; + } + current_watches = NULL; + + if (super_close) + super_close (quitting); +} + void _initialize_mips_linux_nat (void); void _initialize_mips_linux_nat (void) { - struct target_ops *t = linux_trad_target (mips_linux_register_u_offset); + struct target_ops *t; + + deprecated_add_set_cmd ("show-debug-regs", class_maintenance, + var_boolean, (char *) &maint_show_dr, _("\ +Set whether to show variables that mirror the mips debug registers.\n\ +Use \"on\" to enable, \"off\" to disable.\n\ +If enabled, the debug registers values are shown when GDB inserts\n\ +or removes a hardware breakpoint or watchpoint, and when the inferior\n\ +triggers a breakpoint or watchpoint."), + &maintenancelist); + + + t = linux_trad_target (mips_linux_register_u_offset); + + super_close = t->to_close; + t->to_close = mips_linux_close; super_fetch_registers = t->to_fetch_registers; super_store_registers = t->to_store_registers; @@ -370,9 +1066,17 @@ _initialize_mips_linux_nat (void) t->to_fetch_registers = mips64_linux_fetch_registers; t->to_store_registers = mips64_linux_store_registers; + t->to_can_use_hw_breakpoint = mips_linux_can_use_hw_breakpoint; + t->to_remove_watchpoint = mips_linux_remove_watchpoint; + t->to_insert_watchpoint = mips_linux_insert_watchpoint; + t->to_stopped_by_watchpoint = mips_linux_stopped_by_watchpoint; + t->to_stopped_data_address = mips_linux_stopped_data_address; + t->to_region_ok_for_hw_watchpoint = mips_linux_region_ok_for_hw_watchpoint; + t->to_read_description = mips_linux_read_description; linux_nat_add_target (t); + linux_nat_set_new_thread (t, mips_linux_new_thread); /* Initialize the standard target descriptions. */ initialize_tdesc_mips_linux (); Index: doc/gdb.texinfo =================================================================== RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v retrieving revision 1.579 diff -u -p -r1.579 gdb.texinfo --- doc/gdb.texinfo 9 Apr 2009 20:11:57 -0000 1.579 +++ doc/gdb.texinfo 16 Apr 2009 16:34:59 -0000 @@ -25715,9 +25715,9 @@ Configuring with @samp{--enable-profilin compiled with the @samp{-pg} compiler option. @kindex maint show-debug-regs -@cindex x86 hardware debug registers +@cindex hardware debug registers @item maint show-debug-regs -Control whether to show variables that mirror the x86 hardware debug +Control whether to show variables that mirror the hardware debug registers. Use @code{ON} to enable, @code{OFF} to disable. If enabled, the debug registers values are shown when @value{GDBN} inserts or removes a hardware breakpoint or watchpoint, and when the inferior --------------080905020602080206010207--