Re: [Fwd: Re: [Fwd: Re: gdb/725: Crash using debug target and regcaches(in 5.3 branch?)]]

[Andrew Cagney <ac131313 at redhat dot com>]

>   Is there any chance that the attached could be reviewed over the next    few hours?
>
> Apparently not :-(.  Anyway, I've looked at it now and it appears that
> GDB as it stands now isn't ready to cope with the abstraction of the
> lin-lwp layer as I intended it.  So I guess that until we overhaul the
> way GDB deals with threads, Daniels patch (sans the #ifdef) is the way
> to go.
>
> (The US has holidays round this time?)
>
> Thanks!  I'll merge it in, test it and then start the 5.2.91 process.
Er, no I wont :-(

The attached is the refind patch.  I added the comment:

+  /* NOTE: cagney/2002-12-02: This assumes that the target code can
+     directly transfer the register values into the register cache.
+     This works fine when there is a 1:1 mapping between light weight
+     process (LWP) (a.k.a. process on GNU/Linux) and the thread.  On
+     an N:1 (user-land threads), or N:M (combination of user-land and
+     LWP threading), this does not work.  An LWP can be sitting in the
+     thread context switch code and hence, the LWP's registers belong
+     to no thread.  */

however, with the patch applied, I see (and consistently, well 2 out of 
2, which is pretty amasing for the thread testsuite) the new failure:


gdb.threads/killed.exp: GDB exits after multi-threaded program exits messily

looking at the log file:

(gdb) run
Starting program: /home/cagney/gdb/native/gdb/testsuite/gdb.threads/killed
[New Thread 1024 (LWP 6831)]
[New Thread 2049 (LWP 6832)]
[New Thread 1026 (LWP 6833)]
Cannot find user-level thread for LWP 6833: generic error
(gdb) PASS: gdb.threads/killed.exp: run program to completion
quit
The program is running.  Exit anyway? (y or n) y
Cannot find thread 2049: generic error
(gdb) FAIL: gdb.threads/killed.exp: GDB exits after multi-threaded 
program exits
 messily (gdb/568)

Which doesn't occure when the patch isn't applied.

The test system was:
$ uname -a
Linux torrens 2.4.9-13 #1 Tue Oct 30 20:11:04 EST 2001 i686 unknown

I'm instead, for the moment, going to document this as a known problem. 
 (It's a maintainer command so normal users won't use it).

Andrew

2002-12-03  Andrew Cagney  <ac131313@redhat.com>

	* sparc-nat.c (fetch_inferior_registers)
	(store_inferior_registers): Add comment on problem of LWP vs
	threads.
	
	From 2002-11-21 Daniel Jacobowitz <drow@mvista.com>
	* lin-lwp.c (lin_lwp_fetch_registers): Remove.
	(lin_lwp_store_registers): Remove.
	(init_lin_lwp_ops): Use fetch_inferior_registers
	and store_inferior_registers directly.
	* sparc-nat.c (fetch_inferior_registers): Honor LWP ID.
	(store_inferior_registers): Likewise.
	Fix PR gdb/725

Index: lin-lwp.c
===================================================================
RCS file: /cvs/src/src/gdb/lin-lwp.c,v
retrieving revision 1.35.2.1
diff -u -r1.35.2.1 lin-lwp.c
--- lin-lwp.c	26 Nov 2002 01:32:21 -0000	1.35.2.1
+++ lin-lwp.c	3 Dec 2002 16:39:30 -0000
@@ -1346,32 +1346,6 @@
   child_ops.to_mourn_inferior ();
 }
 
-static void
-lin_lwp_fetch_registers (int regno)
-{
-  struct cleanup *old_chain = save_inferior_ptid ();
-
-  if (is_lwp (inferior_ptid))
-    inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid));
-
-  fetch_inferior_registers (regno);
-
-  do_cleanups (old_chain);
-}
-
-static void
-lin_lwp_store_registers (int regno)
-{
-  struct cleanup *old_chain = save_inferior_ptid ();
-
-  if (is_lwp (inferior_ptid))
-    inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid));
-
-  store_inferior_registers (regno);
-
-  do_cleanups (old_chain);
-}
-
 static int
 lin_lwp_xfer_memory (CORE_ADDR memaddr, char *myaddr, int len, int write,
 		     struct mem_attrib *attrib,
@@ -1431,8 +1405,10 @@
   lin_lwp_ops.to_detach = lin_lwp_detach;
   lin_lwp_ops.to_resume = lin_lwp_resume;
   lin_lwp_ops.to_wait = lin_lwp_wait;
-  lin_lwp_ops.to_fetch_registers = lin_lwp_fetch_registers;
-  lin_lwp_ops.to_store_registers = lin_lwp_store_registers;
+  /* fetch_inferior_registers and store_inferior_registers will
+     honor the LWP id, so we can use them directly.  */
+  lin_lwp_ops.to_fetch_registers = fetch_inferior_registers;
+  lin_lwp_ops.to_store_registers = store_inferior_registers;
   lin_lwp_ops.to_xfer_memory = lin_lwp_xfer_memory;
   lin_lwp_ops.to_kill = lin_lwp_kill;
   lin_lwp_ops.to_create_inferior = lin_lwp_create_inferior;
Index: sparc-nat.c
===================================================================
RCS file: /cvs/src/src/gdb/sparc-nat.c,v
retrieving revision 1.13
diff -u -r1.13 sparc-nat.c
--- sparc-nat.c	21 Apr 2002 05:34:06 -0000	1.13
+++ sparc-nat.c	3 Dec 2002 16:39:30 -0000
@@ -1,5 +1,6 @@
 /* Functions specific to running gdb native on a SPARC running SunOS4.
-   Copyright 1989, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000, 2001
+   Copyright 1989, 1992, 1993, 1994, 1996, 1997, 1998, 1999, 2000, 2001,
+   2002
    Free Software Foundation, Inc.
 
    This file is part of GDB.
@@ -58,6 +59,19 @@
   struct regs inferior_registers;
   struct fp_status inferior_fp_registers;
   int i;
+  int fetch_pid;
+
+  /* NOTE: cagney/2002-12-02: This assumes that the target code can
+     directly transfer the register values into the register cache.
+     This works fine when there is a 1:1 mapping between light weight
+     process (LWP) (a.k.a. process on GNU/Linux) and the thread.  On
+     an N:1 (user-land threads), or N:M (combination of user-land and
+     LWP threading), this does not work.  An LWP can be sitting in the
+     thread context switch code and hence, the LWP's registers belong
+     to no thread.  */
+  fetch_pid = TIDGET (inferior_ptid);
+  if (fetch_pid == 0)
+    fetch_pid = PIDGET (inferior_ptid);
 
   /* We should never be called with deferred stores, because a prerequisite
      for writing regs is to have fetched them all (PREPARE_TO_STORE), sigh.  */
@@ -75,7 +89,7 @@
       || regno >= Y_REGNUM
       || (!register_valid[SP_REGNUM] && regno < I7_REGNUM))
     {
-      if (0 != ptrace (PTRACE_GETREGS, PIDGET (inferior_ptid),
+      if (0 != ptrace (PTRACE_GETREGS, fetch_pid,
 		       (PTRACE_ARG3_TYPE) & inferior_registers, 0))
 	perror ("ptrace_getregs");
 
@@ -105,7 +119,7 @@
       regno == FPS_REGNUM ||
       (regno >= FP0_REGNUM && regno <= FP0_REGNUM + 31))
     {
-      if (0 != ptrace (PTRACE_GETFPREGS, PIDGET (inferior_ptid),
+      if (0 != ptrace (PTRACE_GETFPREGS, fetch_pid,
 		       (PTRACE_ARG3_TYPE) & inferior_fp_registers,
 		       0))
 	perror ("ptrace_getfpregs");
@@ -151,6 +165,19 @@
   struct regs inferior_registers;
   struct fp_status inferior_fp_registers;
   int wanna_store = INT_REGS + STACK_REGS + FP_REGS;
+  int store_pid;
+
+  /* NOTE: cagney/2002-12-02: This assumes that the target code can
+     directly transfer the register values into the register cache.
+     This works fine when there is a 1:1 mapping between light weight
+     process (LWP) (a.k.a. process on GNU/Linux) and the thread.  On
+     an N:1 (user-land threads), or N:M (combination of user-land and
+     LWP threading), this does not work.  An LWP can be sitting in the
+     thread context switch code and hence, the LWP's registers belong
+     to no thread.  */
+  store_pid = TIDGET (inferior_ptid);
+  if (store_pid == 0)
+    store_pid = PIDGET (inferior_ptid);
 
   /* First decide which pieces of machine-state we need to modify.  
      Default for regno == -1 case is all pieces.  */
@@ -233,7 +260,7 @@
       inferior_registers.r_y =
 	*(int *) &registers[REGISTER_BYTE (Y_REGNUM)];
 
-      if (0 != ptrace (PTRACE_SETREGS, PIDGET (inferior_ptid),
+      if (0 != ptrace (PTRACE_SETREGS, store_pid,
 		       (PTRACE_ARG3_TYPE) & inferior_registers, 0))
 	perror ("ptrace_setregs");
     }
@@ -247,7 +274,7 @@
       memcpy (&inferior_fp_registers.Fpu_fsr,
 	      &registers[REGISTER_BYTE (FPS_REGNUM)], sizeof (FPU_FSR_TYPE));
       if (0 !=
-	  ptrace (PTRACE_SETFPREGS, PIDGET (inferior_ptid),
+	  ptrace (PTRACE_SETFPREGS, store_pid,
 		  (PTRACE_ARG3_TYPE) & inferior_fp_registers, 0))
 	perror ("ptrace_setfpregs");
     }