Re: [COMMIT PATCH] value_bits_valid: Fix latent bug.

["Andrew Burgess" <aburgess@broadcom.com>]

On 04/07/2013 5:09 PM, Pedro Alves wrote:
> Doing something else, I factored out the bits of the value_bits_valid
> function that actually handle the check_validity hook, and
> surprisingly found out that the result was misbehaving.  Turns out
> value_bits_valid has a latent bug.  If the value is not lval_computed,
> or doesn't have a check_validity hook, then we should assume the value
> is entirely valid, not invalid.  This is currently masked by the
> value->optimized_out check -- I ran the testsuite with a gdb_assert(0)
> inserted in place of that return being touched by the patch, and it
> never triggers.
> 
> Tested on x86_64 Fedora 17.
> 
> gdb/
> 2013-07-04  Pedro Alves  <palves@redhat.com>
> 
> 	* value.c (value_bits_valid): If the value is not lval_computed,
> 	or doesn't have a check_validity hook, assume the value is entirely
> 	valid.
> ---
>  gdb/value.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gdb/value.c b/gdb/value.c
> index ce4b13a..353f62a 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1086,7 +1086,7 @@ value_bits_valid (const struct value *value, int offset, int length)
>      return 1;
>    if (value->lval != lval_computed
>        || !value->location.computed.funcs->check_validity)
> -    return 0;
> +    return 1;
>    return value->location.computed.funcs->check_validity (value, offset,
>  							 length);
>  }
> 

There's a small issue with this patch, in the case of an optimized_out,
non-computed value we now report that the bits are valid when they
should be invalid.

Patch below applies on top of the above and fixes both the original
issue Pedro spotted, and fixes the non-computed issue.

Ok to apply?

Thanks,
Andrew


gdb/ChangeLog

2013-07-05  Andrew Burgess  <aburgess@broadcom.com>

	* value.c (value_bits_valid): If the value is not lval_computed
	then the answer is in the optimized_out flag, otherwise if we have
	no handler assume bits are valid, if there is a handler use that.



diff --git a/gdb/value.c b/gdb/value.c
index 353f62a..ca5463b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1082,13 +1082,18 @@ value_entirely_optimized_out (const struct value *value)
 int
 value_bits_valid (const struct value *value, int offset, int length)
 {
-  if (!value->optimized_out)
-    return 1;
-  if (value->lval != lval_computed
-      || !value->location.computed.funcs->check_validity)
-    return 1;
-  return value->location.computed.funcs->check_validity (value, offset,
-							 length);
+  if (value->lval != lval_computed)
+    return !value->optimized_out;
+  else
+    {
+      /* Computed value, defer to handler if there is one.  */
+      if (!value->location.computed.funcs->check_validity)
+	return 1;
+      else
+	return value->location.computed.funcs->check_validity (value,
+							       offset,
+							       length);
+    }
 }
 
 int