Fix -var-update for registers in frames 1 and upusers/palves/fix-var-update-for-registers

XXX palves:

Watchpoints on locals are quite similar in principle with varobjs on
locals.  Likewise wathpoints/varobjs on registers.

So this actually makes "watch $pc" in non-current frames be a local
watchpoint.

So

 up
 watch $pc
 down
 continue

now does the "right" thing.  I'm not sure this is really right,
though.

Haven't done anything on the varobjs side.

gdb/testsuite/ChangeLog:
2016-06-13  Don Breazeal  <dbreazea@my.domain.org>

	* gdb.ada/mi_interface.exp: Add thread-id field to expected
	output of -var-create and -var-list-children.
	* gdb.ada/mi_var_array.exp: Add thread-id field to expected
	output of -var-list-children.
	* gdb.mi/mi-break.exp (test_error): Add thread-id field to
	expected output of -var-create.
	* gdb.mi/mi-frame-regs.exp: New test script.
	* gdb.mi/mi-var-cmd.exp: Add thread-id field to expected
	output of -var-create.
	* gdb.mi/mi-var-create-rtti.exp: Likewise.

gdb/ChangeLog:
2016-06-13  Don Breazeal  <donb@codesourcery.com>
	    Andrew Burgess <andrew.burgess@embecosm.com>

	* varobj.c (varobj_create): Initialize innermost_block to
	the global block instead of NULL.
	(new_root_variable): Initialize the thread_id and next
	fields.
	(value_of_root_1): Set within_scope if the variable's
	valid_block field is the global block.

8 files changed, 278 insertions, 34 deletions

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 6bb6bbfbef9..7ea55c2484f 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -1867,7 +1867,7 @@ update_watchpoint (struct watchpoint *b, int reparse)
   frame_saved = 0;
 
   /* Determine if the watchpoint is within scope.  */
-  if (b->exp_valid_block == NULL)
+  if (!frame_id_p (b->watchpoint_frame))
     within_current_scope = 1;
   else
     {
@@ -4196,7 +4196,7 @@ breakpoint_init_inferior (enum inf_context context)
 	  struct watchpoint *w = (struct watchpoint *) b;
 
 	  /* Likewise for watchpoints on local expressions.  */
-	  if (w->exp_valid_block != NULL)
+	  if (frame_id_p (w->watchpoint_frame))
 	    delete_breakpoint (b);
 	  else
 	    {
@@ -5145,7 +5145,7 @@ watchpoint_check (void *p)
   if (!watchpoint_in_thread_scope (b))
     return WP_IGNORE;
 
-  if (b->exp_valid_block == NULL)
+  if (!frame_id_p (b->watchpoint_frame))
     within_current_scope = 1;
   else
     {
@@ -5170,7 +5170,7 @@ watchpoint_check (void *p)
 
       /* If we've gotten confused in the unwinder, we might have
 	 returned a frame that can't describe this variable.  */
-      if (within_current_scope)
+      if (within_current_scope && b->exp_valid_block != NULL)
 	{
 	  struct symbol *function;
 
@@ -10529,13 +10529,16 @@ break_range_command (char *arg, int from_tty)
   update_global_location_list (UGLL_MAY_INSERT);
 }
 
+typedef int (for_each_exp_symbol_callback) (const struct expression *exp, int i, void *data);
+
 /*  Return non-zero if EXP is verified as constant.  Returned zero
     means EXP is variable.  Also the constant detection may fail for
     some constant expressions and in such case still falsely return
     zero.  */
 
 static int
-watchpoint_exp_is_const (const struct expression *exp)
+for_each_expression_symbol (const struct expression *exp,
+			    for_each_exp_symbol_callback *cb, void *cb_data)
 {
   int i = exp->nelts;
 
@@ -10613,34 +10616,86 @@ watchpoint_exp_is_const (const struct expression *exp)
 	  break;
 
 	case OP_VAR_VALUE:
-	  /* Check whether the associated symbol is a constant.
-
-	     We use SYMBOL_CLASS rather than TYPE_CONST because it's
-	     possible that a buggy compiler could mark a variable as
-	     constant even when it is not, and TYPE_CONST would return
-	     true in this case, while SYMBOL_CLASS wouldn't.
-
-	     We also have to check for function symbols because they
-	     are always constant.  */
-	  {
-	    struct symbol *s = exp->elts[i + 2].symbol;
-
-	    if (SYMBOL_CLASS (s) != LOC_BLOCK
-		&& SYMBOL_CLASS (s) != LOC_CONST
-		&& SYMBOL_CLASS (s) != LOC_CONST_BYTES)
-	      return 0;
-	    break;
-	  }
+	  if (cb (exp, i, cb_data))
+	    return 1;
+	  break;
 
 	/* The default action is to return 0 because we are using
 	   the optimistic approach here: If we don't know something,
 	   then it is not a constant.  */
 	default:
-	  return 0;
+	  return 1;
 	}
     }
 
-  return 1;
+  return 0;
+}
+
+static int
+exp_element_symbol_is_const (const struct expression *exp, int i,
+			     void *cb_data)
+{
+  if (exp->elts[i].opcode == OP_VAR_VALUE)
+    {
+      /* Check whether the associated symbol is a constant.
+
+	 We use SYMBOL_CLASS rather than TYPE_CONST because it's
+	 possible that a buggy compiler could mark a variable as
+	 constant even when it is not, and TYPE_CONST would return
+	 true in this case, while SYMBOL_CLASS wouldn't.
+
+	 We also have to check for function symbols because they
+	 are always constant.  */
+      struct symbol *s = exp->elts[i + 2].symbol;
+
+      if (SYMBOL_CLASS (s) != LOC_BLOCK
+	  && SYMBOL_CLASS (s) != LOC_CONST
+	  && SYMBOL_CLASS (s) != LOC_CONST_BYTES)
+	{
+	  /* Stop looking.  */
+	  return 1;
+	}
+    }
+
+  return 0;
+}
+
+static int
+watchpoint_exp_is_const (struct expression *exp)
+{
+  if (!for_each_expression_symbol (exp, exp_element_symbol_is_const, NULL))
+    return 1;
+  return 0;
+}
+
+
+static int
+exp_element_symbol_needs_frame (const struct expression *exp, int i,
+				void *cb_data)
+{
+  switch (exp->elts[i].opcode)
+    {
+    case OP_VAR_VALUE:
+      {
+	struct symbol *s = exp->elts[i + 2].symbol;
+
+	if (symbol_read_needs_frame (s))
+	  return 1;
+	break;
+      }
+    case OP_REGISTER:
+      return 1;
+    }
+
+  return 0;
+ }
+
+static int
+expression_needs_frame (const struct expression *exp)
+{
+  if (for_each_expression_symbol (exp, exp_element_symbol_needs_frame, NULL))
+    return 1;
+  return 0;
 }
 
 /* Implement the "dtor" breakpoint_ops method for watchpoints.  */
@@ -11331,13 +11386,16 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
   if (*tok)
     error (_("Junk at end of command."));
 
-  frame = block_innermost_frame (exp_valid_block);
+  if (exp_valid_block == NULL && expression_needs_frame (exp))
+    frame = get_selected_frame (NULL);
+  else
+    frame = block_innermost_frame (exp_valid_block);
 
   /* If the expression is "local", then set up a "watchpoint scope"
      breakpoint at the point where we've left the scope of the watchpoint
      expression.  Create the scope breakpoint before the watchpoint, so
      that we will encounter it first in bpstat_stop_status.  */
-  if (exp_valid_block && frame)
+  if (frame != NULL)
     {
       if (frame_id_p (frame_unwind_caller_id (frame)))
 	{
diff --git a/gdb/testsuite/gdb.ada/mi_interface.exp b/gdb/testsuite/gdb.ada/mi_interface.exp
index 6000ec862a1..b948cd5a727 100644
--- a/gdb/testsuite/gdb.ada/mi_interface.exp
+++ b/gdb/testsuite/gdb.ada/mi_interface.exp
@@ -44,9 +44,9 @@ mi_continue_to_line \
     "stop at start of main Ada procedure"
 
 mi_gdb_test "-var-create ggg1 * ggg1" \
-    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",has_more=\"0\"" \
+    "\\^done,name=\"ggg1\",numchild=\"1\",value=\"{...}\",type=\"<ref> pck.gadatatype\",thread-id=\"1\",has_more=\"0\"" \
     "Create ggg1 varobj"
 
 mi_gdb_test "-var-list-children 1 ggg1" \
-    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\"}\\\],has_more=\"0\"" \
+    "\\^done,numchild=\"1\",children=\\\[child={name=\"ggg1.i\",exp=\"i\",numchild=\"0\",value=\"42\",type=\"integer\",thread-id=\"1\"}\\\],has_more=\"0\"" \
     "list ggg1's children"
diff --git a/gdb/testsuite/gdb.ada/mi_var_array.exp b/gdb/testsuite/gdb.ada/mi_var_array.exp
index c648e7e02ef..c02d4c93ce3 100644
--- a/gdb/testsuite/gdb.ada/mi_var_array.exp
+++ b/gdb/testsuite/gdb.ada/mi_var_array.exp
@@ -48,5 +48,5 @@ mi_gdb_test "-var-create vta * vta" \
     "Create bt varobj"
 
 mi_gdb_test "-var-list-children vta" \
-    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\"}\\\],.*" \
+    "\\^done,numchild=\"2\",children=\\\[child={name=\"vta.n\",exp=\"n\",numchild=\"0\",type=\"bar\\.int\",thread-id=\"1\"},child={name=\"vta.f\",exp=\"f\",numchild=\"0\",type=\"array \\(1 .. n\\) of character\",thread-id=\"1\"}\\\],.*" \
     "list vta's children"
diff --git a/gdb/testsuite/gdb.mi/mi-break.exp b/gdb/testsuite/gdb.mi/mi-break.exp
index 00293d7f443..3a5dd69e42a 100644
--- a/gdb/testsuite/gdb.mi/mi-break.exp
+++ b/gdb/testsuite/gdb.mi/mi-break.exp
@@ -204,7 +204,7 @@ proc test_error {} {
     # containing function call, the internal breakpoint created to handle
     # function call would be reported, messing up MI output.
     mi_gdb_test "-var-create V * return_1()" \
-        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",has_more=\"0\"" \
+        "\\^done,name=\"V\",numchild=\"0\",value=\"1\",type=\"int\",thread-id=\"1\",has_more=\"0\"" \
         "create varobj for function call"
 
     mi_gdb_test "-var-update *" \
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 00000000000..45f81d6b7f7
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-frame-regs.exp
@@ -0,0 +1,183 @@
+# 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 -var-update will provide the correct values for floating
+# and fixed varobjs that represent the pc register.
+#
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+standard_testfile basics.c
+
+if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
+		 executable {debug}] != "" } then {
+     untested mi-frame-regs.exp
+     return -1
+}
+
+# Return the address of the specified breakpoint.
+
+proc breakpoint_address {bpnum} {
+    global hex
+    global expect_out
+    global mi_gdb_prompt
+
+    send_gdb "info breakpoint $bpnum\n"
+    gdb_expect {
+	-re ".*($hex).*$mi_gdb_prompt$" {
+	    return $expect_out(1,string)
+	}
+	-re ".*$mi_gdb_prompt$" {
+	    return ""
+	}
+	timeout {
+	    return ""
+	}
+    }
+}
+
+# Test that a floating varobj representing $pc will provide the
+# correct value via -var-update as the program stops at
+# breakpoints in different functions.
+
+proc do_floating_varobj_test {} {
+    global srcfile
+    global hex
+    global expect_out
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "floating" {
+	mi_run_to_main
+
+	# Create a floating varobj for $pc.
+	mi_gdb_test "-var-create --thread 1 --frame 0 - @ \$pc" \
+		    "\\^done,.*value=\"$hex.*" \
+	            "create varobj for pc in frame 0"
+
+	set nframes 4
+	for {set i 1} {$i < $nframes} {incr i} {
+
+	    # Run to a breakpoint in each callee function in succession.
+	    # Note that we can't use mi_runto because we need the
+	    # breakpoint to be persistent, so we can use its address.
+	    set bpnum [expr $i + 1]
+	    mi_create_breakpoint \
+	        "basics.c:callee$i" \
+		"insert breakpoint at basics.c:callee$i" \
+		-number $bpnum -func callee$i -file ".*basics.c"
+
+	    mi_execute_to "exec-continue" "breakpoint-hit" \
+			  "callee$i" ".*" ".*${srcfile}" ".*" \
+			  { "" "disp=\"keep\"" } "breakpoint hit"
+
+	    # Get the value of $pc from the floating varobj.
+	    mi_gdb_test "-var-update 1 var1" \
+			"\\^done,.*value=\"($hex) .*" \
+			"-var-update for frame $i"
+	    set pcval $expect_out(3,string)
+
+	    # Get the address of the current breakpoint.
+	    set bpaddr [breakpoint_address $bpnum]
+	    if {$bpaddr == ""} then {
+		unresolved "get address of breakpoint $bpnum"
+		return
+	    }
+
+	    # Check that the addresses are the same.
+	    if {[expr $bpaddr != $pcval]} then {
+	        fail "\$pc does not equal address of breakpoint"
+	    } else {
+	        pass "\$pc equals address of breakpoint"
+	    }
+	}
+    }
+}
+
+# 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 -var-update reports that the value of the pc register
+# for each of the frames 1 and above is reported as unchanged.
+
+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"
+    }
+}
+
+# Test that fixed varobjs representing $pc in different stack frames
+# will provide the correct value via -var-update after the program
+# counter changes (without substantially changing the stack).
+
+proc do_fixed_varobj_test {} {
+    global srcfile
+
+    gdb_exit
+    if {[mi_gdb_start]} then {
+	continue
+    }
+
+    with_test_prefix "fixed" {
+	mi_run_to_main
+
+	# Run to the function 'callee3' so we have several frames.
+	mi_create_breakpoint "basics.c:callee3" \
+			     "insert breakpoint at basics.c:callee3" \
+			     -number 2 -func callee3 -file ".*basics.c"
+
+	mi_execute_to "exec-continue" "breakpoint-hit" \
+	              "callee3" ".*" ".*${srcfile}" ".*" \
+		      { "" "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"
+
+	# Check that -var-update reports that the values are unchanged.
+	var_update_regs $nframes
+    }
+}
+
+do_fixed_varobj_test
+do_floating_varobj_test
+
+mi_gdb_exit
+return 0
diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
index 558cd6c5654..68e3cf87e12 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
@@ -381,7 +381,7 @@ mi_gdb_test "-var-update *" \
 	"assign same value to func (update)"
 
 mi_gdb_test "-var-create array_ptr * array_ptr" \
-	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",has_more=\"0\"" \
+	"\\^done,name=\"array_ptr\",numchild=\"1\",value=\"$hex <array>\",type=\"int \\*\",thread-id=\"1\",has_more=\"0\"" \
 	"create global variable array_ptr"
 
 mi_gdb_test "-var-assign array_ptr array2" \
@@ -608,7 +608,7 @@ mi_check_varobj_value F 7 "check F inside callee"
 # A varobj we fail to read during -var-update should be considered
 # out of scope.
 mi_gdb_test "-var-create null_ptr * **0" \
-    {\^done,name="null_ptr",numchild="0",value=".*",type="int",has_more="0"} \
+    {\^done,name="null_ptr",numchild="0",value=".*",type="int",thread-id="1"has_more="0"} \
     "create null_ptr"
 
 # Allow this to succeed, if address zero is readable, although it
diff --git a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
index 3bcb36cdcce..a8cc76f7913 100644
--- a/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-create-rtti.exp
@@ -49,6 +49,6 @@ mi_gdb_test "-gdb-set print object on" ".*"
 # We use a explicit cast to (void *) as that is the
 # type that caused the bug this testcase is testing for.
 mi_gdb_test "-var-create sp1 * ((void*)\$sp)" \
-	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",has_more=\"0\"" \
+	    "\\^done,name=\"sp1\",numchild=\"0\",value=\"$hex\",type=\"void \\*\",thread-id=\"1\",has_more=\"0\"" \
 	    "-var-create sp1 * \$sp"
 gdb_exit
diff --git a/gdb/varobj.c b/gdb/varobj.c
index fb1349a85db..7cab707da8d 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -323,6 +323,7 @@ varobj_create (char *objname,
 	}
 
       p = expression;
+      //      innermost_block = block_global_block (block);
       innermost_block = NULL;
       /* Wrap the call to parse expression, so we can 
          return a sensible error.  */
@@ -2103,6 +2104,8 @@ new_root_variable (void)
   var->root->floating = 0;
   var->root->rootvar = NULL;
   var->root->is_valid = 1;
+  var->root->thread_id = 0;
+  var->root->next = NULL;
 
   return var;
 }