Fix -var-update for registers in frames 1 and up

Message ID 1465335417-36881-1-git-send-email-donb@codesourcery.com
State New, archived
Headers

Commit Message

Don Breazeal June 7, 2016, 9:36 p.m. UTC
  This patch fixes a problem with using the MI -var-update command
to access the values of registers in frames other than the current
frame.  The patch includes a test that demonstrates the problem:

* run so there are several frames on the stack
* create a varobj for $pc in each frame, #'s 1 and above
* step one instruction, to modify the value of $pc
* call -var-update for each of the previously created varobjs
  to verify that they are not reported as having changed.

Without the patch, the -var-update command reported that $pc for all
frames 1 and above had changed to the value of $pc in frame 0.

The -var-create command takes a '--frame' argument and uses that
to select the frame for retrieving the register value.  But -var-update
has no such argument, and previously didn't do anything to select the
frame, so for frames other than frame 0 it returned the value of the
register for frame 0, instead of reporting the value as unchanged.

The solution implemented here is for varobj.c:varobj_update to
handle register varobjs by calling select_frame using the frame
associated with the varobj's value.  If the frame isn't found
varobj_update throws an error.

Thanks
--Don

gdb/testsuite/ChangeLog:
2016-06-07  Don Breazeal  <donb@codesourcery.com>

	* gdb.mi/mi-frame-regs.exp: New test.

gdb/ChangeLog:
2016-06-07  Don Breazeal  <donb@codesourcery.com>

	* varobj.c (varobj_update): If the varobj represents a register,
	select the frame associated with the varobj's value.

---
 gdb/testsuite/gdb.mi/mi-frame-regs.exp | 92 ++++++++++++++++++++++++++++++++++
 gdb/varobj.c                           | 13 +++++
 2 files changed, 105 insertions(+)
  

Comments

Andrew Burgess June 8, 2016, 1 p.m. UTC | #1
* Don Breazeal <donb@codesourcery.com> [2016-06-07 14:36:57 -0700]:

> This patch fixes a problem with using the MI -var-update command
> to access the values of registers in frames other than the current
> frame.  The patch includes a test that demonstrates the problem:
> 
> * run so there are several frames on the stack
> * create a varobj for $pc in each frame, #'s 1 and above
> * step one instruction, to modify the value of $pc
> * call -var-update for each of the previously created varobjs
>   to verify that they are not reported as having changed.
> 
> Without the patch, the -var-update command reported that $pc for all
> frames 1 and above had changed to the value of $pc in frame 0.
> 
> The -var-create command takes a '--frame' argument and uses that
> to select the frame for retrieving the register value.  But -var-update
> has no such argument, and previously didn't do anything to select the
> frame, so for frames other than frame 0 it returned the value of the
> register for frame 0, instead of reporting the value as unchanged.

This shouldn't need special handling for register varobj values, if I
create a varobj watching value 'foo' in frame 1, but have a local
'foo' in frame 0, a change in frame 0 'foo' will not trigger a change
for frame 1's 'foo' varobj.

The magic is actually in varobj.c:check_scope, which makes use of the
varobj->root->frame to select the right frame based on the type of the
varobj, this is setup at varobj creation time.

The problem, is that for register expressions the varobj->root->frame
is not set correctly.  This frame tracking is done using the global
'innermost_block' which is set in the various parser files (for
example c-exp.y), however, this is not set for register expressions.
I think that we probably should be doing this in
write_dollar_variable.

> 
> The solution implemented here is for varobj.c:varobj_update to
> handle register varobjs by calling select_frame using the frame
> associated with the varobj's value.  If the frame isn't found
> varobj_update throws an error.
>

<snip>

> 
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 6f56cba..e8f2e04 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -1610,6 +1610,19 @@ varobj_update (struct varobj **varp, int is_explicit)
>        r.varobj = *varp;
>        r.status = VAROBJ_IN_SCOPE;
>  
> +      /* If updating a register varobj, select the correct frame.  */
> +      if ((*varp)->root->exp != NULL
> +	  && (*varp)->root->exp->elts[0].opcode == OP_REGISTER)
> +	{
> +	  struct frame_info *fi;
> +
> +	  fi = frame_find_by_id (VALUE_FRAME_ID ((*varp)->value));
> +	  if (fi != NULL)
> +	    select_frame (fi);
> +	  else
> +	    error (_("Failed to find the frame for the specified register"));
> +	}
> +
>        /* Update the root variable.  value_of_root can return NULL
>  	 if the variable is no longer around, i.e. we stepped out of
>  	 the frame in which a local existed.  We are letting the 

This will break things if I create a floating varobj tracking (say)
$pc.  In that case I _do_ expect to pick up the $pc in the outermost
frame. Something like:

   -var-create --thread 1 --frame 1 - @ $pc
   next
   -var-update 1 *

Should show my variable as having changed I think.

Thanks,
Andrew
  

Patch

diff --git a/gdb/testsuite/gdb.mi/mi-frame-regs.exp b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
new file mode 100644
index 0000000..9ae0fb0
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,92 @@ 
+# Copyright 1999-2016 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/>.
+
+# Test essential Machine interface (MI) operations
+#
+# Verify that the pc register values for stack frames 1 and above are
+# not reported as changed by -var-update after the current pc changes.
+#
+
+global gdb_prompt
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if {[mi_gdb_start]} then {
+    continue
+}
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Create a varobj for the pc register in each of the frames other
+# than frame 0.
+
+proc var_create_regs {nframes} {
+    global hex
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-create --thread 1 --frame $i - \* \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame $i"
+    }
+}
+
+# Check that the value of the pc register for each of the 
+# frames 1 and above is not reported to have changed.
+
+proc var_update_regs {nframes} {
+
+    for {set i 1} {$i < $nframes} {incr i} {
+	mi_gdb_test "-var-update 1 var$i" \
+	            "\\^done,(changelist=\\\[\\\])" \
+	            "pc unchanged in -var-update for frame $i"
+    }
+}
+
+mi_run_to_main
+
+# Run to the function 'callee3'.
+set line_callee3_head  [gdb_get_line_number "callee3 ("]
+set line_callee3_body  [expr $line_callee3_head + 2]
+mi_create_breakpoint "basics.c:callee3" \
+		     "insert breakpoint at basics.c:callee3" \
+		     -number 2 -func callee3 -file ".*basics.c" \
+		     -line $line_callee3_body
+
+mi_execute_to "exec-continue" "breakpoint-hit" "callee3" ".*" ".*${srcfile}" "$line_callee3_body" \
+	      { "" "disp=\"keep\"" } "breakpoint hit"
+
+# At the breakpoint in callee3 there are 4 frames.  Create a varobj
+# for the pc in each of frames 1 and above.
+set nframes 4
+var_create_regs $nframes
+
+# Step one instruction to change the program counter.
+mi_execute_to "exec-next-instruction" "end-stepping-range" \
+	      "callee3" ".*" ".*${srcfile}" ".*" "" \
+	      "next instruction in callee3"
+
+# Make sure all is well with -var-update.
+var_update_regs $nframes
+
+mi_gdb_exit
+return 0
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 6f56cba..e8f2e04 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1610,6 +1610,19 @@  varobj_update (struct varobj **varp, int is_explicit)
       r.varobj = *varp;
       r.status = VAROBJ_IN_SCOPE;
 
+      /* If updating a register varobj, select the correct frame.  */
+      if ((*varp)->root->exp != NULL
+	  && (*varp)->root->exp->elts[0].opcode == OP_REGISTER)
+	{
+	  struct frame_info *fi;
+
+	  fi = frame_find_by_id (VALUE_FRAME_ID ((*varp)->value));
+	  if (fi != NULL)
+	    select_frame (fi);
+	  else
+	    error (_("Failed to find the frame for the specified register"));
+	}
+
       /* Update the root variable.  value_of_root can return NULL
 	 if the variable is no longer around, i.e. we stepped out of
 	 the frame in which a local existed.  We are letting the