[PATCH] btrace: avoid tp != NULL assertion

[PATCH] btrace: avoid tp != NULL assertion

[Markus Metzger]

On some targets, I see the LWP field patched out of INFERIOR_PTID before
calling record_btrace_fetch_registers.  Looking to the respective linux-nat
versions, they use the PID field if the LWP field is zero.

Implement a similar behavior for record-btrace.

2015-02-09  Markus Metzger  <markus.t.metzger@intel.com>

gdb/
	* record-btrace.c (record_btrace_inferior_thread): New.
	(require_btrace_thread, record_btrace_info):
	(record_btrace_tailcall_frame_sniffer): Call
	record_btrace_inferior_thread.
	(record_btrace_fetch_registers, record_btrace_frame_sniffer):
	Call record_btrace_inferior_thread.  Replace tp != NULL assertion
	with error call.
---
 gdb/record-btrace.c | 45 ++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 38 insertions(+), 7 deletions(-)

diff --git a/gdb/record-btrace.c b/gdb/record-btrace.c
index 102e0eb..b7dedd9 100644
--- a/gdb/record-btrace.c
+++ b/gdb/record-btrace.c
@@ -97,6 +97,35 @@ static struct cmd_list_element *show_record_btrace_bts_cmdlist;
   while (0)
 
 
+/* Return a pointer to the thread_info of inferior_ptid.
+
+   In some cases, we see the LWP field of INFERIOR_PTID patched out.  Use the
+   PID as LWP similar to what i386|amd64_linux_fetch_inferior_registers do.  */
+
+static struct thread_info *
+record_btrace_inferior_thread (void)
+{
+  ptid_t ptid;
+  long lwp;
+
+  DEBUG ("inferior thread");
+
+  ptid = inferior_ptid;
+
+  /* GNU/Linux LWP ID's are process ID's.  */
+  lwp = ptid_get_lwp (ptid);
+  if (lwp == 0)
+    {
+      int pid;
+
+      /* Not a threaded program.  Use the PID as LWP ID.  */
+      pid = ptid_get_pid (ptid);
+      ptid = ptid_build (pid, pid, 0);
+    }
+
+  return find_thread_ptid (ptid);
+}
+
 /* Update the branch trace for the current thread and return a pointer to its
    thread_info.
 
@@ -110,7 +139,7 @@ require_btrace_thread (void)
 
   DEBUG ("require");
 
-  tp = find_thread_ptid (inferior_ptid);
+  tp = record_btrace_inferior_thread ();
   if (tp == NULL)
     error (_("No thread."));
 
@@ -370,7 +399,7 @@ record_btrace_info (struct target_ops *self)
 
   DEBUG ("info");
 
-  tp = find_thread_ptid (inferior_ptid);
+  tp = record_btrace_inferior_thread ();
   if (tp == NULL)
     error (_("No thread."));
 
@@ -1163,8 +1192,9 @@ record_btrace_fetch_registers (struct target_ops *ops,
   struct btrace_insn_iterator *replay;
   struct thread_info *tp;
 
-  tp = find_thread_ptid (inferior_ptid);
-  gdb_assert (tp != NULL);
+  tp = record_btrace_inferior_thread ();
+  if (tp == NULL)
+    error (_("No thread."));
 
   replay = tp->btrace.replay;
   if (replay != NULL && !record_btrace_generating_corefile)
@@ -1414,8 +1444,9 @@ record_btrace_frame_sniffer (const struct frame_unwind *self,
   struct frame_info *next;
 
   /* THIS_FRAME does not contain a reference to its thread.  */
-  tp = find_thread_ptid (inferior_ptid);
-  gdb_assert (tp != NULL);
+  tp = record_btrace_inferior_thread ();
+  if (tp == NULL)
+    error (_("No thread."));
 
   bfun = NULL;
   next = get_next_frame (this_frame);
@@ -1482,7 +1513,7 @@ record_btrace_tailcall_frame_sniffer (const struct frame_unwind *self,
 
   /* This is our frame.  Initialize the frame cache.  */
   cache = bfcache_new (this_frame);
-  cache->tp = find_thread_ptid (inferior_ptid);
+  cache->tp = record_btrace_inferior_thread ();
   cache->bfun = bfun;
 
   *this_cache = cache;
-- 
1.8.3.1

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 02/09/2015 09:25 AM, Markus Metzger wrote:
> On some targets, I see the LWP field patched out of INFERIOR_PTID before
> calling record_btrace_fetch_registers.  Looking to the respective linux-nat
> versions, they use the PID field if the LWP field is zero.

Sorry, I'm not sold on this one.  Please give more detail.

Which targets?  What's the backtrace like?

Thanks,
Pedro Alves

RE: [PATCH] btrace: avoid tp != NULL assertion

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Monday, March 2, 2015 11:10 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion
> 
> On 02/09/2015 09:25 AM, Markus Metzger wrote:
> > On some targets, I see the LWP field patched out of INFERIOR_PTID before
> > calling record_btrace_fetch_registers.  Looking to the respective linux-nat
> > versions, they use the PID field if the LWP field is zero.
> 
> Sorry, I'm not sold on this one.  Please give more detail.
> 
> Which targets?  What's the backtrace like?

I saw this on a 32-bit Fedora 20 running on i5-4250U.

When I debug it using a conditional breakpoint on the assert, I get:

(gdb) rec b
(gdb) n

Breakpoint 1, record_btrace_fetch_registers (ops=0x974bfc0 <record_btrace_ops>, 
    regcache=0x9a0a798, regno=8) at gdb/record-btrace.c:1202
1202	  gdb_assert (tp != NULL);
(gdb) bt
#0  record_btrace_fetch_registers (ops=0x974bfc0 <record_btrace_ops>, regcache=0x9a0a798, regno=8)
    at gdb/record-btrace.c:1202
#1  0x083f4ee2 in delegate_fetch_registers (self=0x974bfc0 <record_btrace_ops>, arg1=0x9a0a798, 
    arg2=8) at gdb/target-delegates.c:149
#2  0x08406562 in target_fetch_registers (regcache=0x9a0a798, regno=8)
    at gdb/target.c:3279
#3  0x08355255 in regcache_raw_read (regcache=0x9a0a798, regnum=8, 
    buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]")
    at gdb/regcache.c:643
....
(gdb) up 3
#3  0x08355255 in regcache_raw_read (regcache=0x9a0a798, regnum=8, 
    buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]") at gdb/regcache.c:643
643	      target_fetch_registers (regcache, regnum);
(gdb) l
638	      && regcache_register_status (regcache, regnum) == REG_UNKNOWN)
639	    {
640	      struct cleanup *old_chain = save_inferior_ptid ();
641	
642	      inferior_ptid = regcache->ptid;
643	      target_fetch_registers (regcache, regnum);
644	      do_cleanups (old_chain);
645	
646	      /* A number of targets can't access the whole set of raw
647		 registers (because the debug API provides no means to get at
(gdb) p regcache->ptid
$3 = {pid = 23856, lwp = 0, tid = 0}
(gdb) p *cleanup_chain
$12 = {next = 0x9a25820, function = 0x83b9ebd <restore_inferior_ptid>, free_arg = 0x0, 
  arg = 0x9a76088}
(gdb) p *(ptid_t *)cleanup_chain->arg
$13 = {pid = 23856, lwp = 23856, tid = 0}
(gdb)

I have no idea how regcache->ptid ended up with lwp=0.

Here's the full backtrace in case it helps.

(gdb) bt
#0  record_btrace_fetch_registers (ops=0x974bfc0 <record_btrace_ops>, regcache=0x9a0a798, regno=8)
    at gdb/record-btrace.c:1202
#1  0x083f4ee2 in delegate_fetch_registers (self=0x974bfc0 <record_btrace_ops>, arg1=0x9a0a798, 
    arg2=8) at gdb/target-delegates.c:149
#2  0x08406562 in target_fetch_registers (regcache=0x9a0a798, regno=8)
    at gdb/target.c:3279
#3  0x08355255 in regcache_raw_read (regcache=0x9a0a798, regnum=8, 
    buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]")
    at gdb/regcache.c:643
#4  0x083558a7 in regcache_cooked_read (regcache=0x9a0a798, regnum=8, 
    buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]")
    at gdb/regcache.c:734
#5  0x08355de3 in regcache_cooked_read_unsigned (regcache=0x9a0a798, regnum=8, val=0xbfffe738)
    at gdb/regcache.c:838
#6  0x0827a106 in i386_linux_resume (ops=0x9737ca0 <linux_ops_saved>, ptid=..., step=1, 
    signal=GDB_SIGNAL_0) at gdb/i386-linux-nat.c:670
#7  0x08280c12 in linux_resume_one_lwp (lp=0x9a0a5b8, step=1, signo=GDB_SIGNAL_0)
    at gdb/linux-nat.c:1529
#8  0x08281281 in linux_nat_resume (ops=0x98da608, ptid=..., step=1, signo=GDB_SIGNAL_0)
    at gdb/linux-nat.c:1708
#9  0x0850738e in record_btrace_resume (ops=0x98da608, ptid=..., step=1, signal=GDB_SIGNAL_0)
    at gdb/record-btrace.c:1760
#10 0x083f4b3d in delegate_resume (self=0x974bfc0 <record_btrace_ops>, arg1=..., arg2=1, 
    arg3=GDB_SIGNAL_0) at gdb/target-delegates.c:87
#11 0x08404b79 in target_resume (ptid=..., step=1, signal=GDB_SIGNAL_0)
    at gdb/target.c:2231
#12 0x083b0ecd in resume (step=1, sig=GDB_SIGNAL_0)
    at gdb/infrun.c:2387
#13 0x083b16fe in proceed (addr=18446744073709551615, siggnal=GDB_SIGNAL_DEFAULT, step=1)
    at gdb/infrun.c:2731
#14 0x083a9d84 in step_once (skip_subroutines=1, single_inst=0, count=1, thread=1)
    at gdb/infcmd.c:1121
#15 0x083a9a2f in step_1 (skip_subroutines=1, single_inst=0, count_string=0x0)
    at gdb/infcmd.c:977
#16 0x083a97a0 in next_command (count_string=0x0, from_tty=1)
    at gdb/infcmd.c:885
#17 0x082bda38 in do_cfunc (c=0x990a3e8, args=0x0, from_tty=1)
    at gdb/cli/cli-decode.c:105
#18 0x082c03a1 in cmd_func (cmd=0x990a3e8, args=0x0, from_tty=1)
    at gdb/cli/cli-decode.c:1893
#19 0x084c8768 in execute_command (p=0x9772ce1 "", from_tty=1)
    at gdb/top.c:476
#20 0x083d1f80 in command_handler (command=0x9772ce0 "n")
    at gdb/event-top.c:494
#21 0x083d249b in command_line_handler (rl=0x9a25ab8 "n")
    at gdb/event-top.c:692
#22 0x08528ae7 in rl_callback_read_char () at readline/callback.c:220
#23 0x083d1aa7 in rl_callback_read_char_wrapper (client_data=0x0)
    at gdb/event-top.c:171
#24 0x083d1ea3 in stdin_event_handler (error=0, client_data=0x0)
    at gdb/event-top.c:432
#25 0x083d0ff8 in handle_file_event (file_ptr=0x9a25638, ready_mask=1)
    at gdb/event-loop.c:657
#26 0x083d127b in gdb_wait_for_event (block=1) at gdb/event-loop.c:772
#27 0x083d0726 in gdb_do_one_event () at gdb/event-loop.c:309
#28 0x083d0777 in start_event_loop () at gdb/event-loop.c:334
#29 0x083d1ad0 in cli_command_loop (data=0x0) at gdb/event-top.c:186
#30 0x083c95a3 in current_interp_command_loop () at gdb/interps.c:317
#31 0x083ca72b in captured_command_loop (data=0x0) at gdb/main.c:321
#32 0x083c698f in catch_errors (func=0x83ca716 <captured_command_loop>, func_args=0x0, 
    errstring=0x8c65769 "", mask=RETURN_MASK_ALL)
    at gdb/exceptions.c:235
#33 0x083cba17 in captured_main (data=0xbfffef54) at gdb/main.c:1148
#34 0x083c698f in catch_errors (func=0x83caac1 <captured_main>, func_args=0xbfffef54, 
    errstring=0x8c65769 "", mask=RETURN_MASK_ALL)
    at gdb/exceptions.c:235
#35 0x083cba42 in gdb_main (args=0xbfffef54) at gdb/main.c:1156
#36 0x080c7097 in main (argc=2, argv=0xbffff004) at gdb/gdb.c:32
(gdb)

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 03/03/2015 10:49 AM, Metzger, Markus T wrote:

> I have no idea how regcache->ptid ended up with lwp=0.
> 
> Here's the full backtrace in case it helps.

Yes it does.


> #6  0x0827a106 in i386_linux_resume (ops=0x9737ca0 <linux_ops_saved>, ptid=..., step=1, 
>     signal=GDB_SIGNAL_0) at gdb/i386-linux-nat.c:670
> #7  0x08280c12 in linux_resume_one_lwp (lp=0x9a0a5b8, step=1, signo=GDB_SIGNAL_0)
>     at gdb/linux-nat.c:1529

Frame #7 where we convert the ptid to something inf-ptrace.c understands.

The fix is just to stop losing information.  This also improves
performance a tiny bit, as currently we fetch registers out of ptrace
twice, once for {pid,lwp} in linux-nat.c, and another for {pid,0} in
inf-ptrace/i386-linux-nat.c.  We'll now use a single regcache object
everywhere.

Please try the patch below.

Note that this in your patch:

+  lwp = ptid_get_lwp (ptid);
+  if (lwp == 0)
+    {
+      int pid;
+
+      /* Not a threaded program.  Use the PID as LWP ID.  */
+      pid = ptid_get_pid (ptid);
+      ptid = ptid_build (pid, pid, 0);
+    }
+
+  return find_thread_ptid (ptid);
+}
+

is wrong because when debugging a multi-threaded program, you'll
have e.g. ({pid,lwp}) {1,1}, {1,2}, {1,3}, etc.  Your patch makes
btrace look for {2,2}, {3,3}, etc. in the thread list, which just
aren't there, and thus return NULL.  I assume that that's why you
needed to downgrade the asserts to errors.


----
From 6f4e842f2cfefa18d789d2b658e2719fa5609e60 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 3 Mar 2015 11:03:34 +0000
Subject: [PATCH] No longer hack ptids when passing the request down to the
 inf-ptrace layer

---
 gdb/i386-linux-nat.c |  5 ++---
 gdb/inf-ptrace.c     | 25 ++++++++++++++++++++++---
 gdb/linux-nat.c      |  6 +-----
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index b95b47e..8cb8c66 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -648,8 +648,7 @@ static void
 i386_linux_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
-  int pid = ptid_get_pid (ptid);
-
+  int pid = ptid_get_lwp (ptid);
   int request;
 
   if (catch_syscall_enabled () > 0)
@@ -659,7 +658,7 @@ i386_linux_resume (struct target_ops *ops,
 
   if (step)
     {
-      struct regcache *regcache = get_thread_regcache (pid_to_ptid (pid));
+      struct regcache *regcache = get_thread_regcache (ptid);
       struct gdbarch *gdbarch = get_regcache_arch (regcache);
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
       ULONGEST pc;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 4c22a84..606bdb4 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -289,6 +289,22 @@ inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
   kill (-inferior_process_group (), SIGINT);
 }
 
+/* Return which PID to pass to ptrace in order to observe/control the
+   tracee identified by PTID.  */
+
+static pid_t
+get_ptrace_pid (ptid_t ptid)
+{
+  pid_t pid;
+
+  /* If we have an LWPID to work with, use it.  Otherwise, we're
+     dealing with a non-threaded program/target.  */
+  pid = ptid_get_lwp (ptid);
+  if (pid == 0)
+    pid = ptid_get_pid (ptid);
+  return pid;
+}
+
 /* Resume execution of thread PTID, or all threads if PTID is -1.  If
    STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
    that signal.  */
@@ -297,13 +313,16 @@ static void
 inf_ptrace_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
-  pid_t pid = ptid_get_pid (ptid);
+  pid_t pid;
+
   int request;
 
-  if (pid == -1)
+  if (ptid_equal (minus_one_ptid, ptid))
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
-    pid = ptid_get_pid (inferior_ptid);
+    pid = get_ptrace_pid (inferior_ptid);
+  else
+    pid = get_ptrace_pid (ptid);
 
   if (catch_syscall_enabled () > 0)
     request = PT_SYSCALL;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 2e1133d..cb10e2c 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1506,8 +1506,6 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
 static void
 linux_resume_one_lwp (struct lwp_info *lp, int step, enum gdb_signal signo)
 {
-  ptid_t ptid;
-
   lp->step = step;
 
   /* stop_pc doubles as the PC the LWP had when it was last resumed.
@@ -1524,9 +1522,7 @@ linux_resume_one_lwp (struct lwp_info *lp, int step, enum gdb_signal signo)
 
   if (linux_nat_prepare_to_resume != NULL)
     linux_nat_prepare_to_resume (lp);
-  /* Convert to something the lower layer understands.  */
-  ptid = pid_to_ptid (ptid_get_lwp (lp->ptid));
-  linux_ops->to_resume (linux_ops, ptid, step, signo);
+  linux_ops->to_resume (linux_ops, lp->ptid, step, signo);
   lp->stop_reason = LWP_STOPPED_BY_NO_REASON;
   lp->stopped = 0;
   registers_changed_ptid (lp->ptid);
-- 
1.9.3

RE: [PATCH] btrace: avoid tp != NULL assertion

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, March 3, 2015 12:55 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion


> Frame #7 where we convert the ptid to something inf-ptrace.c understands.
> 
> The fix is just to stop losing information.  This also improves
> performance a tiny bit, as currently we fetch registers out of ptrace
> twice, once for {pid,lwp} in linux-nat.c, and another for {pid,0} in
> inf-ptrace/i386-linux-nat.c.  We'll now use a single regcache object
> everywhere.
> 
> Please try the patch below.

It works.  Thanks.  I'll drop my patch, then.


> Note that this in your patch:
> 
> +  lwp = ptid_get_lwp (ptid);
> +  if (lwp == 0)
> +    {
> +      int pid;
> +
> +      /* Not a threaded program.  Use the PID as LWP ID.  */
> +      pid = ptid_get_pid (ptid);
> +      ptid = ptid_build (pid, pid, 0);
> +    }
> +
> +  return find_thread_ptid (ptid);
> +}
> +
> 
> is wrong because when debugging a multi-threaded program, you'll
> have e.g. ({pid,lwp}) {1,1}, {1,2}, {1,3}, etc.  Your patch makes
> btrace look for {2,2}, {3,3}, etc. in the thread list, which just
> aren't there, and thus return NULL.  I assume that that's why you
> needed to downgrade the asserts to errors.

No, that wasn't the reason for replacing the assert.  There are no such
errors in the gdb.btrace suite (which is mostly single-threaded) with my
patch and I have not seen any such errors otherwise, either.  I just found
it safer and more user-friendly in case we run into further problems in
this area.

I see what you mean, though...

> -  /* Convert to something the lower layer understands.  */
> -  ptid = pid_to_ptid (ptid_get_lwp (lp->ptid));
> -  linux_ops->to_resume (linux_ops, ptid, step, signo);


Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 03/03/2015 12:25 PM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Tuesday, March 3, 2015 12:55 PM
>> To: Metzger, Markus T
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion
> 
> 
>> Frame #7 where we convert the ptid to something inf-ptrace.c understands.
>>
>> The fix is just to stop losing information.  This also improves
>> performance a tiny bit, as currently we fetch registers out of ptrace
>> twice, once for {pid,lwp} in linux-nat.c, and another for {pid,0} in
>> inf-ptrace/i386-linux-nat.c.  We'll now use a single regcache object
>> everywhere.
>>
>> Please try the patch below.
> 
> It works.  Thanks.  I'll drop my patch, then.

Alright, I've applied it now, as below.

> No, that wasn't the reason for replacing the assert.  There are no such
> errors in the gdb.btrace suite (which is mostly single-threaded) with my
> patch and I have not seen any such errors otherwise, either.  

Then it sounds like we're either lacking basic tests, or the threaded tests
are somehow not running correctly when gdb is a 32-bit program.  I think
that if you step any non-leader thread, you should see it happen.
Grepping the tests, I think gdb.btrace/multi-thread-step.exp should have
caught it.  My machine doesn't do btrace, so I can't try it myself...

BTW, did any existing test in the testsuite catch the assertion we're
fixing?

--------
From 90ad5e1d4f34d02f437ec12d1b65d7252f5b7f1c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Tue, 3 Mar 2015 13:33:44 +0000
Subject: [PATCH] Linux/ptrace: don't convert ptids when asking inf-ptrace
 layer to resume LWP
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Ref: https://sourceware.org/ml/gdb-patches/2015-03/msg00060.html

The record-btrace target can hit an assertion here:

 Breakpoint 1, record_btrace_fetch_registers (ops=0x974bfc0 <record_btrace_ops>,
     regcache=0x9a0a798, regno=8) at gdb/record-btrace.c:1202
 1202	  gdb_assert (tp != NULL);

 (gdb) p regcache->ptid
 $3 = {pid = 23856, lwp = 0, tid = 0}

The problem is that the linux-nat layer converts the ptid to a
single-process ptid before passing the request down to the inf-ptrace
layer, which loses information, and then record-btrace can't find the
corresponding thread in GDB's thread list:

 (gdb) bt
 #0  record_btrace_fetch_registers (ops=0x974bfc0 <record_btrace_ops>, regcache=0x9a0a798, regno=8)
     at gdb/record-btrace.c:1202
 #1  0x083f4ee2 in delegate_fetch_registers (self=0x974bfc0 <record_btrace_ops>, arg1=0x9a0a798,
     arg2=8) at gdb/target-delegates.c:149
 #2  0x08406562 in target_fetch_registers (regcache=0x9a0a798, regno=8)
     at gdb/target.c:3279
 #3  0x08355255 in regcache_raw_read (regcache=0x9a0a798, regnum=8,
     buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]")
     at gdb/regcache.c:643
 #4  0x083558a7 in regcache_cooked_read (regcache=0x9a0a798, regnum=8,
     buf=0xbfffe6c0 "¨\003\222\tÀ8kIøæÿ¿HO5\b\035]")
     at gdb/regcache.c:734
 #5  0x08355de3 in regcache_cooked_read_unsigned (regcache=0x9a0a798, regnum=8, val=0xbfffe738)
     at gdb/regcache.c:838
 #6  0x0827a106 in i386_linux_resume (ops=0x9737ca0 <linux_ops_saved>, ptid=..., step=1,
     signal=GDB_SIGNAL_0) at gdb/i386-linux-nat.c:670
 #7  0x08280c12 in linux_resume_one_lwp (lp=0x9a0a5b8, step=1, signo=GDB_SIGNAL_0)
     at gdb/linux-nat.c:1529
 #8  0x08281281 in linux_nat_resume (ops=0x98da608, ptid=..., step=1, signo=GDB_SIGNAL_0)
     at gdb/linux-nat.c:1708
 #9  0x0850738e in record_btrace_resume (ops=0x98da608, ptid=..., step=1, signal=GDB_SIGNAL_0)
     at gdb/record-btrace.c:1760
 ...

The fix is just to not lose information, and let the intact ptid reach
record-btrace.c.

Tested on x86-64 Fedora 20, -m32.

gdb/ChangeLog:
2015-03-03  Pedro Alves  <palves@redhat.com>

	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
	* inf-ptrace.c (get_ptrace_pid): New function.
	(inf_ptrace_resume): Use it.
	* linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
	to the lower layer.
---
 gdb/ChangeLog        |  9 +++++++++
 gdb/i386-linux-nat.c |  5 ++---
 gdb/inf-ptrace.c     | 25 ++++++++++++++++++++++---
 gdb/linux-nat.c      |  6 +-----
 4 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1db98ff..31a672e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,12 @@
+2015-03-03  Pedro Alves  <palves@redhat.com>
+
+	* i386-linux-nat.c (i386_linux_resume): Get the ptrace PID out of
+	the lwp field of ptid.  Pass the full ptid to get_thread_regcache.
+	* inf-ptrace.c (get_ptrace_pid): New function.
+	(inf_ptrace_resume): Use it.
+	* linux-nat.c (linux_resume_one_lwp): Pass the LWP's ptid ummodified
+	to the lower layer.
+
 2015-03-03  Markus Metzger  <markus.t.metzger@intel.com>
 
 	* nat/linux-btrace.c: Include sys/utsname.h.
diff --git a/gdb/i386-linux-nat.c b/gdb/i386-linux-nat.c
index b95b47e..8cb8c66 100644
--- a/gdb/i386-linux-nat.c
+++ b/gdb/i386-linux-nat.c
@@ -648,8 +648,7 @@ static void
 i386_linux_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
-  int pid = ptid_get_pid (ptid);
-
+  int pid = ptid_get_lwp (ptid);
   int request;
 
   if (catch_syscall_enabled () > 0)
@@ -659,7 +658,7 @@ i386_linux_resume (struct target_ops *ops,
 
   if (step)
     {
-      struct regcache *regcache = get_thread_regcache (pid_to_ptid (pid));
+      struct regcache *regcache = get_thread_regcache (ptid);
       struct gdbarch *gdbarch = get_regcache_arch (regcache);
       enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
       ULONGEST pc;
diff --git a/gdb/inf-ptrace.c b/gdb/inf-ptrace.c
index 4c22a84..606bdb4 100644
--- a/gdb/inf-ptrace.c
+++ b/gdb/inf-ptrace.c
@@ -289,6 +289,22 @@ inf_ptrace_stop (struct target_ops *self, ptid_t ptid)
   kill (-inferior_process_group (), SIGINT);
 }
 
+/* Return which PID to pass to ptrace in order to observe/control the
+   tracee identified by PTID.  */
+
+static pid_t
+get_ptrace_pid (ptid_t ptid)
+{
+  pid_t pid;
+
+  /* If we have an LWPID to work with, use it.  Otherwise, we're
+     dealing with a non-threaded program/target.  */
+  pid = ptid_get_lwp (ptid);
+  if (pid == 0)
+    pid = ptid_get_pid (ptid);
+  return pid;
+}
+
 /* Resume execution of thread PTID, or all threads if PTID is -1.  If
    STEP is nonzero, single-step it.  If SIGNAL is nonzero, give it
    that signal.  */
@@ -297,13 +313,16 @@ static void
 inf_ptrace_resume (struct target_ops *ops,
 		   ptid_t ptid, int step, enum gdb_signal signal)
 {
-  pid_t pid = ptid_get_pid (ptid);
+  pid_t pid;
+
   int request;
 
-  if (pid == -1)
+  if (ptid_equal (minus_one_ptid, ptid))
     /* Resume all threads.  Traditionally ptrace() only supports
        single-threaded processes, so simply resume the inferior.  */
-    pid = ptid_get_pid (inferior_ptid);
+    pid = get_ptrace_pid (inferior_ptid);
+  else
+    pid = get_ptrace_pid (ptid);
 
   if (catch_syscall_enabled () > 0)
     request = PT_SYSCALL;
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 2e1133d..cb10e2c 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -1506,8 +1506,6 @@ linux_nat_detach (struct target_ops *ops, const char *args, int from_tty)
 static void
 linux_resume_one_lwp (struct lwp_info *lp, int step, enum gdb_signal signo)
 {
-  ptid_t ptid;
-
   lp->step = step;
 
   /* stop_pc doubles as the PC the LWP had when it was last resumed.
@@ -1524,9 +1522,7 @@ linux_resume_one_lwp (struct lwp_info *lp, int step, enum gdb_signal signo)
 
   if (linux_nat_prepare_to_resume != NULL)
     linux_nat_prepare_to_resume (lp);
-  /* Convert to something the lower layer understands.  */
-  ptid = pid_to_ptid (ptid_get_lwp (lp->ptid));
-  linux_ops->to_resume (linux_ops, ptid, step, signo);
+  linux_ops->to_resume (linux_ops, lp->ptid, step, signo);
   lp->stop_reason = LWP_STOPPED_BY_NO_REASON;
   lp->stopped = 0;
   registers_changed_ptid (lp->ptid);
-- 
1.9.3

RE: [PATCH] btrace: avoid tp != NULL assertion

[Metzger, Markus T]

> -----Original Message-----
> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] On Behalf Of Pedro Alves
> Sent: Tuesday, March 3, 2015 2:42 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion


> > No, that wasn't the reason for replacing the assert.  There are no such
> > errors in the gdb.btrace suite (which is mostly single-threaded) with my
> > patch and I have not seen any such errors otherwise, either.
> 
> Then it sounds like we're either lacking basic tests, or the threaded tests
> are somehow not running correctly when gdb is a 32-bit program.  I think
> that if you step any non-leader thread, you should see it happen.
> Grepping the tests, I think gdb.btrace/multi-thread-step.exp should have
> caught it.  My machine doesn't do btrace, so I can't try it myself...
> 
> BTW, did any existing test in the testsuite catch the assertion we're
> fixing?

Almost all of them when run on 32-bit systems; -m32 on 64-bit systems does
not catch this.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 03/03/2015 01:55 PM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] On Behalf Of Pedro Alves
>> Sent: Tuesday, March 3, 2015 2:42 PM
>> To: Metzger, Markus T
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion
> 
> 
>>> No, that wasn't the reason for replacing the assert.  There are no such
>>> errors in the gdb.btrace suite (which is mostly single-threaded) with my
>>> patch and I have not seen any such errors otherwise, either.
>>
>> Then it sounds like we're either lacking basic tests, or the threaded tests
>> are somehow not running correctly when gdb is a 32-bit program.  I think
>> that if you step any non-leader thread, you should see it happen.
>> Grepping the tests, I think gdb.btrace/multi-thread-step.exp should have
>> caught it.  My machine doesn't do btrace, so I can't try it myself...
>>
>> BTW, did any existing test in the testsuite catch the assertion we're
>> fixing?
> 
> Almost all of them when run on 32-bit systems; -m32 on 64-bit systems does
> not catch this.

Right, that's why I said "when gdb is a 32-bit program".  Sounds like
no existing test tries a "step" when not replaying then.  It'd be very
nice to have one.  Can I convince you to add one?  :-)

Thanks,
Pedro Alves

RE: [PATCH] btrace: avoid tp != NULL assertion

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, March 3, 2015 3:03 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion


> >>> No, that wasn't the reason for replacing the assert.  There are no such
> >>> errors in the gdb.btrace suite (which is mostly single-threaded) with my
> >>> patch and I have not seen any such errors otherwise, either.
> >>
> >> Then it sounds like we're either lacking basic tests, or the threaded tests
> >> are somehow not running correctly when gdb is a 32-bit program.  I think
> >> that if you step any non-leader thread, you should see it happen.
> >> Grepping the tests, I think gdb.btrace/multi-thread-step.exp should have
> >> caught it.  My machine doesn't do btrace, so I can't try it myself...
> >>
> >> BTW, did any existing test in the testsuite catch the assertion we're
> >> fixing?
> >
> > Almost all of them when run on 32-bit systems; -m32 on 64-bit systems
> does
> > not catch this.
> 
> Right, that's why I said "when gdb is a 32-bit program".  Sounds like
> no existing test tries a "step" when not replaying then.  It'd be very
> nice to have one.  Can I convince you to add one?  :-)

The multi-thread-step.exp test does not catch it because it uses "cont",
which works fine.  When I add a "step" before the "cont", I get the
"No thread" error when using my old patch instead of your new patch.
Or I get the assert when using neither my old nor your new patch.
But then, I got the assert already on other tests.

With my patch dropped and your patch committed, what is the new
test expected to catch?

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 03/03/2015 02:44 PM, Metzger, Markus T wrote:
>> -----Original Message-----
>> From: Pedro Alves [mailto:palves@redhat.com]
>> Sent: Tuesday, March 3, 2015 3:03 PM
>> To: Metzger, Markus T
>> Cc: gdb-patches@sourceware.org
>> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion
> 
> 
>>>>> No, that wasn't the reason for replacing the assert.  There are no such
>>>>> errors in the gdb.btrace suite (which is mostly single-threaded) with my
>>>>> patch and I have not seen any such errors otherwise, either.
>>>>
>>>> Then it sounds like we're either lacking basic tests, or the threaded tests
>>>> are somehow not running correctly when gdb is a 32-bit program.  I think
>>>> that if you step any non-leader thread, you should see it happen.
>>>> Grepping the tests, I think gdb.btrace/multi-thread-step.exp should have
>>>> caught it.  My machine doesn't do btrace, so I can't try it myself...
>>>>
>>>> BTW, did any existing test in the testsuite catch the assertion we're
>>>> fixing?
>>>
>>> Almost all of them when run on 32-bit systems; -m32 on 64-bit systems
>> does
>>> not catch this.
>>
>> Right, that's why I said "when gdb is a 32-bit program".  Sounds like
>> no existing test tries a "step" when not replaying then.  It'd be very
>> nice to have one.  Can I convince you to add one?  :-)
> 
> The multi-thread-step.exp test does not catch it because it uses "cont",
> which works fine.  When I add a "step" before the "cont", I get the
> "No thread" error when using my old patch instead of your new patch.
> Or I get the assert when using neither my old nor your new patch.
> But then, I got the assert already on other tests.
> 
> With my patch dropped and your patch committed, what is the new
> test expected to catch?

You're getting me confused...

The test was expected to catch the assertion, given that apparently
no other test was catching it -- from the dialog above, one understands
no test would be catching this before (that's what I explicitly
asked), but now you're saying the opposite.

If indeed there are already tests that triggered the
error/internal-error before the fix, then I agree a new
test is not necessary.

-- 
Pedro Alves

RE: [PATCH] btrace: avoid tp != NULL assertion

[Metzger, Markus T]

> -----Original Message-----
> From: Pedro Alves [mailto:palves@redhat.com]
> Sent: Tuesday, March 3, 2015 4:25 PM
> To: Metzger, Markus T
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] btrace: avoid tp != NULL assertion


> >>>>> No, that wasn't the reason for replacing the assert.  There are no such
> >>>>> errors in the gdb.btrace suite (which is mostly single-threaded) with
> my
> >>>>> patch and I have not seen any such errors otherwise, either.
> >>>>
> >>>> Then it sounds like we're either lacking basic tests, or the threaded
> tests
> >>>> are somehow not running correctly when gdb is a 32-bit program.  I
> think
> >>>> that if you step any non-leader thread, you should see it happen.
> >>>> Grepping the tests, I think gdb.btrace/multi-thread-step.exp should
> have
> >>>> caught it.  My machine doesn't do btrace, so I can't try it myself...
> >>>>
> >>>> BTW, did any existing test in the testsuite catch the assertion we're
> >>>> fixing?
> >>>
> >>> Almost all of them when run on 32-bit systems; -m32 on 64-bit systems
> >> does
> >>> not catch this.
> >>
> >> Right, that's why I said "when gdb is a 32-bit program".  Sounds like
> >> no existing test tries a "step" when not replaying then.  It'd be very
> >> nice to have one.  Can I convince you to add one?  :-)
> >
> > The multi-thread-step.exp test does not catch it because it uses "cont",
> > which works fine.  When I add a "step" before the "cont", I get the
> > "No thread" error when using my old patch instead of your new patch.
> > Or I get the assert when using neither my old nor your new patch.
> > But then, I got the assert already on other tests.
> >
> > With my patch dropped and your patch committed, what is the new
> > test expected to catch?
> 
> You're getting me confused...
> 
> The test was expected to catch the assertion, given that apparently
> no other test was catching it -- from the dialog above, one understands
> no test would be catching this before (that's what I explicitly
> asked), but now you're saying the opposite.

I think that was a misunderstanding.  The assertion is caught by several
gdb.btrace tests when run with 32-bit GDB.

I thought you were referring to the badness in my patch that would
result in GDB asking for registers in a wrong process.

Regards,
Markus.
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052

Re: [PATCH] btrace: avoid tp != NULL assertion

[Pedro Alves]

On 03/03/2015 03:37 PM, Metzger, Markus T wrote:

> I think that was a misunderstanding.  The assertion is caught by several
> gdb.btrace tests when run with 32-bit GDB.
> 
> I thought you were referring to the badness in my patch that would
> result in GDB asking for registers in a wrong process.

Ah.  Alright, glad we're sorted.  :-)

Thanks,
Pedro Alves