This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
RE: [RFC] Add watchpoint hit address function to procfs.c
- From: "Pierre Muller" <pierre dot muller at ics-cnrs dot unistra dot fr>
- To: "'Pedro Alves'" <pedro at codesourcery dot com>, <gdb-patches at sourceware dot org>
- Date: Sat, 8 May 2010 18:46:11 +0200
- Subject: RE: [RFC] Add watchpoint hit address function to procfs.c
- References: <006601cae54b$368452f0$a38cf8d0$@muller@ics-cnrs.unistra.fr> <201005071625.18583.pedro@codesourcery.com>
> > +/*
> > + * Function: proc_watchpoint_address
> > + *
> > + * returns the prstatus.pr_lwp.pr_info.__data.__fault.__addr field.
> > + */
> > +
> > +CORE_ADDR
> > +proc_watchpoint_address (procinfo *pi)
> > +{
> > + if (!pi->status_valid)
> > + if (!proc_get_status (pi))
> > + return 0; /* FIXME: not a good failure value (but what
> is?) */
>
> Add a new CORE_ADDR pointer parameter, and use that as
> output. C hange the return type to int to indicate failure/success.
> That is, model the interface of this function from
> to_stopped_data_address's
> interface.
OK, modified as you suggested.
> > +
> > +#ifdef NEW_PROC_API
> > + return unsigned_pointer_to_address (target_gdbarch,
> > + builtin_type (target_gdbarch)->builtin_data_ptr,
> > + (gdb_byte *) &pi-
> >prstatus.pr_lwp.pr_info.__data.__fault.__addr);
> > +#else
> > + return unsigned_pointer_to_address (target_gdbarch,
> > + builtin_type (target_gdbarch)->builtin_data_ptr,
> > + (gdb_byte *) &pi->prstatus.pr_info.__data.__fault.__addr);
> > +#endif
>
> Isn't there a macro to access __data.__fault.__addr ? I'd have
> guessed pi->prstatus.pr_info.sa_addr, similarly to
> how we use pi->prstatus.pr_lwp.pr_info.si_signo already. That
> is, pr_info is a siginfo_t. That would probably get rid of
> the NEW_PROC_API usage and worries.
si_addr is indeed a macro for __data.__fault.__addr
but this doesn't allow to remove the NEW_PROC_API
as there is this extra pr_lwp level for the new api.
> > +}
> > +
> >
> > #ifndef PIOCSSPCACT /* The following is not supported on OSF.
*/
> > /*
> > @@ -5560,6 +5584,45 @@ procfs_stopped_by_watchpoint (void)
> > }
> > return 0;
> > }
> > +/*
> > + * Function procfs_stopped_data_address
> > + * Returns non-zero if we can find the position
> > + * of the triggring watchpoint.
> > + */
> > +
> > +static int
> > +procfs_stopped_data_address (struct target_ops *targ, CORE_ADDR
> *addr)
> > +{
> > + CORE_ADDR waddr;
> > + procinfo *pi;
> > + int watchpoint_hit = 0;
> > +
> > + pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0);
> > +
> > + if (!pi) /* If no process, then not stopped by watchpoint! */
> > + return 0;
>
> Find_procinfo_or_die throws if the procinfo isn't
> found (it's the die part), so this check is dead code.
Then we should simply call find_procinfo, no?
and also in procfs_stopped_by_watchpoint.
> I wonder if this procinfo is the correct one. Shouldn't
> this look for the the procinfo for the current thread (the tid
> in inferior_ptid), not the one for the main thread?
I simply adapted the code from procfs_stopped_by_watchpoint
which also uses (PIDGET (inferior_ptid), 0).
Should those two be converted into
(PIDGET (inferior_ptid), TIDGET (inferior_ptid))?
This is what is done below...
New version of the patch below,
I ran it on a OpenSolaris 2.11 (x86_64 processor)
with RUNTESTFLAGS=gdb*/watch*.exp
It gave 152 PASS for 8 FAIL,
while current CVS gives 127 PASS and 33 FAIL.
Using TIDGET (inferior_ptid) or 0
had no effect on that subset of the testsuite...
Pierre
PS: I noticed a strange thing for gdb.threads/watchthreads
Each thread is reported twice in `info threads':
(gdb) inf thr
[New Thread 2 (LWP 2)]
[New Thread 3 (LWP 3)]
[New Thread 5 (LWP 5)]
11 Thread 5 (LWP 5) 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
10 Thread 3 (LWP 3) 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
9 Thread 2 (LWP 2) 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
8 LWP 6 0xfffffd7fff2d4a28 in _thrp_setup ()
from /lib/64/libc.so.1
* 7 Thread 4 (LWP 4) 0x0000000000401146 in thread_function (arg=0x2)
at ../../../src/gdb/testsuite/gdb.threads/watchthreads.c:75
6 LWP 5 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
5 LWP 4 0x0000000000401146 in thread_function (arg=0x2)
at ../../../src/gdb/testsuite/gdb.threads/watchthreads.c:75
4 LWP 3 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
3 LWP 2 0xfffffd7fff2dcf8a in __nanosleep ()
from /lib/64/libc.so.1
2 Thread 1 (LWP 1) 0xfffffd7fff2de03a in __lwp_create ()
from /lib/64/libc.so.1
1 LWP 1 0xfffffd7fff2de03a in __lwp_create ()
from /lib/64/libc.so.1
Is this expected?
ChangeLog entry unchanged:
2010-05-07 Pierre Muller <muller@ics.u-strasbg.fr>
* procfs.c (proc_watchpoint_address): New function.
(procfs_stopped_data_address): New function.
(procfs_use_watchpoints): Register new watchpoint related
function.
Index: src/gdb/procfs.c
===================================================================
RCS file: /cvs/src/src/gdb/procfs.c,v
retrieving revision 1.131
diff -u -p -r1.131 procfs.c
--- src/gdb/procfs.c 20 Apr 2010 22:36:35 -0000 1.131
+++ src/gdb/procfs.c 7 May 2010 16:01:59 -0000
@@ -1312,6 +1312,33 @@ proc_what (procinfo *pi)
return pi->prstatus.pr_what;
#endif
}
+/*
+ * Function: proc_watchpoint_address
+ *
+ * returns the prstatus.pr_lwp.pr_info.__data.__fault.__addr field in ADDR.
+ *
+ * si_addr is a macro for __data.__fault.__addr.
+ */
+
+static int
+proc_watchpoint_address (procinfo *pi, CORE_ADDR *addr)
+{
+ if (!pi->status_valid)
+ if (!proc_get_status (pi))
+ return 0;
+
+#ifdef NEW_PROC_API
+ *addr = (CORE_ADDR) unsigned_pointer_to_address (target_gdbarch,
+ builtin_type (target_gdbarch)->builtin_data_ptr,
+ (gdb_byte *) &pi->prstatus.pr_lwp.pr_info.si_addr);
+#else
+ *addr = (CORE_ADDR) unsigned_pointer_to_address (target_gdbarch,
+ builtin_type (target_gdbarch)->builtin_data_ptr,
+ (gdb_byte *) &pi->prstatus.pr_info.si_addr);
+#endif
+ return 1;
+}
+
#ifndef PIOCSSPCACT /* The following is not supported on OSF. */
/*
@@ -5539,7 +5566,7 @@ procfs_stopped_by_watchpoint (void)
{
procinfo *pi;
- pi = find_procinfo_or_die (PIDGET (inferior_ptid), 0);
+ pi = find_procinfo (PIDGET (inferior_ptid), TIDGET (inferior_ptid));
if (!pi) /* If no process, then not stopped by watchpoint! */
return 0;
@@ -5560,6 +5587,29 @@ procfs_stopped_by_watchpoint (void)
}
return 0;
}
+/*
+ * Function procfs_stopped_data_address
+ * Returns non-zero if we can find the position
+ * of the triggring watchpoint.
+ */
+
+static int
+procfs_stopped_data_address (struct target_ops *targ, CORE_ADDR *addr)
+{
+ CORE_ADDR waddr = 0;
+ procinfo *pi;
+ int res;
+
+ pi = find_procinfo (PIDGET (inferior_ptid), TIDGET (inferior_ptid));
+
+ if (!pi) /* If no process, then not stopped by watchpoint! */
+ return 0;
+
+ res = proc_watchpoint_address (pi, &waddr);
+ if (addr)
+ *addr = waddr;
+ return res;
+}
static int
procfs_insert_watchpoint (CORE_ADDR addr, int len, int type)
@@ -5607,6 +5657,7 @@ procfs_use_watchpoints (struct target_op
t->to_remove_watchpoint = procfs_remove_watchpoint;
t->to_region_ok_for_hw_watchpoint = procfs_region_ok_for_hw_watchpoint;
t->to_can_use_hw_breakpoint = procfs_can_use_hw_breakpoint;
+ t->to_stopped_data_address = procfs_stopped_data_address;
}
/*