This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC]: x86 threaded watchpoint support [1/3]


Daniel Jacobowitz wrote:
On Sat, Jun 12, 2004 at 12:00:19AM +0200, Mark Kettenis wrote:

  Date: Fri, 11 Jun 2004 17:14:24 -0400
  From: Jeff Johnston <jjohnstn@redhat.com>

  The following patch gets threaded watchpoint support working for
  the x86.  On x86 linux, the dr_status register is thread-specific.
  This means that the current method which uses the PID to call
  PTRACE is wrong.  I have changed this to use the current lwp for
  the inferior_ptid.  Corresponding to this, the
  i386_stopped_data_address function switches the inferior_ptid to
  the trap_ptid.  Thus, we can see if we really stopped for a
  watchpoint or hardware breakpoint.

I'm not surprised that the current stuff is wrong.  However, have you
verified that the dr_status register is thread-specific for *all*
versions of GNU/Linux that we support and not just the RedHat kernel
that you're working with?


I'm pretty sure - GNU/Linux has never had a concept of "process
registers", since threads have evolved from processes rather than the
other way around.



I'm glad to see everyone responding.


I did not verify that DR6 is thread-specific for *all* versions of GNU/Linux. I simply made the observation and confirmed it by asking a Red Hat kernel developer. I can get the developer to verify that his response was meant for all GNU/Linux. However, a better question might be: does anyone know of any version of GNU/Linux where the current x86 code works for threads?

My observation is that the rest of the debug registers are global. My example sets the watch addresses and DR7 from the main thread and this works across the new threads. I also am able to set a new watchpoint from a thread that breaks in the main thread, etc... Thus, I only had to modify the DR6 code rather than all the debug registers.

  Because the thread-db.c code changes the trap_ptid into a
  high-level ptid (pid + tid), I had to add a new target vector
  interface which gives back the lwp for a given ptid.  This is used
  by the low level dr get routine.

Really? Doesn't TIDGET work for you?


TIDGET at this point is the thread ID, i.e. internal state from NPTL or
LinuxThreads.  It's not the LWP id that we need in order to all ptrace.

It sounds like this new vector is really the death blow to thread-db.c.
Maybe I should see how much of it we can throw away.  Copious free time
and all that.


Could you provide any details? Is there anything I can do interim?



  Index: lin-lwp.c
  ===================================================================
  RCS file: /cvs/src/src/gdb/lin-lwp.c,v
  retrieving revision 1.55
  diff -u -p -r1.55 lin-lwp.c
  --- lin-lwp.c	25 May 2004 14:58:28 -0000	1.55
  +++ lin-lwp.c	11 Jun 2004 20:55:28 -0000
  @@ -1200,21 +1200,29 @@ child_wait (ptid_t ptid, struct target_w
	   }

	  /* Handle GNU/Linux's extended waitstatus for trace events.  */
  -      if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP
  -	  && status >> 16 != 0)
  +      if (pid != -1 && WIFSTOPPED (status) && WSTOPSIG (status) == SIGTRAP)
	   {
  -	  linux_handle_extended_wait (pid, status, ourstatus);
  +	  /* Set trap_ptid like lin_lwp_wait does.  This is needed
  +	     for watchpoint support.  For example, the x86 linux
  +	     watchpoints need to know what thread an event occurred
  +	     on so as to read the correct thread-specific status.  */
  +          trap_ptid = pid_to_ptid (pid);

  -	  /* If we see a clone event, detach the child, and don't
  -	     report the event.  It would be nice to offer some way to
  -	     switch into a non-thread-db based threaded mode at this
  -	     point.  */
  -	  if (ourstatus->kind == TARGET_WAITKIND_SPURIOUS)
  +	  if (status >> 16 != 0)

What's this shift supposed to do?


This one's my fault so I'll answer for him: it was already there, up a
couple lines in the diff.  It's for the extended waitstatus support.

I must say that I'm not thrilled by having trap_ptid leak out into yet
more files.  It's an extremely underspecified interface.


Is the right way to first ensure that gdb switches to the trap thread so that we simply get the DR6 for the then-current thread?


-- Jeff J.



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]