public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Pass address around within ada-valprint.c
@ 2013-07-30 10:40 Andrew Burgess
  2013-08-06 10:41 ` ping: " Andrew Burgess
  0 siblings, 1 reply; 2+ messages in thread
From: Andrew Burgess @ 2013-07-30 10:40 UTC (permalink / raw)
  To: gdb-patches

I was trying to better understand ada-lang.c:coerce_unspec_val_to_type,
and after looking at the code for a while I tried experimentally
applying this patch to make all values lazy:

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index dc5f2b6..58c4ba1 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -568,15 +568,8 @@ coerce_unspec_val_to_type (struct value *val, struct type *
          trying to allocate some memory for it.  */
       check_size (type);
 
-      if (value_lazy (val)
-          || TYPE_LENGTH (type) > TYPE_LENGTH (value_type (val)))
-       result = allocate_value_lazy (type);
-      else
-       {
-         result = allocate_value (type);
-         memcpy (value_contents_raw (result), value_contents (val),
-                 TYPE_LENGTH (type));
-       }
+      result = allocate_value_lazy (type);
+
       set_value_component_location (result, val);
       set_value_bitsize (result, value_bitsize (val));
       set_value_bitpos (result, value_bitpos (val));


I know we don't want to actually apply this patch as it would
cause gdb to fetch the value contents from the inferior in cases
where we currently reuse the contents we already have to hand,
BUT, as an experiment, I don't see why it should not be fine.

Unfortunately I ran into a few regressions.... it turns out that
in some cases the address parameter on VAL is not set-up correctly [1].

This worries me, given the "|| TYPE_LENGTH (type) > TYPE_LENGTH
(value_type (val))" check, would it not be possible to create an
example where the address of VAL is unset, but the sizes of the types
involved force the creation of a lazy value?  If we could make such
an example then it surely would fail.

The patch below fixes all the regressions that the above experiment
revealed, it's really just passing the address around inside
ada-valprint.c a little more, seems like a good thing to me, but let
me know what you think.

OK to apply?


Thanks,
Andrew

[1] Maybe there's a reason for this that I'm not seeing / understanding.


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

	* ada-valprint.c (print_record): Add address parameter, and pass
	the address on to sub-functions.
	(print_field_values): Add address parameter, and pass the address
	on to sub-functions.
	(ada_val_print_1): Pass address parameter on to sub-functions.
	(print_variant_part): Pass address parameter on to sub-functions.


diff --git a/gdb/ada-valprint.c b/gdb/ada-valprint.c
index 4a04d28..6ec7764 100644
--- a/gdb/ada-valprint.c
+++ b/gdb/ada-valprint.c
@@ -34,14 +34,15 @@
 #include "exceptions.h"
 #include "objfiles.h"
 
-static void print_record (struct type *, const gdb_byte *, int,
+static void print_record (struct type *, const gdb_byte *,
+			  int, CORE_ADDR,
 			  struct ui_file *,
 			  int,
 			  const struct value *,
 			  const struct value_print_options *);
 
 static int print_field_values (struct type *, const gdb_byte *,
-			       int,
+			       int, CORE_ADDR,
 			       struct ui_file *, int,
 			       const struct value *,
 			       const struct value_print_options *,
@@ -658,7 +659,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
       struct value *mark = value_mark ();
       struct value *val;
 
-      val = value_from_contents_and_address (type, valaddr + offset, address);
+      val = value_from_contents_and_address (type, valaddr + offset, address + offset);
       /* If this is a reference, coerce it now.  This helps taking care
 	 of the case where ADDRESS is meaningless because original_value
 	 was not an lval.  */
@@ -732,7 +733,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 	         nonsense value.  Actually, we could use the same
 	         code regardless of lengths; I'm just avoiding a cast.  */
 	      struct value *v1
-		= value_from_contents_and_address (type, valaddr + offset, 0);
+		= value_from_contents_and_address (type, valaddr + offset, address + offset);
 	      struct value *v = value_cast (target_type, v1);
 
 	      ada_val_print_1 (target_type,
@@ -850,7 +851,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 	}
       else
 	{
-	  print_record (type, valaddr, offset_aligned,
+	  print_record (type, valaddr, offset_aligned, address,
 			stream, recurse, original_value, options);
 	  return;
 	}
@@ -916,6 +917,7 @@ ada_val_print_1 (struct type *type, const gdb_byte *valaddr,
 static int
 print_variant_part (struct type *type, int field_num,
 		    const gdb_byte *valaddr, int offset,
+		    CORE_ADDR address,
 		    struct ui_file *stream, int recurse,
 		    const struct value *val,
 		    const struct value_print_options *options,
@@ -934,7 +936,7 @@ print_variant_part (struct type *type, int field_num,
        valaddr,
        offset + TYPE_FIELD_BITPOS (type, field_num) / HOST_CHAR_BIT
        + TYPE_FIELD_BITPOS (var_type, which) / HOST_CHAR_BIT,
-       stream, recurse, val, options,
+       address, stream, recurse, val, options,
        comma_needed, outer_type, outer_offset);
 }
 
@@ -990,7 +992,7 @@ ada_value_print (struct value *val0, struct ui_file *stream,
 
 static void
 print_record (struct type *type, const gdb_byte *valaddr,
-	      int offset,
+	      int offset, CORE_ADDR address,
 	      struct ui_file *stream, int recurse,
 	      const struct value *val,
 	      const struct value_print_options *options)
@@ -999,7 +1001,7 @@ print_record (struct type *type, const gdb_byte *valaddr,
 
   fprintf_filtered (stream, "(");
 
-  if (print_field_values (type, valaddr, offset,
+  if (print_field_values (type, valaddr, offset, address,
 			  stream, recurse, val, options,
 			  0, type, offset) != 0 && options->prettyformat)
     {
@@ -1027,7 +1029,8 @@ print_record (struct type *type, const gdb_byte *valaddr,
 
 static int
 print_field_values (struct type *type, const gdb_byte *valaddr,
-		    int offset, struct ui_file *stream, int recurse,
+		    int offset, CORE_ADDR address,
+		    struct ui_file *stream, int recurse,
 		    const struct value *val,
 		    const struct value_print_options *options,
 		    int comma_needed,
@@ -1049,7 +1052,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 				valaddr,
 				(offset
 				 + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
-				stream, recurse, val, options,
+				address, stream, recurse, val, options,
 				comma_needed, type, offset);
 	  continue;
 	}
@@ -1057,7 +1060,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 	{
 	  comma_needed =
 	    print_variant_part (type, i, valaddr,
-				offset, stream, recurse, val,
+				offset, address, stream, recurse, val,
 				options, comma_needed,
 				outer_type, outer_offset);
 	  continue;
@@ -1111,7 +1114,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 	      opts.deref_ref = 0;
 	      val_print (TYPE_FIELD_TYPE (type, i),
 			 value_contents_for_printing (v),
-			 value_embedded_offset (v), 0,
+			 value_embedded_offset (v), address,
 			 stream, recurse + 1, v,
 			 &opts, current_language);
 	    }
@@ -1125,7 +1128,7 @@ print_field_values (struct type *type, const gdb_byte *valaddr,
 			 valaddr,
 			 (offset
 			  + TYPE_FIELD_BITPOS (type, i) / HOST_CHAR_BIT),
-			 0, stream, recurse + 1, val, &opts);
+			 address, stream, recurse + 1, val, &opts);
 	}
       annotate_field_end ();
     }


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-08-06 10:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-30 10:40 [PATCH] Pass address around within ada-valprint.c Andrew Burgess
2013-08-06 10:41 ` ping: " Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).