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

[Andrew Cagney <ac131313 at redhat dot com>]

The following reply was made to PR gdb/725; it has been noted by GNATS.

From: Andrew Cagney <ac131313@redhat.com>
To: gdb-gnats@sources.redhat.com
Cc:  
Subject: Re: gdb/725: Crash using debug target and regcaches (in 5.3 branch?)
Date: Thu, 21 Nov 2002 16:16:02 -0500

 "regcache.c" is innocent!
 
 (And the little I just learnt about GDB's thread implementation scares 
 me! :-)
 
 Briefly, the GNU/Linux thread code is giving regcache.c conflicting 
 stories over which inferior ptid should be in the register cache.  As a 
 consequence, every single register fetch leads to a regcache flush and 
 re-fetch.  Outch!
 
 
 Briefly,  core GDB tries to fetch a register.  This eventually leads to 
 the call:
 
 regcache_raw_read(REGNUM)
 
 	registers_tpid != inferior_tpid
 (gdb) print registers_ptid
 $6 = {pid = 31263, lwp = 0, tid = 0}
 (gdb) print inferior_ptid
 $7 = {pid = 31263, lwp = 31263, tid = 0}
 		-> flush regcache
 		-> registers_tpid = inferior_tpid
 	-- at this point regnum is invalid
 	target_fetch_registers (regnum)
 
 Since the inferior doesn't match the target, the cache is flushed, 
 inferior_ptid is updated, and the register is fetched.  The fetch flows 
 on down into the depths of the target and the call:
 
 Seen the problem yet?
 
 lin_lwp_fetch_registers(REGNUM):
 
 1347    lin_lwp_fetch_registers (int regno)
 1348    {
 1349      struct cleanup *old_chain = save_inferior_ptid ();
 1350
 1351      if (is_lwp (inferior_ptid))
 1352        inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid));
 1353
 (gdb)
 1354      fetch_inferior_registers (regno);
 1355
 1356      do_cleanups (old_chain);
 1357    }
 
 Seen the problem yet?
 
 Not sure why, but the code takes the path:
 	inferior_ptid = pid_to_ptid (GET_LWP (inferior_ptid));
 changing:
 (gdb) print inferior_ptid
 $10 = {pid = 31263, lwp = 31263, tid = 0}
 (gdb) n
 1354      fetch_inferior_registers (regno);
 (gdb) print inferior_ptid
 $11 = {pid = 31263, lwp = 0, tid = 0}
 (gdb)
 
 Seen the problem yet?
 
 And then proceeds to extract the registers using ptrace.  The extracted 
 registers are fed to the register cache using:
 
 supply_register(REGNUM, VAL):
 
 1209    supply_register (int regnum, const void *val)
 1210    {
 1211    #if 1
 1212      if (! ptid_equal (registers_ptid, inferior_ptid))
 1213        {
 1214          registers_changed ();
 1215          registers_ptid = inferior_ptid;
 1216        }
 (gdb)
 1217    #endif
 1218
 1219      set_register_cached (regnum, 1);
 
 and the assignment:
 
 1215          registers_ptid = inferior_ptid;
 (gdb) n
 1219      set_register_cached (regnum, 1);
 (gdb) print registers_ptid
 $14 = {pid = 31263, lwp = 0, tid = 0}
 (gdb)
 
 So, registers_ptid is set back to {pid = 31263, lwp = 0, tid = 0} ready 
 to start the entire cycle, again, on that very next register fetch!
 
 
 Two problems:
 
 
 - this means that the current register cache is being rendered useless 
 as it will never hang onto any register value.  Looks very like reason#2 
 for GDB being slow when debugging a threaded application.
 
 
 - in the case of `set debug target 1', the bug causes an infinite 
 recursion (the cause of the crash) because the code:
 1709    static void
 1710    debug_to_fetch_registers (int regno)
 1711    {
 1712      debug_target.to_fetch_registers (regno);
 1713      debug_print_register ("target_fetch_registers", regno);
 1714    }
 is assuming that the debug_target.to_fetch_registers (regno) call 
 results in the the register's value being entered into the cache, and 
 hence a further deprecated_read_register_gen() won't lead to a call to 
 target_fetch_registers()
 
 
 Fix?
 
 The long term fix is to have per-thread register caches, that is 
 progressing.
 
 I don't know about a short term fix though.
 
 Andrew