[19/28] Don't write to inferior_ptid in btrace_fetch

Message ID 20200414175434.8047-20-palves@redhat.com
State New
Headers show
Series
  • Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412)
Related show

Commit Message

Shahab Vahedi via Gdb-patches April 14, 2020, 5:54 p.m.
AFAICT, this isn't required nowadays.

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* btrace.c (btrace_fetch): Don't save/restore inferior_ptid.
---
 gdb/btrace.c | 6 ------
 1 file changed, 6 deletions(-)

-- 
2.14.5

Comments

Shahab Vahedi via Gdb-patches April 15, 2020, 4:52 a.m. | #1
Hello Pedro,

> AFAICT, this isn't required nowadays.


How is this working nowadays?  There are still references to inferior_ptid.

Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
Shahab Vahedi via Gdb-patches April 15, 2020, 2:13 p.m. | #2
On 4/15/20 5:52 AM, Metzger, Markus T via Gdb-patches wrote:
> Hello Pedro,

> 

>> AFAICT, this isn't required nowadays.

> 

> How is this working nowadays?  There are still references to inferior_ptid.


Clearly I didn't think this one through.  I put a tp->ptid==inferior_ptid
assertion in place, and that regressed gdb.python/py-record-btrace-threads.exp.
This should mean that the target_read_btrace calls in btrace_fetch can hit
the wrong thread.

So here's an alternative patch.  No regressions when tested with:

 $ make check TESTS="gdb.btrace/*.exp */*btrace*.exp"

From 92c8fa469295b5737e6cb5e882024af2e9872c4c Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>

Date: Mon, 13 Apr 2020 20:02:22 +0100
Subject: [PATCH 19/28] Don't write to inferior_ptid in btrace.c

gdb/ChangeLog:
yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

	* btrace.c (btrace_fetch): Use switch_to_thread instead of writing
	to inferior_ptid.
---
 gdb/btrace.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/gdb/btrace.c b/gdb/btrace.c
index bbf8749649..2964455680 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1908,13 +1908,13 @@ btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
   if (btinfo->replay != NULL)
     return;
 
-  /* With CLI usage, TP->PTID always equals INFERIOR_PTID here.  Now that we
-     can store a gdb.Record object in Python referring to a different thread
-     than the current one, temporarily set INFERIOR_PTID.  */
-  scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-  inferior_ptid = tp->ptid;
+  /* With CLI usage, TP is always the current thread when we get here.
+     However, since we can also store a gdb.Record object in Python
+     referring to a different thread than the current one, we need to
+     temporarily set the current thread.  */
+  scoped_restore_current_thread restore_thread;
+  switch_to_thread (tp);
 
   /* We should not be called on running or exited threads.  */
   gdb_assert (can_access_registers_thread (tp));

-- 
2.14.5
Shahab Vahedi via Gdb-patches April 15, 2020, 3:17 p.m. | #3
> From 92c8fa469295b5737e6cb5e882024af2e9872c4c Mon Sep 17 00:00:00 2001

> From: Pedro Alves <palves@redhat.com>

> Date: Mon, 13 Apr 2020 20:02:22 +0100

> Subject: [PATCH 19/28] Don't write to inferior_ptid in btrace.c

> 

> gdb/ChangeLog:

> yyyy-mm-dd  Pedro Alves  <palves@redhat.com>

> 

> 	* btrace.c (btrace_fetch): Use switch_to_thread instead of writing

> 	to inferior_ptid.

> ---

>  gdb/btrace.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)


Looks good to me.

> So here's an alternative patch.  No regressions when tested with:

> 

>  $ make check TESTS="gdb.btrace/*.exp */*btrace*.exp"


And thanks for explicitly running the btrace tests.

Regards,
Markus.

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928

Patch

diff --git a/gdb/btrace.c b/gdb/btrace.c
index bbf8749649..889507bfd2 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1908,12 +1908,6 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
   if (btinfo->replay != NULL)
     return;
 
-  /* With CLI usage, TP->PTID always equals INFERIOR_PTID here.  Now that we
-     can store a gdb.Record object in Python referring to a different thread
-     than the current one, temporarily set INFERIOR_PTID.  */
-  scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-  inferior_ptid = tp->ptid;
-
   /* We should not be called on running or exited threads.  */
   gdb_assert (can_access_registers_thread (tp));