Re: [RFA] Have block_innermost_frame start from selected frame

[Paul Hilfinger <Hilfinger@adacore.com>]

[-- Attachment #1: Type: text/plain, Size: 2814 bytes --]

This is an update of my previous patch to address a problem that Eli
uncovered: references to BLOCK:LOCAL_VARIABLE did not work in watched
expressions.  In fact, due to a bug, a local variable referred to with
BLOCK:VAR has always been handled incorrectly in 'watch', even if it
is interpreted to refer to the innermost frame associated with BLOCK,
because no breakpoint would be set to remove the watch when the
variable's frame was deallocated.

Eli, your actual comment was a question about whether the syntax
applied to watch (given that all the examples used print).  On
consideration, I decided to keep things as they are, because if the
issue is that it is unclear where variables may be used, it is a
problem that extends to the entire chapter on "Examining Data", and
should be addressed by a rather larger editorial assault.

GDB used to search for the frame containing variables in a particular
lexical block starting from the current (top) frame, ignoring any
currently selected frame.  It is not clear why this is desirable for
variables that require a frame; why would a user deliberately select
one frame and then expect to see the value of a variable in a more
recent frame?  This change causes block_innermost_frame to start
looking from the selected frame, if there is one.

This change is perhaps unnecessarily conservative.  It uses
get_selected_frame_if_set rather than get_selected_frame in order to
avoid the side effect of calling select_frame, which would probably be
harmless.

Second, expression-parsing routines previously made the unwarranted assumption
that all block-qualified variables (written with the GDB extension
<block>::<variable>) are static.  As a result, they failed to update
innermost_block, which confused the watch commands about when variables in
watched expressions went out of scope, and also caused the wrong variables to
be watched.  This change modifies these routines to treat all local variables
the same whether or not they are block-qualified.

Finally, we add a paragraph to the "Program Variables" section of the texinfo
documentation concerning the use of "::" for accessing non-static variables.

Paul N. Hilfinger
(Hilfinger@adacore.com)


2011-12-27  Paul Hilfinger  <hilfingr@adacore.com>

  * gdb/blockframe.c (block_innermost_frame): Start search from selected frame,
    if present, or otherwise the current frame.

  * gdb/c-exp.y, gdb/m2-exp.y, gdb/objc-exp.y: Update innermost_block

  * gdb/doc/gdb.texinfo (Variables): Document use of :: for non-static
    variables.
---
 gdb/blockframe.c    |    9 ++++++---
 gdb/c-exp.y         |    7 +++++++
 gdb/doc/gdb.texinfo |   44 ++++++++++++++++++++++++++++++++++++++++++--
 gdb/m2-exp.y        |    7 +++++++
 gdb/objc-exp.y      |    7 +++++++
 5 files changed, 69 insertions(+), 5 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Have-block_innermost_frame-start-from-selected-frame.patch --]
[-- Type: text/x-patch; name="0001-Have-block_innermost_frame-start-from-selected-frame.patch", Size: 4908 bytes --]

diff --git a/gdb/blockframe.c b/gdb/blockframe.c
index 3897366..1917b68 100644
--- a/gdb/blockframe.c
+++ b/gdb/blockframe.c
@@ -352,8 +352,9 @@ find_pc_partial_function (CORE_ADDR pc, char **name, CORE_ADDR *address,
   return find_pc_partial_function_gnu_ifunc (pc, name, address, endaddr, NULL);
 }
 
-/* Return the innermost stack frame executing inside of BLOCK, or NULL
-   if there is no such frame.  If BLOCK is NULL, just return NULL.  */
+/* Return the innermost stack frame that is executing inside of BLOCK and is
+ * at least as old as the selected frame. Return NULL if there is no
+ * such frame.  If BLOCK is NULL, just return NULL.  */
 
 struct frame_info *
 block_innermost_frame (const struct block *block)
@@ -368,7 +369,9 @@ block_innermost_frame (const struct block *block)
   start = BLOCK_START (block);
   end = BLOCK_END (block);
 
-  frame = get_current_frame ();
+  frame = get_selected_frame_if_set ();
+  if (frame == NULL)
+    frame = get_current_frame ();
   while (frame != NULL)
     {
       struct block *frame_block = get_frame_block (frame, NULL);
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index bdcae33..5b57f24 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -778,6 +778,13 @@ variable:	block COLONCOLON name
 			  if (sym == 0)
 			    error (_("No symbol \"%s\" in specified context."),
 				   copy_name ($3));
+			  if (symbol_read_needs_frame (sym))
+			    {
+			      if (innermost_block == 0
+				  || contained_in (block_found, 
+						   innermost_block))
+				innermost_block = block_found;
+			    }
 
 			  write_exp_elt_opcode (OP_VAR_VALUE);
 			  /* block_found is set by lookup_symbol.  */
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 2f4aa4f..d1a2dde 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -7306,7 +7306,7 @@ scope is a single source file even if the current execution point is not
 in this file.  But it is possible to have more than one such variable or
 function with the same name (in different source files).  If that
 happens, referring to that name has unpredictable effects.  If you wish,
-you can specify a static variable in a particular function or file,
+you can specify a static variable in a particular function or file by
 using the colon-colon (@code{::}) notation:
 
 @cindex colon-colon, context for variables/functions
@@ -7329,8 +7329,48 @@ to print a global value of @code{x} defined in @file{f2.c}:
 (@value{GDBP}) p 'f2.c'::x
 @end smallexample
 
+The @code{::} notation is normally used for referring to
+static variables, since you typically disambiguate uses of local variables
+in functions by selecting the appropriate frame and using the
+simple name of the variable.  However, you may also use this notation
+to refer to local variables in frames enclosing the selected frame:
+
+@smallexample
+void
+foo (int a)
+@{
+  if (a < 10)
+      bar (a);
+  else
+      process (a);    /* Stop here */
+@}
+
+int
+bar (int a)
+@{
+  foo (a + 5);
+@}
+@end smallexample
+
+@noindent
+If you have a breakpoint at the commented line
+when the program executes the call @code{bar(0)}, then when the program
+stops, the commands
+
+@smallexample
+(@value{GDBP}) p a
+(@value{GDBP}) p bar::a
+(@value{GDBP}) up 2
+(@value{GDBP}) p a
+(@value{GDBP}) p bar::a
+@end smallexample
+
+@noindent
+will print the values @samp{10}, @samp{5}, @samp{5}, and @samp{0} in that
+order.
+
 @cindex C@t{++} scope resolution
-This use of @samp{::} is very rarely in conflict with the very similar
+These uses of @samp{::} are very rarely in conflict with the very similar
 use of the same notation in C@t{++}.  @value{GDBN} also supports use of the C@t{++}
 scope resolution operator in @value{GDBN} expressions.
 @c FIXME: Um, so what happens in one of those rare cases where it's in
diff --git a/gdb/m2-exp.y b/gdb/m2-exp.y
index 1e3e3cb..d59678b 100644
--- a/gdb/m2-exp.y
+++ b/gdb/m2-exp.y
@@ -588,6 +588,13 @@ variable:	block COLONCOLON NAME
 			  if (sym == 0)
 			    error (_("No symbol \"%s\" in specified context."),
 				   copy_name ($3));
+			  if (symbol_read_needs_frame (sym))
+			    {
+			      if (innermost_block == 0
+				  || contained_in (block_found, 
+						   innermost_block))
+				innermost_block = block_found;
+			    }
 
 			  write_exp_elt_opcode (OP_VAR_VALUE);
 			  /* block_found is set by lookup_symbol.  */
diff --git a/gdb/objc-exp.y b/gdb/objc-exp.y
index 346b404..00e5e7f 100644
--- a/gdb/objc-exp.y
+++ b/gdb/objc-exp.y
@@ -648,6 +648,13 @@ variable:	block COLONCOLON name
 			  if (sym == 0)
 			    error (_("No symbol \"%s\" in specified context."),
 				   copy_name ($3));
+			  if (symbol_read_needs_frame (sym))
+			    {
+			      if (innermost_block == 0
+				  || contained_in (block_found, 
+						   innermost_block))
+				innermost_block = block_found;
+			    }
 
 			  write_exp_elt_opcode (OP_VAR_VALUE);
 			  /* block_found is set by lookup_symbol.  */