[review] gdb/record: Get gdbarch from regcache

[Andrew Burgess (Code Review)]

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
record-full.c.

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
process_stratum_target::thread_architecture.

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
target_thread_architecture.

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.

gdb/ChangeLog:

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

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



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 @@
 
   if (!RECORD_FULL_IS_REPLAY)
     {
-      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)
         {