aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVladimir Prus <vladimir@codesourcery.com>2006-11-28 17:23:10 +0000
committerVladimir Prus <vladimir@codesourcery.com>2006-11-28 17:23:10 +0000
commitacd65feb7ed6511f571de5b5ec7f2dd759819fe5 (patch)
treee0a7f98afb50eae5c17ae270bc31974c9e384697
parent74ca34cea9f2a3521ecb6b58f57bb799cd19b3ae (diff)
downloadgdb-acd65feb7ed6511f571de5b5ec7f2dd759819fe5.zip
gdb-acd65feb7ed6511f571de5b5ec7f2dd759819fe5.tar.gz
gdb-acd65feb7ed6511f571de5b5ec7f2dd759819fe5.tar.bz2
2006-11-28 Vladimir Prus <vladimir@codesourcery.com>
Fetch varobj values from memory in a single place, and only fetch the values that are really needed. * varobj.c (struct varobj): Clarify comment. (my_value_equal): Remove. (install_new_value): New function. (type_of_child): Remove. (varobj_create): Use install_new_value. (varobj_set_value): Use value_contents_equal, not my_value_equal. (varobj_update): Use install_new_value. (create_child): Likewise. Inline type_of_child here. (value_of_child): Don't fetch the value. (c_value_of_root): Likewise. (c_value_of_variable): Likewise. (type_changeable): Improve comments.
-rw-r--r--gdb/ChangeLog18
-rw-r--r--gdb/testsuite/ChangeLog7
-rw-r--r--gdb/testsuite/gdb.mi/mi-var-cmd.exp37
-rw-r--r--gdb/testsuite/gdb.mi/var-cmd.c4
-rw-r--r--gdb/varobj.c293
5 files changed, 235 insertions, 124 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3c8f9d9..c1c6026 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,21 @@
+2006-11-28 Vladimir Prus <vladimir@codesourcery.com>
+
+ Fetch varobj values from memory in a single place,
+ and only fetch the values that are really needed.
+ * varobj.c (struct varobj): Clarify comment.
+ (my_value_equal): Remove.
+ (install_new_value): New function.
+ (type_of_child): Remove.
+ (varobj_create): Use install_new_value.
+ (varobj_set_value): Use value_contents_equal, not
+ my_value_equal.
+ (varobj_update): Use install_new_value.
+ (create_child): Likewise. Inline type_of_child here.
+ (value_of_child): Don't fetch the value.
+ (c_value_of_root): Likewise.
+ (c_value_of_variable): Likewise.
+ (type_changeable): Improve comments.
+
2006-11-28 Daniel Jacobowitz <dan@codesourcery.com>
* remote.c (struct remote_arch_state): Doc fix.
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 40685ea..c6550e4 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2006-11-28 Vladimir Prus <vladimir@codesourcery.com>
+
+ * gdb.mi/mi-var-cmd.exp: Check -var-update after
+ assignement of arrays and function pointers.
+ * gdb.mi/var-cmd.c: Add declaration necessary for above
+ tests.
+
2006-11-27 Nathan Sidwell <nathan@codesourcery.com>
* gdb.base/break.c (main): Call malloc.
diff --git a/gdb/testsuite/gdb.mi/mi-var-cmd.exp b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
index a6023e9..7efb438 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cmd.exp
@@ -382,6 +382,43 @@ mi_gdb_test "-var-assign lsimple.integer 333" \
"\\^done,value=\"333\"" \
"assign to lsimple.integer"
+mi_gdb_test "-var-update *" \
+ "\\^done,changelist=.*" \
+ "var update"
+
+# Check that assignment of function and array values
+# promotes the assigned value to function pointer/data
+# pointer before comparing with the existing value,
+# and does not incorrectly make the value as changed.
+mi_gdb_test "-var-assign func do_block_tests" \
+ "\\^done,value=\"$hex <do_block_tests>\"" \
+ "assign same value to func"
+
+mi_gdb_test "-var-update *" \
+ "\\^done,changelist=\\\[\\\]" \
+ "assign same value to func (update)"
+
+mi_gdb_test "-var-create array_ptr * array_ptr" \
+ "\\^done,name=\"array_ptr\",numchild=\"1\",type=\"int \\*\"" \
+ "create global variable array_ptr"
+
+mi_gdb_test "-var-assign array_ptr array2" \
+ "\\^done,value=\"$hex\"" \
+ "assign array to pointer"
+
+mi_gdb_test "-var-update *" \
+ "\\^done,changelist=\\\[\{name=\"array_ptr\",in_scope=\"true\",type_changed=\"false\"\}\\\]" \
+ "assign array to pointer (update)"
+
+mi_gdb_test "-var-assign array_ptr array2" \
+ "\\^done,value=\"$hex\"" \
+ "assign same array to pointer"
+
+mi_gdb_test "-var-update *" \
+ "\\^done,changelist=\\\[\\\]" \
+ "assign same array to pointer (update)"
+
+
######
# End of assign tests
#####
diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
index 761e804..d0eb6f8 100644
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -106,6 +106,10 @@ void incr_a (char a)
b = a;
}
+int array[] = {1,2,3};
+int array2[] = {4,5,6};
+int *array_ptr = array;
+
void
do_locals_tests ()
{
diff --git a/gdb/varobj.c b/gdb/varobj.c
index c664bfd..42ed597 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -101,7 +101,9 @@ struct varobj
/* The type of this variable. This may NEVER be NULL. */
struct type *type;
- /* The value of this expression or subexpression. This may be NULL. */
+ /* The value of this expression or subexpression. This may be NULL.
+ Invariant: if type_changeable (this) is non-zero, the value is either
+ NULL, or not lazy. */
struct value *value;
/* Did an error occur evaluating the expression or getting its value? */
@@ -202,8 +204,6 @@ static struct type *get_target_type (struct type *);
static enum varobj_display_formats variable_default_display (struct varobj *);
-static int my_value_equal (struct value *, struct value *, int *);
-
static void vpush (struct vstack **pstack, struct varobj *var);
static struct varobj *vpop (struct vstack **pstack);
@@ -212,6 +212,9 @@ static void cppush (struct cpstack **pstack, char *name);
static char *cppop (struct cpstack **pstack);
+static int install_new_value (struct varobj *var, struct value *value,
+ int initial);
+
/* Language-specific routines. */
static enum varobj_languages variable_language (struct varobj *var);
@@ -226,8 +229,6 @@ static struct value *value_of_root (struct varobj **var_handle, int *);
static struct value *value_of_child (struct varobj *parent, int index);
-static struct type *type_of_child (struct varobj *var);
-
static int variable_editable (struct varobj *var);
static char *my_value_of_variable (struct varobj *var);
@@ -445,6 +446,7 @@ varobj_create (char *objname,
{
char *p;
enum varobj_languages lang;
+ struct value *value;
/* Parse and evaluate the expression, filling in as much
of the variable's data as possible */
@@ -505,17 +507,16 @@ varobj_create (char *objname,
/* We definitively need to catch errors here.
If evaluate_expression succeeds we got the value we wanted.
But if it fails, we still go on with a call to evaluate_type() */
- if (gdb_evaluate_expression (var->root->exp, &var->value))
- {
- /* no error */
- release_value (var->value);
- if (value_lazy (var->value))
- gdb_value_fetch_lazy (var->value);
- }
- else
- var->value = evaluate_type (var->root->exp);
+ if (!gdb_evaluate_expression (var->root->exp, &value))
+ /* Error getting the value. Try to at least get the
+ right type. */
+ value = evaluate_type (var->root->exp);
+
+ release_value (value);
+
+ var->type = value_type (value);
- var->type = value_type (var->value);
+ install_new_value (var, value, 1 /* Initial assignment */);
/* Set language info */
lang = variable_language (var);
@@ -825,8 +826,28 @@ varobj_set_value (struct varobj *var, char *expression)
return 0;
}
- if (!my_value_equal (var->value, value, &error))
+ /* All types that are editable must also be changeable. */
+ gdb_assert (type_changeable (var));
+
+ /* The value of a changeable variable object must not be lazy. */
+ gdb_assert (!value_lazy (var->value));
+
+ /* Need to coerce the input. We want to check if the
+ value of the variable object will be different
+ after assignment, and the first thing value_assign
+ does is coerce the input.
+ For example, if we are assigning an array to a pointer variable we
+ should compare the pointer with the the array's address, not with the
+ array's content. */
+ value = coerce_array (value);
+
+ if (!value_contents_equal (var->value, value))
var->updated = 1;
+
+ /* The new value may be lazy. gdb_value_assign, or
+ rather value_contents, will take care of this.
+ If fetching of the new value will fail, gdb_value_assign
+ with catch the exception. */
if (!gdb_value_assign (var->value, value, &val))
return 0;
value_free (var->value);
@@ -870,6 +891,101 @@ varobj_list (struct varobj ***varlist)
return rootcount;
}
+/* Assign a new value to a variable object. If INITIAL is non-zero,
+ this is the first assignement after the variable object was just
+ created, or changed type. In that case, just assign the value
+ and return 0.
+ Otherwise, assign the value and if type_changeable returns non-zero,
+ find if the new value is different from the current value.
+ Return 1 if so, and 0 if the values are equal. */
+static int
+install_new_value (struct varobj *var, struct value *value, int initial)
+{
+ int changeable;
+ int need_to_fetch;
+ int changed = 0;
+
+ var->error = 0;
+ /* We need to know the varobj's type to decide if the value should
+ be fetched or not. C++ fake children (public/protected/private) don't have
+ a type. */
+ gdb_assert (var->type || CPLUS_FAKE_CHILD (var));
+ changeable = type_changeable (var);
+ need_to_fetch = changeable;
+
+ if (var->type && TYPE_CODE (var->type) == TYPE_CODE_UNION)
+ /* For unions, we need to fetch the value implicitly because
+ of implementation of union member fetch. When gdb
+ creates a value for a field and the value of the enclosing
+ structure is not lazy, it immediately copies the necessary
+ bytes from the enclosing values. If the enclosing value is
+ lazy, the call to value_fetch_lazy on the field will read
+ the data from memory. For unions, that means we'll read the
+ same memory more than once, which is not desirable. So
+ fetch now. */
+ need_to_fetch = 1;
+
+ /* The new value might be lazy. If the type is changeable,
+ that is we'll be comparing values of this type, fetch the
+ value now. Otherwise, on the next update the old value
+ will be lazy, which means we've lost that old value. */
+ if (need_to_fetch && value && value_lazy (value))
+ {
+ if (!gdb_value_fetch_lazy (value))
+ {
+ var->error = 1;
+ /* Set the value to NULL, so that for the next -var-update,
+ we don't try to compare the new value with this value,
+ that we couldn't even read. */
+ value = NULL;
+ }
+ else
+ var->error = 0;
+ }
+
+ /* If the type is changeable, compare the old and the new values.
+ If this is the initial assignment, we don't have any old value
+ to compare with. */
+ if (!initial && changeable)
+ {
+ /* If the value of the varobj was changed by -var-set-value, then the
+ value in the varobj and in the target is the same. However, that value
+ is different from the value that the varobj had after the previous
+ -var-update. So need to the varobj as changed. */
+ if (var->updated)
+ changed = 1;
+ else
+ {
+ /* Try to compare the values. That requires that both
+ values are non-lazy. */
+
+ /* Quick comparison of NULL values. */
+ if (var->value == NULL && value == NULL)
+ /* Equal. */
+ ;
+ else if (var->value == NULL || value == NULL)
+ changed = 1;
+ else
+ {
+ gdb_assert (!value_lazy (var->value));
+ gdb_assert (!value_lazy (value));
+
+ if (!value_contents_equal (var->value, value))
+ changed = 1;
+ }
+ }
+ }
+
+ /* We must always keep the new value, since children depend on it. */
+ if (var->value != NULL)
+ value_free (var->value);
+ var->value = value;
+ var->updated = 0;
+
+ return changed;
+}
+
+
/* Update the values for a variable and its children. This is a
two-pronged attack. First, re-parse the value for the root's
expression to see if it's changed. Then go all the way
@@ -939,24 +1055,16 @@ varobj_update (struct varobj **varp, struct varobj ***changelist)
vpush (&result, *varp);
changed++;
}
- /* If values are not equal, note that it's changed.
- There a couple of exceptions here, though.
- We don't want some types to be reported as "changed". */
- else if (type_changeable (*varp) &&
- ((*varp)->updated || !my_value_equal ((*varp)->value, new, &error)))
+
+ if (install_new_value ((*varp), new, type_changed))
{
- vpush (&result, *varp);
- (*varp)->updated = 0;
+ /* If type_changed is 1, install_new_value will never return
+ non-zero, so we'll never report the same variable twice. */
+ gdb_assert (!type_changed);
+ vpush (&result, (*varp));
changed++;
- /* Its value is going to be updated to NEW. */
- (*varp)->error = error;
}
- /* We must always keep around the new value for this root
- variable expression, or we lose the updated children! */
- value_free ((*varp)->value);
- (*varp)->value = new;
-
/* Initialize a stack */
vpush (&stack, NULL);
@@ -982,21 +1090,13 @@ varobj_update (struct varobj **varp, struct varobj ***changelist)
/* Update this variable */
new = value_of_child (v->parent, v->index);
- if (type_changeable (v) &&
- (v->updated || !my_value_equal (v->value, new, &error)))
- {
+ if (install_new_value (v, new, 0 /* type not changed */))
+ {
/* Note that it's changed */
vpush (&result, v);
v->updated = 0;
changed++;
}
- /* Its value is going to be updated to NEW. */
- v->error = error;
-
- /* We must always keep new values, since children depend on it. */
- if (v->value != NULL)
- value_free (v->value);
- v->value = new;
/* Get next child */
v = vpop (&stack);
@@ -1257,15 +1357,14 @@ create_child (struct varobj *parent, int index, char *name)
{
struct varobj *child;
char *childs_name;
+ struct value *value;
child = new_variable ();
/* name is allocated by name_of_child */
child->name = name;
child->index = index;
- child->value = value_of_child (parent, index);
- if ((!CPLUS_FAKE_CHILD (child) && child->value == NULL) || parent->error)
- child->error = 1;
+ value = value_of_child (parent, index);
child->parent = parent;
child->root = parent->root;
childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
@@ -1275,8 +1374,20 @@ create_child (struct varobj *parent, int index, char *name)
/* Save a pointer to this child in the parent */
save_child_in_parent (parent, child);
- /* Note the type of this child */
- child->type = type_of_child (child);
+ /* Compute the type of the child. Must do this before
+ calling install_new_value. */
+ if (value != NULL)
+ /* If the child had no evaluation errors, var->value
+ will be non-NULL and contain a valid type. */
+ child->type = value_type (value);
+ else
+ /* Otherwise, we must compute the type. */
+ child->type = (*child->root->lang->type_of_child) (child->parent,
+ child->index);
+ install_new_value (child, value, 1);
+
+ if ((!CPLUS_FAKE_CHILD (child) && child->value == NULL) || parent->error)
+ child->error = 1;
return child;
}
@@ -1451,42 +1562,6 @@ variable_default_display (struct varobj *var)
return FORMAT_NATURAL;
}
-/* This function is similar to GDB's value_contents_equal, except that
- this one is "safe"; it never longjmps. It determines if the VAL1's
- value is the same as VAL2. If for some reason the value of VAR2
- can't be established, *ERROR2 is set to non-zero. */
-
-static int
-my_value_equal (struct value *val1, struct value *volatile val2, int *error2)
-{
- volatile struct gdb_exception except;
-
- /* As a special case, if both are null, we say they're equal. */
- if (val1 == NULL && val2 == NULL)
- return 1;
- else if (val1 == NULL || val2 == NULL)
- return 0;
-
- /* The contents of VAL1 are supposed to be known. */
- gdb_assert (!value_lazy (val1));
-
- /* Make sure we also know the contents of VAL2. */
- val2 = coerce_array (val2);
- TRY_CATCH (except, RETURN_MASK_ERROR)
- {
- if (value_lazy (val2))
- value_fetch_lazy (val2);
- }
- if (except.reason < 0)
- {
- *error2 = 1;
- return 0;
- }
- gdb_assert (!value_lazy (val2));
-
- return value_contents_equal (val1, val2);
-}
-
/* FIXME: The following should be generic for any pointer */
static void
vpush (struct vstack **pstack, struct varobj *var)
@@ -1678,33 +1753,9 @@ value_of_child (struct varobj *parent, int index)
value = (*parent->root->lang->value_of_child) (parent, index);
- /* If we're being lazy, fetch the real value of the variable. */
- if (value != NULL && value_lazy (value))
- {
- /* If we fail to fetch the value of the child, return
- NULL so that callers notice that we're leaving an
- error message. */
- if (!gdb_value_fetch_lazy (value))
- value = NULL;
- }
-
return value;
}
-/* What is the type of VAR? */
-static struct type *
-type_of_child (struct varobj *var)
-{
-
- /* If the child had no evaluation errors, var->value
- will be non-NULL and contain a valid type. */
- if (var->value != NULL)
- return value_type (var->value);
-
- /* Otherwise, we must compute the type. */
- return (*var->root->lang->type_of_child) (var->parent, var->index);
-}
-
/* Is this variable editable? Use the variable's type to make
this determination. */
static int
@@ -1720,9 +1771,15 @@ my_value_of_variable (struct varobj *var)
return (*var->root->lang->value_of_variable) (var);
}
-/* Is VAR something that can change? Depending on language,
- some variable's values never change. For example,
- struct and unions never change values. */
+/* Return non-zero if changes in value of VAR
+ must be detected and reported by -var-update.
+ Return zero is -var-update should never report
+ changes of such values. This makes sense for structures
+ (since the changes in children values will be reported separately),
+ or for artifical objects (like 'public' pseudo-field in C++).
+
+ Return value of 0 means that gdb need not call value_fetch_lazy
+ for the value of this variable object. */
static int
type_changeable (struct varobj *var)
{
@@ -1898,24 +1955,12 @@ c_value_of_root (struct varobj **var_handle)
go on */
if (gdb_evaluate_expression (var->root->exp, &new_val))
{
- if (value_lazy (new_val))
- {
- /* We need to catch errors because if
- value_fetch_lazy fails we still want to continue
- (after making val->error = 1) */
- /* FIXME: Shouldn't be using value_contents()? The
- comment on value_fetch_lazy() says it is only called
- from the macro... */
- if (!gdb_value_fetch_lazy (new_val))
- var->error = 1;
- else
- var->error = 0;
- }
+ var->error = 0;
+ release_value (new_val);
}
else
var->error = 1;
- release_value (new_val);
return new_val;
}
@@ -2094,8 +2139,8 @@ c_value_of_variable (struct varobj *var)
struct cleanup *old_chain = make_cleanup_ui_file_delete (stb);
char *thevalue;
- if (value_lazy (var->value))
- gdb_value_fetch_lazy (var->value);
+ gdb_assert (type_changeable (var));
+ gdb_assert (!value_lazy (var->value));
common_val_print (var->value, stb,
format_code[(int) var->format], 1, 0, 0);
thevalue = ui_file_xstrdup (stb, &dummy);