[Patch] Fix for -var-update to use natural format to compare

[Patch] Fix for -var-update to use natural format to compare

[Marc Khouzam]

This patch addresses a discussion titled "-var-update using formatted value" from
the GDB mailing list.
It modifies the variable object code to always uses the natural format to compare 
old and new values for the -var-update command (one line change).
It also renames the member print_value of varobj to natural_value.

### Eclipse Workspace Patch 1.0
#P gdb
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.99
diff -u -r1.99 varobj.c
--- varobj.c    1 Jan 2008 22:53:13 -0000       1.99
+++ varobj.c    22 Jan 2008 03:19:28 -0000
@@ -139,8 +139,8 @@
   /* Was this variable updated via a varobj_set_value operation */
   int updated;
 
-  /* Last print value.  */
-  char *print_value;
+  /* Last value in natural format.  */
+  char *natural_value;
 
   /* Is this variable frozen.  Frozen variables are never implicitly
      updated by -var-update * 
@@ -966,7 +966,7 @@
   int need_to_fetch;
   int changed = 0;
   int intentionally_not_fetched = 0;
-  char *print_value = NULL;
+  char *natural_value = NULL;
 
   /* 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
@@ -1025,11 +1025,15 @@
     }
 
   /* Below, we'll be comparing string rendering of old and new
-     values.  Don't get string rendering if the value is
+     values.  We only use the natural format to make sure we
+     catch changes that may affect some but not all formats.
+     E.g, float 1.1 and 1.2 are the same in all format except
+     natural; we don't want to miss this.
+     Don't get string rendering if the value is
      lazy -- if it is, the code above has decided that the value
      should not be fetched.  */
   if (value && !value_lazy (value))
-      print_value = value_get_print_value (value, var->format);
+      natural_value = value_get_print_value (value, FORMAT_NATURAL);
 
   /* 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
@@ -1069,8 +1073,8 @@
              gdb_assert (!value_lazy (var->value));
              gdb_assert (!value_lazy (value));
 
-             gdb_assert (var->print_value != NULL && print_value != NULL);
-             if (strcmp (var->print_value, print_value) != 0)
+             gdb_assert (var->natural_value != NULL && natural_value != NULL);
+             if (strcmp (var->natural_value, natural_value) != 0)
                changed = 1;
            }
        }
@@ -1080,9 +1084,9 @@
   if (var->value != NULL && var->value != value)
     value_free (var->value);
   var->value = value;
-  if (var->print_value)
-    xfree (var->print_value);
-  var->print_value = print_value;
+  if (var->natural_value)
+    xfree (var->natural_value);
+  var->natural_value = natural_value;
   if (value && value_lazy (value) && intentionally_not_fetched)
     var->not_fetched = 1;
   else
@@ -1489,7 +1493,7 @@
   var->format = 0;
   var->root = NULL;
   var->updated = 0;
-  var->print_value = NULL;
+  var->natural_value = NULL;
   var->frozen = 0;
   var->not_fetched = 0;
 
@@ -1526,7 +1530,7 @@
 
   xfree (var->name);
   xfree (var->obj_name);
-  xfree (var->print_value);
+  xfree (var->natural_value);
   xfree (var->path_expr);
   xfree (var);
 }

Re: [Patch] Fix for -var-update to use natural format to compare

[Daniel Jacobowitz]

On Mon, Jan 21, 2008 at 10:24:40PM -0500, Marc Khouzam wrote:
> This patch addresses a discussion titled "-var-update using formatted value" from
> the GDB mailing list.
> It modifies the variable object code to always uses the natural format to compare 
> old and new values for the -var-update command (one line change).
> It also renames the member print_value of varobj to natural_value.

I don't think this is an appropriate change.

-var-update is supposed to tell when the displayed value has changed.
If the value in the current format hasn't changed, there's no need to
refetch.  It's not about the underlying memory bytes, which is why we
changed to a string comparison.  There are various other ways to
change the underlying variable without updating the representation
(e.g. changes in padding bits).

I think you've explained this already - probably to me - but could you
do it once more, please: why do you want to keep track of the value in
all formats?  And if you're actually displaying them all
simultaneously to the user, why shouldn't they be independent varobjs?

-- 
Daniel Jacobowitz
CodeSourcery

RE: [Patch] Fix for -var-update to use natural format to compare

[Marc Khouzam]

> > It modifies the variable object code to always uses the natural format to compare 
> > old and new values for the -var-update command (one line change).
> > It also renames the member print_value of varobj to natural_value.
> 
> I don't think this is an appropriate change.
> 
> -var-update is supposed to tell when the displayed value has changed.
> If the value in the current format hasn't changed, there's no need to
> refetch.  

This is assuming the frontend only displays a single value.
In DSF, we (sometimes) display all values at once.  Can be nice for the user.

> why do you want to keep track of the value in
> all formats?  And if you're actually displaying them all
> simultaneously to the user, why shouldn't they be independent varobjs?

Independent varObjects would work.  However every time the program stops
the frontend will need to issue a var-update on five objects instead of one,
and what is worse is that GDB will need to read the memory from the target
five times instead of once.  This is less efficient than the frontend
forcing GDB to use natural format (see below.)

You can refer to http://sourceware.org/ml/gdb/2008-01/msg00175.html
where you had agreed with me :-)

One solution is the patch I suggested.

An alternative suggestion that would support both use cases,
was to have an extra flag to var-update.  
Something like [--content-changed | --last-display-value-changed]
It would be a separate flag than the --no-values one.
The front-end could then decide which behavior it wants. 
Although, and I can understand, adding this new flag did not seem too
popular.

To be honest, I will have to support GDB 6.7 anyway, so I will need to
work around this by myself, no matter what.  What I will do is that
every time I issue a var-set-format and an evaluate-expression,
I will follow it by another var-set-format to natural to keep all variable
objects to the natural format.  This will force GDB to use the natural
format for its var-update comparison.
I just thought that GDB would want to support frontends that display more
than one format at once.

Thanks

Marc

Re: [Patch] Fix for -var-update to use natural format to compare

[Daniel Jacobowitz]

On Tue, Jan 22, 2008 at 09:43:21AM -0500, Marc Khouzam wrote:
> You can refer to http://sourceware.org/ml/gdb/2008-01/msg00175.html
> where you had agreed with me :-)

That's what happens when the thread goes on too long :-)

Here's the problem, as I see it.  There are two different reasonable
definitions of "varobj has changed".  One is that the textual
representation has changed.  The other is that the underlying bytes
have changed.

We used to check the underlying bytes.  This was a problem because the
textual representation for a "char *" includes more bytes than the
value, for C-like languages.  So, as Nick mentioned last week, we
would not detect a change from "GNU" to "GDB" in a "char *", but we
would display the "GNU" string in -var-evaluate-expression.

We switched to checking the string representation.  But if you're
looking in lots of formats, then this is not enough.

We could check in all supported formats; check in the natural format;
or go back to checking the underlying bytes.  Checking in the natural
format is appealing because it's efficient.  But does the natural
format always capture changes in any of the other formats?

-- 
Daniel Jacobowitz
CodeSourcery

RE: [Patch] Fix for -var-update to use natural format to compare

[Marc Khouzam]

> That's what happens when the thread goes on too long :-)

I try to keep my responses short, but somehow I always get carried away :-)

> We could check in all supported formats; check in the natural format;
> or go back to checking the underlying bytes.  Checking in the natural
> format is appealing because it's efficient.  But does the natural
> format always capture changes in any of the other formats?

I didn't find an example where checking the natural format would not work.
But that may not mean much :-)
Boolean could have been a problem (0x1, 0x2, 0x3 etc are all "true" in natural),
but it seems that for now, GDB always shows 0x1 for true.

A safer way may be to have GDB check for content difference, as it used to do
before and to also check the natural printed format for the case of char*
(to detect a change from "GDB" to "GNU").

Marc

Re: [Patch] Fix for -var-update to use natural format to compare

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Tue, Jan 22, 2008 at 09:43:21AM -0500, Marc Khouzam wrote:
>> You can refer to http://sourceware.org/ml/gdb/2008-01/msg00175.html
>> where you had agreed with me :-)
> 
> That's what happens when the thread goes on too long :-)
> 
> Here's the problem, as I see it.  There are two different reasonable
> definitions of "varobj has changed".  One is that the textual
> representation has changed.  The other is that the underlying bytes
> have changed.
> 
> We used to check the underlying bytes.  This was a problem because the
> textual representation for a "char *" includes more bytes than the
> value, for C-like languages.  So, as Nick mentioned last week, we
> would not detect a change from "GNU" to "GDB" in a "char *", but we
> would display the "GNU" string in -var-evaluate-expression.
> 
> We switched to checking the string representation.  But if you're
> looking in lots of formats, then this is not enough.
> 
> We could check in all supported formats; check in the natural format;
> or go back to checking the underlying bytes.  Checking in the natural
> format is appealing because it's efficient.  But does the natural
> format always capture changes in any of the other formats?

We don't know. In particular, because we haven't yet
fully implemented Python-based custom formatting of things, so we're
not sure how that work and interact with traditional formats. 
Specifically, suppose there a command that specifies Python function
used to compute string representation of a value. In that case, 'natural'
format in the current meaning might not change while string 
representation return by the Python code changes.

I think there are actually two different problems:

1. Given single varobj that is displayed in one format, how does one
one if the value is "really changed". The answer here is that you don't
get to find that -- -var-update reports only changes in displayed
value, and since changing format is fast, nothing else is needed here.

2. If UI wishes to show varobj in several formats, how does it
figure out of *any* of formats has changed. There are two possible
solutions:

   (a) Always keep varobj (from frontnend) in natural format, and assume 
     that if natural format changes, all values possibly change. In this
     case, getting a value in different format will involve switching
     to that format and then switching back.
   (b) Allow a varobj to have a *list* formats to show. Make
     -var-update check all formats, make -var-evaluate-expression
     return all formats.

Now (a) can be done either on frontend side, or on gdb side (by making
comparisons use naturally formatted string). I suspect that the frontend
side is better here, since very few frontends regularly show same
value in several formats.

- Volodya




- Volodya

RE: [Patch] Fix for -var-update to use natural format to compare

[Marc Khouzam]

> I think there are actually two different problems:
> 
> 1. Given single varobj that is displayed in one format, how does one
> one if the value is "really changed". The answer here is that you don't
> get to find that -- -var-update reports only changes in displayed
> value, and since changing format is fast, nothing else is needed here.

Ok.

> 2. If UI wishes to show varobj in several formats, how does it
> figure out of *any* of formats has changed. There are two possible
> solutions:
> 
>    (a) Always keep varobj (from frontnend) in natural format, and assume 
>      that if natural format changes, all values possibly change. In this
>      case, getting a value in different format will involve switching
>      to that format and then switching back.

That is what I will do.
If it is the best thing for GDB to make this a requirement for a frontend that
wants to show mutliple formats, may I suggest that it be added to the
documentation of var-update?  A short mention that "if a frontend
wants to be notified of a change in any format, it should keep the variable
object in the natural format".

Also, if GDB requires this of frontends showing mutliple formats, it would
be nice to have -var-evaluate-expression take a format as an optional
parameter.  Then, the frontend would not need to use -var-set-format
to get a different formatted value, and could keep the variable object
in the natural format at all times.

I can provide a patch if this is a supported idea.

>    (b) Allow a varobj to have a *list* formats to show. Make
>      -var-update check all formats, make -var-evaluate-expression
>      return all formats.
> 

This seems cumbersome to me and will provide superfluous information
for case 1 above.

RE: [Patch] Fix for -var-update to use natural format to compare

[Vladimir Prus]

Marc Khouzam wrote:

> 
>> I think there are actually two different problems:
>> 
>> 1. Given single varobj that is displayed in one format, how does one
>> one if the value is "really changed". The answer here is that you don't
>> get to find that -- -var-update reports only changes in displayed
>> value, and since changing format is fast, nothing else is needed here.
> 
> Ok.
> 
>> 2. If UI wishes to show varobj in several formats, how does it
>> figure out of *any* of formats has changed. There are two possible
>> solutions:
>> 
>>    (a) Always keep varobj (from frontnend) in natural format, and assume
>>      that if natural format changes, all values possibly change. In this
>>      case, getting a value in different format will involve switching
>>      to that format and then switching back.
> 
> That is what I will do.
> If it is the best thing for GDB to make this a requirement for a frontend
> that wants to show mutliple formats, may I suggest that it be added to the
> documentation of var-update?  A short mention that "if a frontend
> wants to be notified of a change in any format, it should keep the
> variable object in the natural format".

It's good to improve docs in this way. It should be noted that this
might not be true in future, say when we add Python scripting,  or
(maybe) when we'll add a way to specify floating point precision.

> Also, if GDB requires this of frontends showing mutliple formats, it would
> be nice to have -var-evaluate-expression take a format as an optional
> parameter.  Then, the frontend would not need to use -var-set-format
> to get a different formatted value, and could keep the variable object
> in the natural format at all times.
> 
> I can provide a patch if this is a supported idea.

I personally think this is a good idea! It means that if we for example 
need to quickly show some value in specific format, like hex value in tooltip,
we don't need two format setting commands before and after. So, such
a patch will be valuable independently of how -var-update operates.

- Volodya

Re: [Patch] Fix for -var-update to use natural format to compare

[Daniel Jacobowitz]

On Tue, Jan 22, 2008 at 08:49:53PM +0300, Vladimir Prus wrote:
> It's good to improve docs in this way. It should be noted that this
> might not be true in future, say when we add Python scripting,  or
> (maybe) when we'll add a way to specify floating point precision.

Let's keep the documentation focused on what's true today - no point
saying "this might not work someday" when there isn't an alternative
yet.

> > Also, if GDB requires this of frontends showing mutliple formats, it would
> > be nice to have -var-evaluate-expression take a format as an optional
> > parameter.  Then, the frontend would not need to use -var-set-format
> > to get a different formatted value, and could keep the variable object
> > in the natural format at all times.
> > 
> > I can provide a patch if this is a supported idea.
> 
> I personally think this is a good idea! It means that if we for example 
> need to quickly show some value in specific format, like hex value in tooltip,
> we don't need two format setting commands before and after. So, such
> a patch will be valuable independently of how -var-update operates.

I think this is a good idea too.

-- 
Daniel Jacobowitz
CodeSourcery

RE: [Patch] Fix for -var-update to use natural format to compare

[Nick Roberts]

 > This is assuming the frontend only displays a single value.
 > In DSF, we (sometimes) display all values at once.  Can be nice for the user.
 > 
 > > why do you want to keep track of the value in
 > > all formats?  And if you're actually displaying them all
 > > simultaneously to the user, why shouldn't they be independent varobjs?
 > 
 > Independent varObjects would work.  However every time the program stops
 > the frontend will need to issue a var-update on five objects instead of one,
 > and what is worse is that GDB will need to read the memory from the target
 > five times instead of once.  This is less efficient than the frontend
 > forcing GDB to use natural format (see below.)

You say it "Can be nice for the user." but you haven't really answered the
first question:

   why do you want to keep track of the value in all formats?

Why, for example, would a user want to see the value of a double as a
decimal?  It just seems to be a misleading thing to do.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

RE: [Patch] Fix for -var-update to use natural format to compare

[Marc Khouzam]

>> In DSF, we (sometimes) display all values at once.  Can be nice for the user.
> 
> You say it "Can be nice for the user." but you haven't really answered the
> first question:
> 
>    why do you want to keep track of the value in all formats?
> 
> Why, for example, would a user want to see the value of a double as a
> decimal?  It just seems to be a misleading thing to do.

Users will be users :-)
But you make a good point.  The answer can be found in
http://dev.eclipse.org/mhonarc/lists/dsdp-dd-dev/msg01004.html
which I copied here:

"The DSF GDB debugger uses the detail pane in the variables, registers, and 
expressions views to display the variable in all available formats at the same
time.

There is a bigger issue here though.  The architecture of the debugger 
integration in Eclipse separates the UI code (views, actions, etc) and the 
non-UI code with an API that abstracts the details of the debugger back end.
However, GDB's design of the variable objects makes some assumptions about the
presentation of the data in the UI, and these assumptions are making it 
somewhat difficult to create an efficient implementation of the  non-UI layer 
of the debugger integration. "

So, to paraphrase, in DSF don't make any assumptions on what the UI will 
request, but instead, we give the UI the possibility to request any of the 
formats at any time, without restrictions.

Hope this clarifies a bit.

Marc