[PATCH 1/9] gdb: Check the selected-frame in frame_find_by_id.

[Andrew Burgess <andrew dot burgess at embecosm dot com>]

In gdb we track two different things, the current-frame, and the
selected-frame.  The current-frame represents the frame in which we are
currently stopped.  If gdb stops at a breakpoint and the user asks for a
backtrace then all the frames in the backtrace will be the ancestors of
the the current-frame.

Also the current-frame does not change until the inferior is set running
again (though it is possible for the user to alter the contents of the
current-frame this is not the same as changing the current-frame).

There is also the selected-frame.  This is the frame that the user has
"selected" using the frame selection commands 'frame', 'up', or 'down'
being the most common.

Usually the selected-frame is always an ancestor of the current-frame,
however, there is a feature of the 'frame' command that allows arbitrary
frames to be created, in this case the selected-frame might not be an
ancestor of the current-frame.

The problem this causes is that lazy register values hold the frame-id
for the frame in which the real register value can be obtained.  When
gdb needs to fetch the actual register value we try to find the frame
using frame_find_by_id, which currently looks at the current-frame, and
all the ancestors of the current-frame.

When the selected-frame is not an ancestor of the current-frame then
frame_find_by_id will fail to find the required frame, and gdb will fail
with an assertion while trying to fetch the lazy value (the assertion is
that the frame will always be found).

This patch extends frame_find_by_id to also check the selected-frame as
a separate comparison as well as the current-frame and its ancestors.
This resolves the issue.

There are also two new tests added that show this issue.  The first is
an mi test using var objects, which is where I first saw this issue.
The second is a slightly simpler test, just creating a new frame then
calling 'info frame' is enough to trigger the assertion.

gdb/ChangeLog:

	* frame.c (frame_find_by_id): Also check the selected frame.

gdb/testsuite/ChangeLog:

	* gdb.mi/mi-var-frame.c: New file.
	* gdb.mi/mi-var-frame.exp: New file.
	* gdb.base/frame-cmds.c: New file.
	* gdb.base/frame-cmds.exp: New file.
---
 gdb/ChangeLog                         |  4 ++
 gdb/frame.c                           | 18 +++++++--
 gdb/testsuite/ChangeLog               |  7 ++++
 gdb/testsuite/gdb.base/frame-cmds.c   | 34 +++++++++++++++++
 gdb/testsuite/gdb.base/frame-cmds.exp | 30 +++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-frame.c   | 34 +++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-frame.exp | 70 +++++++++++++++++++++++++++++++++++
 7 files changed, 194 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/frame-cmds.c
 create mode 100644 gdb/testsuite/gdb.base/frame-cmds.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.exp

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d7a8d31..b4fe9cc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* frame.c (frame_find_by_id): Also check the selected frame.
+
 2015-09-11  Pierre Langlois  <pierre.langlois@arm.com>
 
 	* aarch64-tdep.c (decode_cb): Move up comment describing the
diff --git a/gdb/frame.c b/gdb/frame.c
index 745e007..114fa6a 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -732,7 +732,7 @@ frame_id_inner (struct gdbarch *gdbarch, struct frame_id l, struct frame_id r)
 struct frame_info *
 frame_find_by_id (struct frame_id id)
 {
-  struct frame_info *frame, *prev_frame;
+  struct frame_info *frame, *prev_frame, *selected_frame;
 
   /* ZERO denotes the null frame, let the caller decide what to do
      about it.  Should it instead return get_current_frame()?  */
@@ -761,7 +761,7 @@ frame_find_by_id (struct frame_id id)
 
       prev_frame = get_prev_frame (frame);
       if (!prev_frame)
-	return NULL;
+	break;
 
       /* As a safety net to avoid unnecessary backtracing while trying
 	 to find an invalid ID, we check for a common situation where
@@ -772,8 +772,20 @@ frame_find_by_id (struct frame_id id)
 	  && !frame_id_inner (get_frame_arch (frame), id, self)
 	  && frame_id_inner (get_frame_arch (prev_frame), id,
 			     get_frame_id (prev_frame)))
-	return NULL;
+	break;
+    }
+
+  /* We check the selected frame specifically here to handle the case where
+     the selected frame is not an ancestor of the current frame.  */
+  selected_frame = get_selected_frame_if_set ();
+  if (selected_frame)
+    {
+      struct frame_id selected_frame_id = get_frame_id (selected_frame);
+
+      if (frame_id_eq (id, selected_frame_id))
+	return frame;
     }
+
   return NULL;
 }
 
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 1538d15..cf83b3a 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-09-11  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.mi/mi-var-frame.c: New file.
+	* gdb.mi/mi-var-frame.exp: New file.
+	* gdb.base/frame-cmds.c: New file.
+	* gdb.base/frame-cmds.exp: New file.
+
 2015-09-09  Doug Evans  <dje@google.com>
 
 	* gdb.python/py-prettyprint.exp: Check result of run_lang_tests.
diff --git a/gdb/testsuite/gdb.base/frame-cmds.c b/gdb/testsuite/gdb.base/frame-cmds.c
new file mode 100644
index 0000000..53b222d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-cmds.c
@@ -0,0 +1,34 @@
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+int
+frame_2 (void)
+{
+  return 0;
+}
+
+int
+frame_1 (void)
+{
+  return frame_2 ();
+}
+
+int
+main (void)
+{
+  return frame_1 ();
+}
diff --git a/gdb/testsuite/gdb.base/frame-cmds.exp b/gdb/testsuite/gdb.base/frame-cmds.exp
new file mode 100644
index 0000000..2518463
--- /dev/null
+++ b/gdb/testsuite/gdb.base/frame-cmds.exp
@@ -0,0 +1,30 @@
+# Copyright 2015 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/>.
+
+if { [prepare_for_testing create-frame.exp "create-frame" {create-frame.c} {debug nowarnings}] } {
+    return -1
+}
+set srcfile create-frame.c
+
+runto_main
+gdb_breakpoint frame_2
+gdb_continue_to_breakpoint frame_2
+
+gdb_test "bt" "#0.*#1.*#2.*" "backtrace at breakpoint"
+
+gdb_test_no_output "select-frame 3"
+
+gdb_test "info frame" "Stack level 0, frame at 0x3:.*" \
+    "info frame for created frame"
diff --git a/gdb/testsuite/gdb.mi/mi-var-frame.c b/gdb/testsuite/gdb.mi/mi-var-frame.c
new file mode 100644
index 0000000..53b222d
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-var-frame.c
@@ -0,0 +1,34 @@
+/* Copyright 2015 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   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/>.  */
+
+int
+frame_2 (void)
+{
+  return 0;
+}
+
+int
+frame_1 (void)
+{
+  return frame_2 ();
+}
+
+int
+main (void)
+{
+  return frame_1 ();
+}
diff --git a/gdb/testsuite/gdb.mi/mi-var-frame.exp b/gdb/testsuite/gdb.mi/mi-var-frame.exp
new file mode 100644
index 0000000..5c3f196
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-var-frame.exp
@@ -0,0 +1,70 @@
+# Copyright 2015 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/>.
+
+# This tests an issue relating to updating var objects in user created
+# frames.
+
+# This test requires a register name for creating a var object.
+# Currently only x86/x86_64 like targets are supported.
+if {![istarget "x86_64-*-*"] && ![istarget "i?86-*-*"]} {
+    return 0
+}
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+standard_testfile mi-var-frame.c
+
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
+    untested mi-var-frame.exp
+    return -1
+}
+
+mi_delete_breakpoints
+mi_gdb_reinitialize_dir $srcdir/$subdir
+mi_gdb_load ${binfile}
+
+mi_create_breakpoint "frame_2" \
+    "break-insert into frame_2" \
+    -number 1 -func frame_2 -file ".*${srcfile}"
+
+mi_run_cmd
+mi_expect_stop "breakpoint-hit" "frame_2" "" ".*${srcfile}" \
+	".*" { "" "disp=\"keep\"" } "run to breakpoint"
+
+mi_gdb_test "-stack-info-depth" \
+    "\\^done,depth=\"3\"" \
+    "stack info-depth"
+
+set register "\$eax"
+
+mi_gdb_test "-var-create R * $register" \
+    "\\^done,name=\"R\",numchild=\"0\",value=\".*\",type=\".*\",has_more=\"0\"" \
+    "create varobj 'R' for register '$register'"
+
+for {set i 0} {$i < 5} {incr i} {
+    mi_gdb_test "-stack-select-frame $i" \
+	{\^done} \
+	"-stack-select-frame $i"
+
+    mi_gdb_test "-var-update R" \
+	"\\^done,changelist=\\\[\\\]" \
+	"update 'R' in frame $i: no change"
+}
-- 
2.5.1