diff mbox

[review] gdb/record: Get gdbarch from regcache

Message ID gerrit.1571522289000.I8b1666e28ac25e69839faa6a954eece01bc8fa80@gnutoolchain-gerrit.osci.io
State New
Headers show

Commit Message

Simon Marchi (Code Review) Oct. 19, 2019, 9:58 p.m. UTC
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/163

gdb/record: Get gdbarch from regcache

While experimenting with an out of tree simulator and trying to get
process record working (in order to support reverse execution) I ran
into an issue in record_full_target::resume (in record-full.c).

First an observation, all of the targets that setup process record
support seem to be linux targets in files *-linux-tdep.c, except for
s390-tdep.c.  The target I'm working on is a bare metal simulator.
Maybe my mistake is thinking that I'll be able to get bare metal
reverse execution working, however, my limited experiments so far seem
to be working, so I wonder if this is just me trying something new
that's exposing an issue.

So, my GDB session goes something like this:

  (gdb) file test.exe
  (gdb) target sim
  (gdb) load
  (gdb) start
  (gdb) record
  (gdb) stepi
  /project/gdb/process-stratum-target.c:47: internal-error: virtual gdbarch* process_stratum_target::thread_architecture(ptid_t): Assertion `inf != NULL' failed.

The cause of the assertion starts in infrun.c:resume_1 when we call
internal_resume_ptid, which calls user_visible_resume_ptid.  As the
target doesn't support multi process we end up returning RESUME_ALL,
which is minus_one_ptid.

Eventually we end up back in resume_1, which calls do_target_resume,
which eventually ends up with a call to record_full_target::resume in

In record_full_target::resume we call target_thread_architecture to
get the gdbarch, this is actually a call to thread_architecture on the
current_top_target (), which lands us in

In process_stratum_target::thread_architecture, we call
find_inferior_ptid with the RESUME_ALL ptid (minus_one_ptid), however,
this doesn't correspond to any single inferior, so we get back a NULL
pointer, and the assertion fires.

For a solution I looked back at record_full_target::resume, I notice
that after calling target_thread_architecture we call
get_current_regcache, which relies on the result of calling
inferior_thread ().  If this is acceptable, then we can get the
gdbarch from the regcache, which avoids then need to call

With the simulator I'm working on being currently out of tree I don't
really have a good way to share a reproducer for this issue, so I'm
more interested in just discussing whether my diagnosis makes sense,
or if there's some aspect of the problem I'm missing.


	* record-full.c (record_full_target::resume): Avoid
	target_thread_architecture, and get the gdbarch from the regcache

Change-Id: I8b1666e28ac25e69839faa6a954eece01bc8fa80
M gdb/ChangeLog
M gdb/record-full.c
2 files changed, 9 insertions(+), 2 deletions(-)


Simon Marchi (Code Review) Oct. 25, 2019, 2:54 p.m. UTC | #1
Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/163

Patch Set 1: Code-Review+1

Thanks for the patch and the great explanation.

I don't know much about the record code, but your explanation made
sense and the patch looks reasonable to me.

I would say that if it passes record/replay testing on native, it seems ok.
diff mbox


diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c6e516d..2705e74 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@ 
+2019-10-19  Andrew Burgess  <andrew.burgess@embecosm.com>
+	* record-full.c (record_full_target::resume): Avoid
+	target_thread_architecture, and get the gdbarch from the regcache
+	instead.
 2019-10-19  Sergio Durigan Junior  <sergiodj@redhat.com>
 	* symfile.c (init_entry_point_info): Fix typo.
diff --git a/gdb/record-full.c b/gdb/record-full.c
index a940274..4e16170 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -1070,9 +1070,10 @@ 
-      struct gdbarch *gdbarch = target_thread_architecture (ptid);
+      struct regcache *regcache = get_current_regcache ();
+      struct gdbarch *gdbarch = regcache->arch ();
-      record_full_message (get_current_regcache (), signal);
+      record_full_message (regcache, signal);
       if (!step)