[PATCH] Associate dummy_frame with ptid

[Yao Qi <yao at codesourcery dot com>]

This patch is to add ptid into dummy_frame and extend frame_id to
dummy_frame_id (which has a ptid field).  With this change, GDB uses
dummy_frame_id (thread ptid and frame_id) to find the dummy frames.

Currently, dummy frames are looked up by frame_id, which isn't
accurate in non-stop or multi-process mode.  The test case
gdb.multi/dummy-frame-restore.exp shows the problem and this patch can
fix it.

Test dummy-frame-restore.exp makes two inferiors stop at
different functions, say, inferior 1 stops at f1 while inferior 2
stops at f2.  Set a breakpoint to a function, do the inferior call
in two inferiors, and GDB has two dummy frames of the same frame_id.
When the inferior call is finished, GDB will look up a dummy frame
from its stack/list and restore the inferior's regcache.  Two
inferiors are finished in different orders, the inferiors' states are
restored differently, which is wrong.  Running dummy-frame-restore.exp
under un-patched GDB, we'll get two fails:

FAIL: gdb.multi/dummy-frame-restore.exp: inf 2 first: after infcall: bt in inferior 2
FAIL: gdb.multi/dummy-frame-restore.exp: inf 2 first: after infcall: bt in inferior 1

With this patch applied, GDB will choose the correct dummy_frame to
restore for a given inferior, because ptid is considered when looking up
dummy frames.  Two fails above are fixed.

Regression tested on x86_64-linux, both native and gdbserver.

gdb:

2014-06-26  Yao Qi  <yao@codesourcery.com>

	* breakpoint.c (check_longjmp_breakpoint_for_call_dummy):
	Change parameter type to 'struct thread_info *'.  Caller
	updated.
	* breakpoint.h (check_longjmp_breakpoint_for_call_dummy):
	Update declaration.
	* dummy-frame.c (struct dummy_frame_id): New.
	(dummy_frame_id_eq): New function.
	(struct dummy_frame) <id>: Change its type to 'struct
	dummy_frame_id'.
	(dummy_frame_push): Add parameter ptid and save it in
	dummy_frame_id.
	(pop_dummy_frame_bpt): Use ptid of dummy_frame instead of
	inferior_ptid.
	(pop_dummy_frame): Assert that the ptid of dummy_frame equals
	to inferior_ptid.
	(lookup_dummy_frame): Change parameter type to 'struct
	dummy_frame_id *'.  Callers updated.  Call dummy_frame_id_eq
	instead of frame_id_eq.
	(dummy_frame_pop): Add parameter ptid.  Callers updated.
	Update comments.  Compose dummy_frame_id and pass it to
	lookup_dummy_frame.
	(dummy_frame_discard): Add parameter ptid.
	(dummy_frame_sniffer): Compose dummy_frame_id and call
	dummy_frame_id_eq instead of frame_id_eq.
	* dummy-frame.h: Remove comments.
	(dummy_frame_push): Add ptid in declaration.
	(dummy_frame_pop, dummy_frame_discard): Likewise.

gdb/testsuite:

2014-06-26  Yao Qi  <yao@codesourcery.com>

	* gdb.multi/dummy-frame-restore.exp: New.
	* gdb.multi/dummy-frame-restore.c: New.
---
 gdb/breakpoint.c                                |  8 +--
 gdb/breakpoint.h                                |  2 +-
 gdb/dummy-frame.c                               | 70 +++++++++++++-------
 gdb/dummy-frame.h                               | 11 +---
 gdb/frame.c                                     |  2 +-
 gdb/infcall.c                                   |  6 +-
 gdb/infrun.c                                    |  2 +-
 gdb/testsuite/gdb.multi/dummy-frame-restore.c   | 36 +++++++++++
 gdb/testsuite/gdb.multi/dummy-frame-restore.exp | 85 +++++++++++++++++++++++++
 9 files changed, 180 insertions(+), 42 deletions(-)
 create mode 100644 gdb/testsuite/gdb.multi/dummy-frame-restore.c
 create mode 100644 gdb/testsuite/gdb.multi/dummy-frame-restore.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index b04f3b7..0b49dec 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -7460,7 +7460,7 @@ set_longjmp_breakpoint_for_call_dummy (void)
 }
 
 /* Verify all existing dummy frames and their associated breakpoints for
-   THREAD.  Remove those which can no longer be found in the current frame
+   TP.  Remove those which can no longer be found in the current frame
    stack.
 
    You should call this function only at places where it is safe to currently
@@ -7468,12 +7468,12 @@ set_longjmp_breakpoint_for_call_dummy (void)
    frames.  */
 
 void
-check_longjmp_breakpoint_for_call_dummy (int thread)
+check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp)
 {
   struct breakpoint *b, *b_tmp;
 
   ALL_BREAKPOINTS_SAFE (b, b_tmp)
-    if (b->type == bp_longjmp_call_dummy && b->thread == thread)
+    if (b->type == bp_longjmp_call_dummy && b->thread == tp->num)
       {
 	struct breakpoint *dummy_b = b->related_breakpoint;
 
@@ -7483,7 +7483,7 @@ check_longjmp_breakpoint_for_call_dummy (int thread)
 	    || frame_find_by_id (dummy_b->frame_id) != NULL)
 	  continue;
 	
-	dummy_frame_discard (dummy_b->frame_id);
+	dummy_frame_discard (dummy_b->frame_id, tp->ptid);
 
 	while (b->related_breakpoint != b)
 	  {
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index d8e88fc..83ae9e6 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1329,7 +1329,7 @@ extern void delete_longjmp_breakpoint (int thread);
 extern void delete_longjmp_breakpoint_at_next_stop (int thread);
 
 extern struct breakpoint *set_longjmp_breakpoint_for_call_dummy (void);
-extern void check_longjmp_breakpoint_for_call_dummy (int thread);
+extern void check_longjmp_breakpoint_for_call_dummy (struct thread_info *tp);
 
 extern void enable_overlay_breakpoints (void);
 extern void disable_overlay_breakpoints (void);
diff --git a/gdb/dummy-frame.c b/gdb/dummy-frame.c
index dd2ff12..a7cd086 100644
--- a/gdb/dummy-frame.c
+++ b/gdb/dummy-frame.c
@@ -31,6 +31,25 @@
 #include "observer.h"
 #include "gdbthread.h"
 
+struct dummy_frame_id
+{
+  /* This frame's ID.  Must match the value returned by
+     gdbarch_dummy_id.  */
+  struct frame_id id;
+
+  /* The thread this dummy_frame relates to.  */
+  ptid_t ptid;
+};
+
+/* Return whether dummy_frame_id *ID1 and *ID2 are equal.  */
+
+static int
+dummy_frame_id_eq (struct dummy_frame_id *id1,
+		   struct dummy_frame_id *id2)
+{
+  return frame_id_eq (id1->id, id2->id) && ptid_equal (id1->ptid, id2->ptid);
+}
+
 /* Dummy frame.  This saves the processor state just prior to setting
    up the inferior function call.  Older targets save the registers
    on the target stack (but that really slows down function calls).  */
@@ -38,9 +57,10 @@
 struct dummy_frame
 {
   struct dummy_frame *next;
-  /* This frame's ID.  Must match the value returned by
-     gdbarch_dummy_id.  */
-  struct frame_id id;
+
+  /* An id represents a dummy frame.  */
+  struct dummy_frame_id id;
+
   /* The caller's state prior to the call.  */
   struct infcall_suspend_state *caller_state;
 };
@@ -52,13 +72,14 @@ static struct dummy_frame *dummy_frame_stack = NULL;
 
 void
 dummy_frame_push (struct infcall_suspend_state *caller_state,
-		  const struct frame_id *dummy_id)
+		  const struct frame_id *dummy_id, ptid_t ptid)
 {
   struct dummy_frame *dummy_frame;
 
   dummy_frame = XCNEW (struct dummy_frame);
   dummy_frame->caller_state = caller_state;
-  dummy_frame->id = (*dummy_id);
+  dummy_frame->id.id = (*dummy_id);
+  dummy_frame->id.ptid = ptid;
   dummy_frame->next = dummy_frame_stack;
   dummy_frame_stack = dummy_frame;
 }
@@ -83,8 +104,8 @@ pop_dummy_frame_bpt (struct breakpoint *b, void *dummy_voidp)
 {
   struct dummy_frame *dummy = dummy_voidp;
 
-  if (b->thread == pid_to_thread_id (inferior_ptid)
-      && b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id))
+  if (b->thread == pid_to_thread_id (dummy->id.ptid)
+      && b->disposition == disp_del && frame_id_eq (b->frame_id, dummy->id.id))
     {
       while (b->related_breakpoint != b)
 	delete_breakpoint (b->related_breakpoint);
@@ -107,6 +128,7 @@ pop_dummy_frame (struct dummy_frame **dummy_ptr)
 {
   struct dummy_frame *dummy = *dummy_ptr;
 
+  gdb_assert (ptid_equal (dummy->id.ptid, inferior_ptid));
   restore_infcall_suspend_state (dummy->caller_state);
 
   iterate_over_breakpoints (pop_dummy_frame_bpt, dummy);
@@ -125,48 +147,47 @@ pop_dummy_frame (struct dummy_frame **dummy_ptr)
    Return NULL if not found.  */
 
 static struct dummy_frame **
-lookup_dummy_frame (struct frame_id dummy_id)
+lookup_dummy_frame (struct dummy_frame_id *dummy_id)
 {
   struct dummy_frame **dp;
 
   for (dp = &dummy_frame_stack; *dp != NULL; dp = &(*dp)->next)
     {
-      if (frame_id_eq ((*dp)->id, dummy_id))
+      if (dummy_frame_id_eq (&(*dp)->id, dummy_id))
 	return dp;
     }
 
   return NULL;
 }
 
-/* Pop the dummy frame DUMMY_ID, restoring program state to that before the
-   frame was created.
+/* Find the dummy frame by DUMMY_ID and PTID, and pop it, restoring
+   program state to that before the frame was created.
    On return reinit_frame_cache has been called.
-   If the frame isn't found, flag an internal error.
-
-   NOTE: This can only pop the one frame, even if it is in the middle of the
-   stack, because the other frames may be for different threads, and there's
-   currently no way to tell which stack frame is for which thread.  */
+   If the frame isn't found, flag an internal error.  */
 
 void
-dummy_frame_pop (struct frame_id dummy_id)
+dummy_frame_pop (struct frame_id dummy_id, ptid_t ptid)
 {
   struct dummy_frame **dp;
+  struct dummy_frame_id id = { dummy_id, ptid };
 
-  dp = lookup_dummy_frame (dummy_id);
+  dp = lookup_dummy_frame (&id);
   gdb_assert (dp != NULL);
 
   pop_dummy_frame (dp);
 }
 
-/* Drop dummy frame DUMMY_ID.  Do nothing if it is not found.  Do not restore
-   its state into inferior, just free its memory.  */
+/* Find the dummy frame by DUMMY_ID and PTID and drop it.  Do nothing
+   if it is not found.  Do not restore its state into inferior, just
+   free its memory.  */
 
 void
-dummy_frame_discard (struct frame_id dummy_id)
+dummy_frame_discard (struct frame_id dummy_id, ptid_t ptid)
 {
   struct dummy_frame **dp;
+  struct dummy_frame_id id = { dummy_id, ptid };
 
-  dp = lookup_dummy_frame (dummy_id);
+  dp = lookup_dummy_frame (&id);
   if (dp)
     remove_dummy_frame (dp);
 }
@@ -211,13 +232,14 @@ dummy_frame_sniffer (const struct frame_unwind *self,
 	 dummy ID, assuming it is a dummy frame.  */
       struct frame_id this_id
 	= gdbarch_dummy_id (get_frame_arch (this_frame), this_frame);
+      struct dummy_frame_id dummy_id = { this_id, inferior_ptid };
 
       /* Use that ID to find the corresponding cache entry.  */
       for (dummyframe = dummy_frame_stack;
 	   dummyframe != NULL;
 	   dummyframe = dummyframe->next)
 	{
-	  if (frame_id_eq (dummyframe->id, this_id))
+	  if (dummy_frame_id_eq (&dummyframe->id, &dummy_id))
 	    {
 	      struct dummy_frame_cache *cache;
 
@@ -297,7 +319,7 @@ fprint_dummy_frames (struct ui_file *file)
       gdb_print_host_address (s, file);
       fprintf_unfiltered (file, ":");
       fprintf_unfiltered (file, " id=");
-      fprint_frame_id (file, s->id);
+      fprint_frame_id (file, s->id.id);
       fprintf_unfiltered (file, "\n");
     }
 }
diff --git a/gdb/dummy-frame.h b/gdb/dummy-frame.h
index 6db312e..bac1aac 100644
--- a/gdb/dummy-frame.h
+++ b/gdb/dummy-frame.h
@@ -28,18 +28,13 @@ struct frame_unwind;
 /* Push the information needed to identify, and unwind from, a dummy
    frame onto the dummy frame stack.  */
 
-/* NOTE: cagney/2004-08-02: This interface will eventually need to be
-   parameterized with the caller's thread - that will allow per-thread
-   dummy-frame stacks and, hence, per-thread inferior function
-   calls.  */
-
 /* NOTE: cagney/2004-08-02: In the case of ABIs using push_dummy_code
    containing more than one instruction, this interface many need to
    be expanded so that it knowns the lower/upper extent of the dummy
    frame's code.  */
 
 extern void dummy_frame_push (struct infcall_suspend_state *caller_state,
-                              const struct frame_id *dummy_id);
+                              const struct frame_id *dummy_id, ptid_t ptid);
 
 /* Pop the dummy frame DUMMY_ID, restoring program state to that before the
    frame was created.
@@ -50,9 +45,9 @@ extern void dummy_frame_push (struct infcall_suspend_state *caller_state,
    stack, because the other frames may be for different threads, and there's
    currently no way to tell which stack frame is for which thread.  */
 
-extern void dummy_frame_pop (struct frame_id dummy_id);
+extern void dummy_frame_pop (struct frame_id dummy_id, ptid_t ptid);
 
-extern void dummy_frame_discard (struct frame_id dummy_id);
+extern void dummy_frame_discard (struct frame_id dummy_id, ptid_t ptid);
 
 /* If the PC falls in a dummy frame, return a dummy frame
    unwinder.  */
diff --git a/gdb/frame.c b/gdb/frame.c
index e052069..8a2b8bf 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -960,7 +960,7 @@ frame_pop (struct frame_info *this_frame)
     {
       /* Popping a dummy frame involves restoring more than just registers.
 	 dummy_frame_pop does all the work.  */
-      dummy_frame_pop (get_frame_id (this_frame));
+      dummy_frame_pop (get_frame_id (this_frame), inferior_ptid);
       return;
     }
 
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 685b8a4..52f0612 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -826,7 +826,7 @@ call_function_by_hand (struct value *function, int nargs, struct value **args)
   /* Everything's ready, push all the info needed to restore the
      caller (and identify the dummy-frame) onto the dummy-frame
      stack.  */
-  dummy_frame_push (caller_state, &dummy_id);
+  dummy_frame_push (caller_state, &dummy_id, inferior_ptid);
 
   /* Discard both inf_status and caller_state cleanups.
      From this point on we explicitly restore the associated state
@@ -952,7 +952,7 @@ When the function is done executing, GDB will silently stop."),
 
 	      /* We must get back to the frame we were before the
 		 dummy call.  */
-	      dummy_frame_pop (dummy_id);
+	      dummy_frame_pop (dummy_id, call_thread_ptid);
 
 	      /* We also need to restore inferior status to that before the
 		 dummy call.  */
@@ -993,7 +993,7 @@ When the function is done executing, GDB will silently stop."),
 	{
 	  /* We must get back to the frame we were before the dummy
 	     call.  */
-	  dummy_frame_pop (dummy_id);
+	  dummy_frame_pop (dummy_id, call_thread_ptid);
 
 	  /* We also need to restore inferior status to that before
 	     the dummy call.  */
diff --git a/gdb/infrun.c b/gdb/infrun.c
index de5a938..3c58ff2 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4443,7 +4443,7 @@ process_event_stop_test (struct execution_control_state *ecs)
 
 	if (what.is_longjmp)
 	  {
-	    check_longjmp_breakpoint_for_call_dummy (ecs->event_thread->num);
+	    check_longjmp_breakpoint_for_call_dummy (ecs->event_thread);
 
 	    if (!frame_id_p (ecs->event_thread->initiating_frame))
 	      {
diff --git a/gdb/testsuite/gdb.multi/dummy-frame-restore.c b/gdb/testsuite/gdb.multi/dummy-frame-restore.c
new file mode 100644
index 0000000..118c541
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/dummy-frame-restore.c
@@ -0,0 +1,36 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2014 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+static void
+f1 (void)
+{}
+
+static void
+f2 (void)
+{}
+
+void
+commonfun (void)
+{}
+
+int
+main (void)
+{
+  f1 ();
+  f2 ();
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.multi/dummy-frame-restore.exp b/gdb/testsuite/gdb.multi/dummy-frame-restore.exp
new file mode 100644
index 0000000..3b30eb2
--- /dev/null
+++ b/gdb/testsuite/gdb.multi/dummy-frame-restore.exp
@@ -0,0 +1,85 @@
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+standard_testfile .c
+set executable ${testfile}
+
+# The plain remote target can't do multiple inferiors.
+if [target_info exists use_gdb_stub] {
+    return
+}
+
+if {[prepare_for_testing $testfile.exp $testfile $srcfile {nowarnings debug}]} {
+    return -1
+}
+
+# Inferior 1 stops at f1.
+
+if ![runto f1] then {
+    fail "Can't run to f1"
+    return 0
+}
+
+gdb_test "add-inferior -exec ${binfile}" \
+    "Added inferior 2.*" \
+    "add inferior 2 with -exec ${executable}"
+gdb_test "inferior 2" "witching to inferior 2 .*" ""
+gdb_test "run" "Breakpoint.* f1 .*" "start to f1 inferior 2"
+gdb_breakpoint f2
+# Inferior 2 stops at f2.
+gdb_continue_to_breakpoint f2
+
+gdb_breakpoint commonfun
+
+# Check the stack bactrace in inferior INF.
+
+proc check_bt { inf msg } {
+    with_test_prefix "$msg" {
+	gdb_test "bt 1" "#0  f$inf .*" "bt in inferior $inf"
+    }
+}
+
+# Do inferior call commonfun () in inferior 2 and inferior 1, but
+# finish it in inferior INF1 first and then inferior INF2.  Check
+# the stack backtrace in each inferior is still correct after
+# inferior call.
+
+proc test { inf1 inf2 } {
+
+    with_test_prefix "inf $inf1 first" {
+	gdb_test "inferior 2" "witching to inferior 2 .*" ""
+	check_bt 2 "before infcall"
+	gdb_test "p commonfun()" "Breakpoint .*The program being debugged stopped while in a function called from GDB.*" "infcall in inferior 2"
+
+	gdb_test "inferior 1" "witching to inferior 1 .*" ""
+	check_bt 1 "before infcall"
+	gdb_test "p commonfun()" "Breakpoint .*The program being debugged stopped while in a function called from GDB.*" "infcall in inferior 1"
+
+	gdb_test "maintenance print dummy-frames" ": id={stack=.*}.*: id=.*" \
+	    "two dummy frames"
+
+	gdb_test "inferior $inf1" "witching to inferior $inf1 .*" ""
+	gdb_test "finish" "" ""
+	check_bt $inf1 "after infcall"
+
+	gdb_test "inferior $inf2" "witching to inferior $inf2 .*" ""
+	gdb_test "finish" "" ""
+	check_bt $inf2 "after infcall"
+    }
+}
+
+# Do the test in different orders.
+test 1 2
+test 2 1
-- 
1.9.0