[binutils-gdb] Add thread after updating gdbarch when exec'ing

[Simon Marchi <simark at sourceware dot org>]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=bf93d7ba99319c4ff5f23f35d9645dc9c3d391c2

commit bf93d7ba99319c4ff5f23f35d9645dc9c3d391c2
Author: Simon Marchi <simon.marchi@ericsson.com>
Date:   Tue Sep 5 17:30:27 2017 +0200

    Add thread after updating gdbarch when exec'ing
    
    As mentioned in the previous patch, we should avoid doing register reads
    after a process does an exec and before we've updated that inferior's
    gdbarch.  Otherwise, we may interpret the registers using the wrong
    architecture.  When a process does an exec with "follow-exec-mode new",
    a new inferior is added by follow_exec.  The gdbarch of that new
    inferior is at first set to some default value, probably specific to the
    gdb build (I get "i386" here), which may not be the right one.  It is
    updated later by the call to target_find_description.  Before that
    point, if we try to read the inferior's registers, we may not interpret
    them correctly.  This has been exposed by a failure in
    gdb.base/foll-exec-mode.exp after the previous patch, with:
    
      Remote 'g' packet reply is too long (expected 312 bytes, got 816 bytes)
    
    The call to "add_thread" done just after adding the inferior is
    problematic, because it ends up reading the registers (because the ptid
    is re-used, we end up doing a switch_to_thread to it, which tries to
    update stop_pc).  The registers returned by gdbserver are the x86-64
    ones, while we try to interpret them using the "i386" gdbarch.
    
    Postponing the call to add_thread to until the target
    description/gdbarch has been updated seems to fix the issue.
    
    As to why this issue was uncovered by the previous patch: what I think
    happened before that patch is that since we were updating stop_pc before
    switching to the new inferior, we were filling the regcache associated
    to the ptid (this worked fine as long as the architectures of the
    previous and new process images were the same).  The call to
    switch_to_thread then worked, because the register read hit the
    regcache.  Now, it triggers a register read, while the gdbarch is not
    set correctly, leading to the "reply is too long" error.  If this is
    right, it sounds wrong that we delete and re-add a thread with the same
    ptid, and are able to access the registers from the deleted thread.
    When we delete a thread, should we clear the regcache associated to that
    ptid, so that the new thread starts with a fresh/empty regcache?
    
    gdb/ChangeLog:
    
    	* infrun.c (follow_exec): Call add_thread after
    	target_find_description.

Diff:
---
 gdb/ChangeLog | 5 +++++
 gdb/infrun.c  | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 2fe8ab1..7e3a7ea 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-09-05  Simon Marchi  <simon.marchi@ericsson.com>
 
+	* infrun.c (follow_exec): Call add_thread after
+	target_find_description.
+
+2017-09-05  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* infrun.c (handle_inferior_event_1): When exec'ing, read
 	stop_pc after follow_exec.
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 1c6b186..1e301d3 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1211,7 +1211,6 @@ follow_exec (ptid_t ptid, char *exec_file_target)
 
       set_current_inferior (inf);
       set_current_program_space (inf->pspace);
-      add_thread (ptid);
     }
   else
     {
@@ -1243,6 +1242,11 @@ follow_exec (ptid_t ptid, char *exec_file_target)
      registers.  */
   target_find_description ();
 
+  /* The add_thread call ends up reading registers, so do it after updating the
+     target description.  */
+  if (follow_exec_mode_string == follow_exec_mode_new)
+    add_thread (ptid);
+
   solib_create_inferior_hook (0);
 
   jit_inferior_created_hook ();