public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id.
@ 2015-07-08 16:46 ` Andrew Burgess
  2015-07-23 11:07   ` Andrew Burgess
  2015-07-23 11:07   ` [PATCH 2/2] gdb: Select a frame for frame_info Andrew Burgess
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Burgess @ 2015-07-08 16:46 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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/create-frame.c: New file.
	* gdb.base/create-frame.exp: New file.
---
 gdb/ChangeLog                           |  4 ++
 gdb/frame.c                             | 18 +++++++--
 gdb/testsuite/ChangeLog                 |  7 ++++
 gdb/testsuite/gdb.base/create-frame.c   | 34 ++++++++++++++++
 gdb/testsuite/gdb.base/create-frame.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/create-frame.c
 create mode 100644 gdb/testsuite/gdb.base/create-frame.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 b565dde..4f735e9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-07-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* frame.c (frame_find_by_id): Also check the selected frame.
+
 2015-07-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	PR compile/18484
diff --git a/gdb/frame.c b/gdb/frame.c
index da5bfb9..b442592 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 fe6fddc..b5c0308 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-07-08  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.mi/mi-var-frame.c: New file.
+	* gdb.mi/mi-var-frame.exp: New file.
+	* gdb.base/create-frame.c: New file.
+	* gdb.base/create-frame.exp: New file.
+
 2015-07-08  Jan Kratochvil  <jan.kratochvil@redhat.com>
 
 	PR compile/18484
diff --git a/gdb/testsuite/gdb.base/create-frame.c b/gdb/testsuite/gdb.base/create-frame.c
new file mode 100644
index 0000000..53b222d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/create-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.base/create-frame.exp b/gdb/testsuite/gdb.base/create-frame.exp
new file mode 100644
index 0000000..2518463
--- /dev/null
+++ b/gdb/testsuite/gdb.base/create-frame.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.4.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] gdb: Select a frame for frame_info.
  2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
  2015-07-23 11:07   ` Andrew Burgess
@ 2015-07-23 11:07   ` Andrew Burgess
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Burgess @ 2015-07-23 11:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Within the 'info frame' command (implemented in the frame_info function)
gdb will create register values, which are then displayed to the user.
These register values will be created as lazy value.  As part of
fetching the value of a lazy register value we must find the frame in
which the value was created based on the frame id, which is stored in
the value, this is done with a call to frame_find_by_id.

In a previous commit I already addressed the problem that
frame_find_by_id did not used to consider the selected frame when trying
to find a frame.  This is now fixed.

However, if in frame_info the user passed a frame specification, then it
is possible that a brand new frame will have been created, a frame
outside of the current backtrace, and also not the currently selected
frame.  So, usually, something like, 'info frame 4' will cause gdb to
crash with an assertion failure.  This can be especially annoying if the
user accidentally types 'info frame 4' intending to ask about the fourth
stack frame, but, there happens to only be 3 stack frames at present.

The solution I propose here is that frame_info should temporarily select
the frame identified by the user.  The previously selected frame will be
restored at the end of the function.

gdb/ChangeLog:

	* frame.h (make_restore_selected_frame_cleanup): Declare new
	function.
	* frame.c (do_restore_selected_frame): New function.
	(make_restore_selected_frame_cleanup): Define new function.
	* stack.c (frame_info): Temporarily select frame for duration of
	this function.

gdb/testsuite/ChangeLog:

	* gdb.base/create-frame.exp: Add test for 'info frame' with
	specific address.
---
 gdb/ChangeLog                           |  9 +++++++++
 gdb/frame.c                             | 21 +++++++++++++++++++++
 gdb/frame.h                             |  5 +++++
 gdb/stack.c                             | 10 ++++++++++
 gdb/testsuite/ChangeLog                 |  5 +++++
 gdb/testsuite/gdb.base/create-frame.exp |  8 +++++++-
 6 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b4aa365..747a7c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,14 @@
 2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* frame.h (make_restore_selected_frame_cleanup): Declare new
+	function.
+	* frame.c (do_restore_selected_frame): New function.
+	(make_restore_selected_frame_cleanup): Define new function.
+	* stack.c (frame_info): Temporarily select frame for duration of
+	this function.
+
+2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* frame.c (frame_find_by_id): Also check the selected frame.
 
 2015-07-23  Yao Qi  <yao.qi@linaro.org>
diff --git a/gdb/frame.c b/gdb/frame.c
index b442592..6d7fb82 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -2711,6 +2711,27 @@ frame_prepare_for_sniffer (struct frame_info *frame,
   return make_cleanup (frame_cleanup_after_sniffer, frame);
 }
 
+/* Used as a clean-up to restore the selected frame.  */
+
+static void
+do_restore_selected_frame (void *arg)
+{
+  struct frame_info *fi = (struct frame_info *) arg;
+
+  select_frame (fi);
+}
+
+/* See frame.h.  */
+
+struct cleanup *
+make_restore_selected_frame_cleanup (void)
+{
+  struct frame_info *fi;
+
+  fi = get_selected_frame_if_set ();
+  return make_cleanup (do_restore_selected_frame, fi);
+}
+
 extern initialize_file_ftype _initialize_frame; /* -Wmissing-prototypes */
 
 static struct cmd_list_element *set_backtrace_cmdlist;
diff --git a/gdb/frame.h b/gdb/frame.h
index be64c57..d90a196 100644
--- a/gdb/frame.h
+++ b/gdb/frame.h
@@ -814,4 +814,9 @@ extern struct frame_info *create_new_frame (CORE_ADDR base, CORE_ADDR pc);
 extern int frame_unwinder_is (struct frame_info *fi,
 			      const struct frame_unwind *unwinder);
 
+/* Create a clean-up to restore the selected frame.  This includes
+   restoring the selected frame to NULL if that is its current value.  */
+
+extern struct cleanup * make_restore_selected_frame_cleanup (void);
+
 #endif /* !defined (FRAME_H)  */
diff --git a/gdb/stack.c b/gdb/stack.c
index 4878825..9453fea 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1434,6 +1434,16 @@ frame_info (char *addr_exp, int from_tty)
   fi = parse_frame_specification_1 (addr_exp, "No stack.", &selected_frame_p);
   gdbarch = get_frame_arch (fi);
 
+  /* During the following value will be created and then displayed.
+     Displaying a value can require using frame_find_by_id, so we must make
+     sure that the frame FI is findable using frame_find_by_id.  As it is
+     possible that the frame specification caused a new frame to be
+     created, one outside of the current stack trace, and different to the
+     currently selected frame, then the only way to ensure that we can find
+     this frame again later is if we temporarily select this new frame.  */
+  make_restore_selected_frame_cleanup ();
+  select_frame (fi);
+
   /* Name of the value returned by get_frame_pc().  Per comments, "pc"
      is not a good name.  */
   if (gdbarch_pc_regnum (gdbarch) >= 0)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 51d5937..a6e70bb 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,10 @@
 2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
 
+	* gdb.base/create-frame.exp: Add test for 'info frame' with
+	specific address.
+
+2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
+
 	* gdb.mi/mi-var-frame.c: New file.
 	* gdb.mi/mi-var-frame.exp: New file.
 	* gdb.base/create-frame.c: New file.
diff --git a/gdb/testsuite/gdb.base/create-frame.exp b/gdb/testsuite/gdb.base/create-frame.exp
index 2518463..e285a4e 100644
--- a/gdb/testsuite/gdb.base/create-frame.exp
+++ b/gdb/testsuite/gdb.base/create-frame.exp
@@ -27,4 +27,10 @@ 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"
+    "info frame for currently selected frame, at 0x3"
+
+gdb_test "info frame 4" "Stack frame at 0x4:.*" \
+    "info frame for specific frame, created at 0x4"
+
+gdb_test "info frame" "Stack level 0, frame at 0x3:.*" \
+    "info frame for currently selected frame (again), at 0x3"
-- 
2.4.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id.
  2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
@ 2015-07-23 11:07   ` Andrew Burgess
  2015-07-23 11:07   ` [PATCH 2/2] gdb: Select a frame for frame_info Andrew Burgess
  1 sibling, 0 replies; 6+ messages in thread
From: Andrew Burgess @ 2015-07-23 11:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

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/create-frame.c: New file.
	* gdb.base/create-frame.exp: New file.
---
 gdb/ChangeLog                           |  4 ++
 gdb/frame.c                             | 18 +++++++--
 gdb/testsuite/ChangeLog                 |  7 ++++
 gdb/testsuite/gdb.base/create-frame.c   | 34 ++++++++++++++++
 gdb/testsuite/gdb.base/create-frame.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/create-frame.c
 create mode 100644 gdb/testsuite/gdb.base/create-frame.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 8f390a4..b4aa365 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* frame.c (frame_find_by_id): Also check the selected frame.
+
 2015-07-23  Yao Qi  <yao.qi@linaro.org>
 
 	* aarch64-linux-nat.c (aarch64_linux_can_use_hw_breakpoint): If
diff --git a/gdb/frame.c b/gdb/frame.c
index da5bfb9..b442592 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 17af4fc..51d5937 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2015-07-23  Andrew Burgess  <andrew.burgess@embecosm.com>
+
+	* gdb.mi/mi-var-frame.c: New file.
+	* gdb.mi/mi-var-frame.exp: New file.
+	* gdb.base/create-frame.c: New file.
+	* gdb.base/create-frame.exp: New file.
+
 2015-07-20  Joel Brobecker  <brobecker@adacore.com>
 
 	* gdb.ada/info_exc.exp: Adjust "info exceptions" expected output.
diff --git a/gdb/testsuite/gdb.base/create-frame.c b/gdb/testsuite/gdb.base/create-frame.c
new file mode 100644
index 0000000..53b222d
--- /dev/null
+++ b/gdb/testsuite/gdb.base/create-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.base/create-frame.exp b/gdb/testsuite/gdb.base/create-frame.exp
new file mode 100644
index 0000000..2518463
--- /dev/null
+++ b/gdb/testsuite/gdb.base/create-frame.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.4.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* PING: [PATCH 0/2] frame_find_by_id & selected_frame issues
  2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
@ 2015-07-23 11:07 Andrew Burgess
  2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
  2015-08-07 11:56 ` PING: [PATCH 0/2] frame_find_by_id & selected_frame issues Andrew Burgess
  1 sibling, 2 replies; 6+ messages in thread
From: Andrew Burgess @ 2015-07-23 11:07 UTC (permalink / raw)
  To: gdb-patches; +Cc: Andrew Burgess

Seems that I originally failed to post patch 2/2 from the original
series.  So here's both of them again, rebased on latest master.

I believe that these patches should still be applied even in light of
the discussion here:
  https://sourceware.org/ml/gdb-patches/2015-07/msg00241.html
as any solution that does come out of that conversation will probably
be limited to changing the user interface of gdb, not the underlying
behaviour.  Also that conversation is about future clean up, while
this patch series addresses real, user visible, assertion failures.

--

Andrew Burgess (2):
  gdb: Check the selected-frame in frame_find_by_id.
  gdb: Select a frame for frame_info.

 gdb/ChangeLog                           | 13 ++++++
 gdb/frame.c                             | 39 ++++++++++++++++--
 gdb/frame.h                             |  5 +++
 gdb/stack.c                             | 10 +++++
 gdb/testsuite/ChangeLog                 | 12 ++++++
 gdb/testsuite/gdb.base/create-frame.c   | 34 ++++++++++++++++
 gdb/testsuite/gdb.base/create-frame.exp | 36 +++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-frame.c     | 34 ++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-frame.exp   | 70 +++++++++++++++++++++++++++++++++
 9 files changed, 250 insertions(+), 3 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/create-frame.c
 create mode 100644 gdb/testsuite/gdb.base/create-frame.exp
 create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.c
 create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.exp

-- 
2.4.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* PING: [PATCH 0/2] frame_find_by_id & selected_frame issues
  2015-07-23 11:07 PING: [PATCH 0/2] frame_find_by_id & selected_frame issues Andrew Burgess
  2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
@ 2015-08-07 11:56 ` Andrew Burgess
  2015-09-11 18:53   ` ABANDONED: " Andrew Burgess
  1 sibling, 1 reply; 6+ messages in thread
From: Andrew Burgess @ 2015-08-07 11:56 UTC (permalink / raw)
  To: gdb-patches

Ping!

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2015-07-23 12:07:32 +0100]:

> Seems that I originally failed to post patch 2/2 from the original
> series.  So here's both of them again, rebased on latest master.
> 
> I believe that these patches should still be applied even in light of
> the discussion here:
>   https://sourceware.org/ml/gdb-patches/2015-07/msg00241.html
> as any solution that does come out of that conversation will probably
> be limited to changing the user interface of gdb, not the underlying
> behaviour.  Also that conversation is about future clean up, while
> this patch series addresses real, user visible, assertion failures.
> 
> --
> 
> Andrew Burgess (2):
>   gdb: Check the selected-frame in frame_find_by_id.
>   gdb: Select a frame for frame_info.
> 
>  gdb/ChangeLog                           | 13 ++++++
>  gdb/frame.c                             | 39 ++++++++++++++++--
>  gdb/frame.h                             |  5 +++
>  gdb/stack.c                             | 10 +++++
>  gdb/testsuite/ChangeLog                 | 12 ++++++
>  gdb/testsuite/gdb.base/create-frame.c   | 34 ++++++++++++++++
>  gdb/testsuite/gdb.base/create-frame.exp | 36 +++++++++++++++++
>  gdb/testsuite/gdb.mi/mi-var-frame.c     | 34 ++++++++++++++++
>  gdb/testsuite/gdb.mi/mi-var-frame.exp   | 70 +++++++++++++++++++++++++++++++++
>  9 files changed, 250 insertions(+), 3 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/create-frame.c
>  create mode 100644 gdb/testsuite/gdb.base/create-frame.exp
>  create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.c
>  create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.exp
> 
> -- 
> 2.4.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: ABANDONED: [PATCH 0/2] frame_find_by_id & selected_frame issues
  2015-08-07 11:56 ` PING: [PATCH 0/2] frame_find_by_id & selected_frame issues Andrew Burgess
@ 2015-09-11 18:53   ` Andrew Burgess
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Burgess @ 2015-09-11 18:53 UTC (permalink / raw)
  To: gdb-patches

I've now merged these patches into a large patch series here:

  https://sourceware.org/ml/gdb-patches/2015-09/msg00248.html

Thanks,
Andrew


* Andrew Burgess <andrew.burgess@embecosm.com> [2015-08-07 13:56:46 +0200]:

> Ping!
> 
> Thanks,
> Andrew
> 
> 
> * Andrew Burgess <andrew.burgess@embecosm.com> [2015-07-23 12:07:32 +0100]:
> 
> > Seems that I originally failed to post patch 2/2 from the original
> > series.  So here's both of them again, rebased on latest master.
> > 
> > I believe that these patches should still be applied even in light of
> > the discussion here:
> >   https://sourceware.org/ml/gdb-patches/2015-07/msg00241.html
> > as any solution that does come out of that conversation will probably
> > be limited to changing the user interface of gdb, not the underlying
> > behaviour.  Also that conversation is about future clean up, while
> > this patch series addresses real, user visible, assertion failures.
> > 
> > --
> > 
> > Andrew Burgess (2):
> >   gdb: Check the selected-frame in frame_find_by_id.
> >   gdb: Select a frame for frame_info.
> > 
> >  gdb/ChangeLog                           | 13 ++++++
> >  gdb/frame.c                             | 39 ++++++++++++++++--
> >  gdb/frame.h                             |  5 +++
> >  gdb/stack.c                             | 10 +++++
> >  gdb/testsuite/ChangeLog                 | 12 ++++++
> >  gdb/testsuite/gdb.base/create-frame.c   | 34 ++++++++++++++++
> >  gdb/testsuite/gdb.base/create-frame.exp | 36 +++++++++++++++++
> >  gdb/testsuite/gdb.mi/mi-var-frame.c     | 34 ++++++++++++++++
> >  gdb/testsuite/gdb.mi/mi-var-frame.exp   | 70 +++++++++++++++++++++++++++++++++
> >  9 files changed, 250 insertions(+), 3 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.base/create-frame.c
> >  create mode 100644 gdb/testsuite/gdb.base/create-frame.exp
> >  create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.c
> >  create mode 100644 gdb/testsuite/gdb.mi/mi-var-frame.exp
> > 
> > -- 
> > 2.4.0
> > 

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-09-11 18:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-23 11:07 PING: [PATCH 0/2] frame_find_by_id & selected_frame issues Andrew Burgess
2015-07-08 16:46 ` [PATCH 1/2] gdb: Check the selected-frame in frame_find_by_id Andrew Burgess
2015-07-23 11:07   ` Andrew Burgess
2015-07-23 11:07   ` [PATCH 2/2] gdb: Select a frame for frame_info Andrew Burgess
2015-08-07 11:56 ` PING: [PATCH 0/2] frame_find_by_id & selected_frame issues Andrew Burgess
2015-09-11 18:53   ` ABANDONED: " Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).