[patch 10/12] entryval: @entry values even for references

[patch 10/12] entryval: @entry values even for references

[Jan Kratochvil]

Hi,

for parameters passed by reference the parameter itself does not change; only
the referenced (pointed to) value changes.  This would not be caught at all.

Therefore GCC produces DW_AT_GNU_call_site_data_value (besides regular
DW_AT_GNU_call_site_value) with the dereferenced value.

But GDB needs to create `struct value' of TYPE_CODE_REF which after coerce_ref
will have a different content than where the pointer points to.  

this testcase:
#0  reference (refparam=@0x7fffffffdc3c: 10) at gdb.arch/amd64-entry-value.cc:133
->
#0  reference (refparam=@0x7fffffffdc3c: 10) at gdb.arch/amd64-entry-value.cc:133
        refparam@entry = @0x7fffffffdc3c: 5
so GDB has now found out the entry value really was not 10, so it prints it.

The *-valprint.c patches are ugly, this is because they do not / cannot use
coerce_ref.  This would be fixed by the big value printing rework plan
	http://sourceware.org/ml/gdb-patches/2010-10/msg00127.html
but just I did not get sidetracked by it for this patch, that unpack_pointer
there was ugly enough there already, this patch does not make it much worse.

The `indirect' method gets overloaded for both TYPE_CODE_REF and TYPE_CODE_PTR
by this patch, I do not mind having a new `coerceref' method instead.


Thanks,
Jan


gdb/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Display @entry parameter values even for references.
	* ada-valprint.c (ada_val_print_1) <TYPE_CODE_REF>: Try also
	value_computed_funcs->indirect.
	* c-valprint.c (c_val_print) <TYPE_CODE_REF>: Likewise.
	* dwarf2loc.c (entry_data_value_indirect)
	(entry_data_value_copy_closure, entry_data_value_free_closure)
	(entry_data_value_funcs): New.
	(value_of_dwarf_reg_entry): New variables checked_type, outer_val.  Try
	to fetch and create also referenced value content.
	* f-valprint.c (f_val_print) <TYPE_CODE_REF>: Try also
	value_computed_funcs->indirect.
	* p-valprint.c (pascal_val_print) <TYPE_CODE_REF>: Likewise.
	* printcmd.c (print_variable_and_value): Try to fetch and compare also
	referenced value content.
	* valops.c (value_ind): Call value_computed_funcs->indirect only for
	TYPE_CODE_PTR value types.
	* value.c (coerce_ref): Call value_computed_funcs->indirect for
	TYPE_CODE_REF value types.
	* value.h (struct lval_funcs) <indirect>: Extend comment for
	TYPE_CODE_PTR vs. TYPE_CODE_REF.

gdb/testsuite/
2011-07-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Display @entry parameter values even for references.
	* gdb.arch/amd64-entry-value.exp (entry_reference: bt full): Update the
	expected output.

--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -891,9 +891,27 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       
       if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
         {
-          CORE_ADDR deref_val_int
-	    = unpack_pointer (type, valaddr + offset_aligned);
+          CORE_ADDR deref_val_int;
 
+	  if (VALUE_LVAL (original_value) == lval_computed)
+	    {
+	      const struct lval_funcs *funcs;
+	      
+	      funcs = value_computed_funcs (original_value);
+	      if (funcs->indirect)
+	        {
+		  struct value *result = funcs->indirect (original_value);
+
+		  if (result)
+		    {
+		      common_val_print (result, stream, recurse,
+					options, current_language);
+		      return 0;
+		    }
+	        }
+	    }
+
+          deref_val_int = unpack_pointer (type, valaddr + offset_aligned);
           if (deref_val_int != 0)
             {
               struct value *deref_val =
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -379,10 +379,29 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
 	{
 	  if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
 	    {
-	      struct value *deref_val =
-		value_at
-		(TYPE_TARGET_TYPE (type),
-		 unpack_pointer (type, valaddr + embedded_offset));
+	      struct value *deref_val;
+
+	      if (VALUE_LVAL (original_value) == lval_computed)
+		{
+		  const struct lval_funcs *funcs;
+		  
+		  funcs = value_computed_funcs (original_value);
+		  if (funcs->indirect)
+		    {
+		      struct value *result = funcs->indirect (original_value);
+
+		      if (result)
+			{
+			  common_val_print (result, stream, recurse,
+					    options, current_language);
+			  return 0;
+			}
+		    }
+		}
+
+	      deref_val = value_at (TYPE_TARGET_TYPE (type),
+				    unpack_pointer (type,
+						    valaddr + embedded_offset));
 
 	      common_val_print (deref_val, stream, recurse, options,
 				current_language);
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -3578,6 +3578,60 @@ loclist_read_variable (struct symbol *symbol, struct frame_info *frame)
   return val;
 }
 
+/* VALUE must be of type lval_computed with entry_data_value_funcs.  Perform
+   the indirect method on it, that is use its stored target value, the sole
+   purpose of entry_data_value_funcs..  */
+
+struct value *
+entry_data_value_indirect (struct value *value)
+{
+  struct type *checked_type = check_typedef (value_type (value));
+  struct value *target_val = value_computed_closure (value);
+
+  gdb_assert (TYPE_CODE (checked_type) == TYPE_CODE_PTR
+	      || TYPE_CODE (checked_type) == TYPE_CODE_REF);
+
+  value_incref (target_val);
+  return target_val;
+}
+
+/* Implement copy_closure.  */
+
+static void *
+entry_data_value_copy_closure (const struct value *v)
+{
+  struct value *target_val = value_computed_closure (v);
+
+  value_incref (target_val);
+  return target_val;
+}
+
+/* Implement free_closure.  */
+
+static void
+entry_data_value_free_closure (struct value *v)
+{
+  struct value *target_val = value_computed_closure (v);
+
+  value_free (target_val);
+}
+
+/* Vector for methods for an entry value reference where the referenced value
+   is stored in the caller.  On the first dereference use
+   DW_AT_GNU_call_site_data_value in the caller.  */
+
+static struct lval_funcs entry_data_value_funcs =
+{
+  NULL,	/* read */
+  NULL,	/* write */
+  NULL,	/* check_validity */
+  NULL,	/* check_any_valid */
+  entry_data_value_indirect,
+  NULL,	/* check_synthetic_pointer */
+  entry_data_value_copy_closure,
+  entry_data_value_free_closure
+};
+
 /* Return value of parameter of TYPE at (callee) FRAME which at function entry
    point.  Parameter has been passed in DWARF_REG or FB_OFFSET, see their
    description at struct dwarf_expr_context_funcs->push_dwarf_reg_entry_value.
@@ -3589,13 +3643,72 @@ static struct value *
 value_of_dwarf_reg_entry (struct type *type, struct frame_info *frame,
 			  int dwarf_reg, CORE_ADDR fb_offset)
 {
+  struct type *checked_type = check_typedef (type);
   struct frame_info *caller_frame = get_prev_frame (frame);
+  struct value *outer_val;
   struct call_site_parameter *parameter;
 
   parameter = dwarf_expr_dwarf_reg_entry_value (frame, dwarf_reg, fb_offset);
 
-  return dwarf_entry_parameter_to_value (parameter, -1 /* deref_size */, type,
-					 caller_frame);
+  outer_val = dwarf_entry_parameter_to_value (parameter, -1 /* deref_size */,
+					      type, caller_frame);
+
+  /* Check if DW_AT_GNU_call_site_data_value cannot be used.  */
+
+  if ((TYPE_CODE (checked_type) == TYPE_CODE_PTR
+       || TYPE_CODE (checked_type) == TYPE_CODE_REF)
+      && TYPE_TARGET_TYPE (checked_type) != NULL)
+    {
+      struct type *target_type = TYPE_TARGET_TYPE (checked_type);
+      volatile struct gdb_exception e;
+      struct value *target_val;
+
+      TRY_CATCH (e, RETURN_MASK_ERROR)
+	{
+	  int target_length = TYPE_LENGTH (target_type);
+
+	  target_val = dwarf_entry_parameter_to_value (parameter, target_length,
+						       target_type,
+						       caller_frame);
+	}
+      if (e.reason < 0)
+	{
+	  if (e.error == NOT_FOUND_ERROR)
+	    {
+	      if (info_verbose)
+		exception_print (gdb_stdout, e);
+	    }
+	  else
+	    throw_exception (e);
+	}
+      else
+	{
+	  CORE_ADDR addr;
+	  struct value *val;
+
+	  /* value_as_address dereferences TYPE_CODE_REF.  */
+	  addr = extract_typed_address (value_contents (outer_val),
+					checked_type);
+
+	  /* The target entry value has artificial address of the entry value
+	     reference.  */
+	  VALUE_LVAL (target_val) = lval_memory;
+	  set_value_address (target_val, addr);
+
+	  release_value (target_val);
+	  val = allocate_computed_value (type, &entry_data_value_funcs,
+					 target_val /* closure */);
+
+	  /* Copy the referencing pointer to the new computed value.  */
+	  memcpy (value_contents_raw (val), value_contents_raw (outer_val),
+		  TYPE_LENGTH (checked_type));
+	  set_value_lazy (val, 0);
+
+	  return val;
+	}
+    }
+
+  return outer_val;
 }
 
 /* Read variable SYMBOL like loclist_read_variable at (callee) FRAME's function
--- a/gdb/f-valprint.c
+++ b/gdb/f-valprint.c
@@ -344,10 +344,29 @@ f_val_print (struct type *type, const gdb_byte *valaddr, int embedded_offset,
 	{
 	  if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
 	    {
-	      struct value *deref_val =
-		value_at
-		(TYPE_TARGET_TYPE (type),
-		 unpack_pointer (type, valaddr + embedded_offset));
+	      struct value *deref_val;
+
+	      if (VALUE_LVAL (original_value) == lval_computed)
+		{
+		  const struct lval_funcs *funcs;
+		  
+		  funcs = value_computed_funcs (original_value);
+		  if (funcs->indirect)
+		    {
+		      struct value *result = funcs->indirect (original_value);
+
+		      if (result)
+			{
+			  common_val_print (result, stream, recurse,
+					    options, current_language);
+			  return 0;
+			}
+		    }
+		}
+
+	      deref_val = value_at (TYPE_TARGET_TYPE (type),
+				    unpack_pointer (type,
+						    valaddr + embedded_offset));
 
 	      common_val_print (deref_val, stream, recurse,
 				options, current_language);
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -272,10 +272,29 @@ pascal_val_print (struct type *type, const gdb_byte *valaddr,
 	{
 	  if (TYPE_CODE (elttype) != TYPE_CODE_UNDEF)
 	    {
-	      struct value *deref_val =
-		value_at
-		(TYPE_TARGET_TYPE (type),
-		 unpack_pointer (type, valaddr + embedded_offset));
+	      struct value *deref_val;
+
+	      if (VALUE_LVAL (original_value) == lval_computed)
+		{
+		  const struct lval_funcs *funcs;
+		  
+		  funcs = value_computed_funcs (original_value);
+		  if (funcs->indirect)
+		    {
+		      struct value *result = funcs->indirect (original_value);
+
+		      if (result)
+			{
+			  common_val_print (result, stream, recurse,
+					    options, current_language);
+			  return 0;
+			}
+		    }
+		}
+
+	      deref_val = value_at (TYPE_TARGET_TYPE (type),
+				    unpack_pointer (type,
+						    valaddr + embedded_offset));
 
 	      common_val_print (deref_val, stream, recurse + 1, options,
 				current_language);
--- a/gdb/printcmd.c
+++ b/gdb/printcmd.c
@@ -1987,7 +1987,46 @@ print_variable_and_value (const char *name, struct symbol *var,
 		value_fetch_lazy (entryval);
 	      if (!value_optimized_out (val)
 		  && value_available_contents_eq (val, 0, entryval, 0, len))
-		entryval = NULL;
+		{
+		  volatile struct gdb_exception deref_ex;
+		  struct value *val_deref, *entryval_deref;
+
+		  /* DW_AT_GNU_call_site_value does match with the current
+		     value.  If it is a reference still try to verify if
+		     dereferenced DW_AT_GNU_call_site_data_value does not
+		     differ.  */
+
+		  TRY_CATCH (deref_ex, RETURN_MASK_ERROR)
+		    {
+		      unsigned len_deref;
+
+		      val_deref = coerce_ref (val);
+		      if (value_lazy (val_deref))
+			value_fetch_lazy (val_deref);
+		      len_deref = TYPE_LENGTH (value_type (val_deref));
+
+		      entryval_deref = coerce_ref (entryval);
+		      if (value_lazy (entryval_deref))
+			value_fetch_lazy (entryval_deref);
+
+		      /* If the reference addresses match but dereferenced
+		         content does not match print them.  */
+		      if (val != val_deref
+			  && value_available_contents_eq (val_deref, 0,
+							  entryval_deref, 0,
+							  len_deref))
+			entryval = NULL;
+		    }
+
+		  /* If the dereferenced content could not be fetch do not
+		     display anything.  */
+		  if (deref_ex.reason < 0)
+		    entryval = NULL;
+
+		  /* Value was not a reference; and its content matches.  */
+		  if (val == val_deref)
+		    entryval = NULL;
+		}
 	    }
 	}
 
--- a/gdb/testsuite/gdb.arch/amd64-entry-value.exp
+++ b/gdb/testsuite/gdb.arch/amd64-entry-value.exp
@@ -97,11 +97,12 @@ gdb_continue_to_breakpoint "entry_reference: breakhere_reference"
 
 # (gdb) bt full
 # #0  reference (refparam=@0x7fffffffdc3c: 10) at gdb.arch/amd64-entry-value.cc:133
+#         refparam@entry = @0x7fffffffdc3c: 5
 #         refcopy = 5
 # #1  0x0000000000400424 in main () at gdb.arch/amd64-entry-value.cc:145
 #         refvar = 10
 # Check refparam@entry is present:
-gdb_test "bt full" "#0 +reference \\(refparam=@0x\[0-9a-f\]+: 10\\) \[^\r\n\]*\r\n\[ \t\]*refcopy = 5\r\n#1 +0x\[0-9a-f\]+ in main \\(\\) \[^\r\n\]*\r\n\[ \t\]*refvar = 10" \
+gdb_test "bt full" "#0 +reference \\(refparam=@0x\[0-9a-f\]+: 10\\) \[^\r\n\]*\r\n\[ \t\]*refparam@entry = @0x\[0-9a-f\]+: 5\r\n\[ \t\]*refcopy = 5\r\n#1 +0x\[0-9a-f\]+ in main \\(\\) \[^\r\n\]*\r\n\[ \t\]*refvar = 10" \
 	 "entry_reference: bt full"
 gdb_test "ptype refparam" " = int &" "entry_reference: ptype refparam"
 
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -1743,22 +1743,22 @@ value_ind (struct value *arg1)
 
   base_type = check_typedef (value_type (arg1));
 
-  if (VALUE_LVAL (arg1) == lval_computed)
+  if (TYPE_CODE (base_type) == TYPE_CODE_PTR)
     {
-      const struct lval_funcs *funcs = value_computed_funcs (arg1);
+      struct type *enc_type;
 
-      if (funcs->indirect)
+      if (VALUE_LVAL (arg1) == lval_computed)
 	{
-	  struct value *result = funcs->indirect (arg1);
+	  const struct lval_funcs *funcs = value_computed_funcs (arg1);
 
-	  if (result)
-	    return result;
-	}
-    }
+	  if (funcs->indirect)
+	    {
+	      struct value *result = funcs->indirect (arg1);
 
-  if (TYPE_CODE (base_type) == TYPE_CODE_PTR)
-    {
-      struct type *enc_type;
+	      if (result)
+		return result;
+	    }
+	}
 
       /* We may be pointing to something embedded in a larger object.
          Get the real type of the enclosing object.  */
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -3077,11 +3077,25 @@ coerce_ref (struct value *arg)
 {
   struct type *value_type_arg_tmp = check_typedef (value_type (arg));
 
-  if (TYPE_CODE (value_type_arg_tmp) == TYPE_CODE_REF)
-    arg = value_at_lazy (TYPE_TARGET_TYPE (value_type_arg_tmp),
-			 unpack_pointer (value_type (arg),		
-					 value_contents (arg)));
-  return arg;
+  if (TYPE_CODE (value_type_arg_tmp) != TYPE_CODE_REF)
+    return arg;
+
+  if (VALUE_LVAL (arg) == lval_computed)
+    {
+      const struct lval_funcs *funcs = value_computed_funcs (arg);
+
+      if (funcs->indirect)
+	{
+	  struct value *result = funcs->indirect (arg);
+
+	  if (result)
+	    return result;
+	}
+    }
+
+  return value_at_lazy (TYPE_TARGET_TYPE (value_type_arg_tmp),
+			unpack_pointer (value_type (arg),
+					value_contents (arg)));
 }
 
 struct value *
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -175,9 +175,12 @@ struct lval_funcs
   /* Return 1 if any bit in VALUE is valid, 0 if they are all invalid.  */
   int (*check_any_valid) (const struct value *value);
 
-  /* If non-NULL, this is used to implement pointer indirection for
-     this value.  This method may return NULL, in which case value_ind
-     will fall back to ordinary indirection.  */
+  /* If non-NULL, this is used to implement pointer and/or reference
+     indirection for this value.  This method may return NULL, in which case
+     value_ind will fall back to ordinary indirection.
+
+     TYPE_CODE (check_typedef (value)) specifies which operation should be
+     done.  It is always either TYPE_CODE_PTR or TYPE_CODE_REF.  */
   struct value *(*indirect) (struct value *value);
 
   /* If non-NULL, this is used to determine whether the indicated bits

Re: [patch 10/12] entryval: @entry values even for references

[Daniel Jacobowitz]

On Mon, Jul 18, 2011 at 4:22 PM, Jan Kratochvil
<jan.kratochvil@redhat.com> wrote:
> Hi,
>
> for parameters passed by reference the parameter itself does not change; only
> the referenced (pointed to) value changes.  This would not be caught at all.

This is a little off topic from the current patch, but if the
reference value is not known, and the reference entry value is known,
should we use the entry value automatically?  Since it's supposed to
be read-only.  This could potentially hide a corrupted reference
pointer, that's the only downside.

-- 
Thanks,
Daniel

Re: [patch 10/12] entryval: @entry values even for references

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Jan> +          CORE_ADDR deref_val_int;
 
Jan> +	  if (VALUE_LVAL (original_value) == lval_computed)
Jan> +	    {
Jan> +	      const struct lval_funcs *funcs;
Jan> +	      
Jan> +	      funcs = value_computed_funcs (original_value);
Jan> +	      if (funcs->indirect)
Jan> +	        {
Jan> +		  struct value *result = funcs->indirect (original_value);
Jan> +
Jan> +		  if (result)
Jan> +		    {
Jan> +		      common_val_print (result, stream, recurse,
Jan> +					options, current_language);
Jan> +		      return 0;
Jan> +		    }
Jan> +	        }
Jan> +	    }

This code is repeated a few times.  I think it should go into a new
function.

Jan> +struct value *
Jan> +entry_data_value_indirect (struct value *value)
Jan> +{

I think this should be static.

Jan> +static struct lval_funcs entry_data_value_funcs =

const

Tom

Re: [patch 10/12] entryval: @entry values even for references

[Jan Kratochvil]

On Tue, 19 Jul 2011 17:49:01 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> +	  if (VALUE_LVAL (original_value) == lval_computed)
[...]
> 
> This code is repeated a few times.  I think it should go into a new
> function.

done.

> I think this should be static.
+ 
> const

done, done.


It will be in a new patchset resubmit.


Thanks,
Jan

Re: [patch 10/12] entryval: @entry values even for references

[Jan Kratochvil]

On Mon, 18 Jul 2011 23:04:06 +0200, Daniel Jacobowitz wrote:
> On Mon, Jul 18, 2011 at 4:22 PM, Jan Kratochvil <jan.kratochvil@redhat.com> wrote:
> > for parameters passed by reference the parameter itself does not change; only
> > the referenced (pointed to) value changes.  This would not be caught at all.
> 
> This is a little off topic from the current patch, but if the
> reference value is not known, and the reference entry value is known,
> should we use the entry value automatically?  Since it's supposed to
> be read-only.  This could potentially hide a corrupted reference
> pointer, that's the only downside.

IIUC added there:
+  /* Check if DW_AT_GNU_call_site_data_value cannot be used.  If it should be
+     used and it is not available do not fall back to OUTER_VAL - dereferencing
+     TYPE_CODE_REF with non-entry data value would give current value - not the
+     entry value.  */

That is if DW_AT_GNU_call_site_data_value could be used and it is not there do
cancel any call site computation at all.

+gdb_test "p nodataparam@entry" "Cannot resolve DW_AT_GNU_call_site_data_value" "entry_reference: p nodataparam@entry"

This works for TYPE_CODE_REF.  But still for some:
	(gdb) print glib_hash_pointer@entry
a pretty printer gets into effect which will print the current data values
referenced by the (unchanged) @entry pointer; this issue is not solved.


It will be in a new patchset resubmit.


Thanks,
Jan