[patch] Fix valgrind false warning (by Ada)

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

Hi,

gdb.ada/arrayparam.exp under valgrind (x86_64-fedora13-linux-gnu) reports:

Invalid read of size 1
   extract_unsigned_integer (findvar.c:111)
   modify_field (value.c:2213)
   modify_general_field (ada-lang.c:1352)
   make_array_descriptor (ada-lang.c:4046)
   ada_convert_actual (ada-lang.c:3970)
   value_arg_coerce (infcall.c:146)
   call_function_by_hand (infcall.c:670)
   ada_evaluate_subexp (ada-lang.c:9274)
   evaluate_subexp (eval.c:76)
   evaluate_expression (eval.c:167)
   print_command_1 (printcmd.c:969)
   print_command (printcmd.c:1017)
 Address 0x6c0fc5b is 3 bytes after a block of size 8 alloc'd
   calloc (vg_replace_malloc.c:418)
   xcalloc (utils.c:1514)
   xzalloc (utils.c:1478)
   allocate_value_contents (value.c:298)
   allocate_value (value.c:308)
   make_array_descriptor (ada-lang.c:4036)
   ada_convert_actual (ada-lang.c:3970)
   value_arg_coerce (infcall.c:146)
   call_function_by_hand (infcall.c:670)
   ada_evaluate_subexp (ada-lang.c:9274)
   evaluate_subexp (eval.c:76)
   evaluate_expression (eval.c:167)

followed by similiar:
Invalid write of size 1

No regressions on {x86_64,x86_64-m32,i686}-fedora14snapshot-linux-gnu.
No regressions for gdb.ada/* on {ppc64-m64,ppc64-m32}-rhel60-linux-gnu.

I will check it in in some days.


Thanks,
Jan


gdb/
2010-10-18  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* ada-lang.c (modify_general_field): Remove.
	(make_array_descriptor): Replace all modify_general_field calls by
	modify_field.
	* value.c (modify_field): Update comment.  New variable bytesize.
	Normalize BITPOS.  Initialize BYTESIZE, use it.

--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -69,8 +69,6 @@
 #define TRUNCATION_TOWARDS_ZERO ((-5 / 2) == -2)
 #endif
 
-static void modify_general_field (struct type *, char *, LONGEST, int, int);
-
 static struct type *desc_base_type (struct type *);
 
 static struct type *desc_bounds_type (struct type *);
@@ -1343,15 +1341,6 @@ static char *bound_name[] = {
 
 #define MAX_ADA_DIMENS (sizeof(bound_name) / (2*sizeof(char *)))
 
-/* Like modify_field, but allows bitpos > wordlength.  */
-
-static void
-modify_general_field (struct type *type, char *addr,
-		      LONGEST fieldval, int bitpos, int bitsize)
-{
-  modify_field (type, addr + bitpos / 8, fieldval, bitpos % 8, bitsize);
-}
-
 
 /* The desc_* routines return primitive portions of array descriptors
    (fat pointers).  */
@@ -4038,33 +4027,31 @@ make_array_descriptor (struct type *type, struct value *arr)
 
   for (i = ada_array_arity (ada_check_typedef (value_type (arr))); i > 0; i -= 1)
     {
-      modify_general_field (value_type (bounds),
-			    value_contents_writeable (bounds),
-                            ada_array_bound (arr, i, 0),
-                            desc_bound_bitpos (bounds_type, i, 0),
-                            desc_bound_bitsize (bounds_type, i, 0));
-      modify_general_field (value_type (bounds),
-			    value_contents_writeable (bounds),
-                            ada_array_bound (arr, i, 1),
-                            desc_bound_bitpos (bounds_type, i, 1),
-                            desc_bound_bitsize (bounds_type, i, 1));
+      modify_field (value_type (bounds), value_contents_writeable (bounds),
+		    ada_array_bound (arr, i, 0),
+		    desc_bound_bitpos (bounds_type, i, 0),
+		    desc_bound_bitsize (bounds_type, i, 0));
+      modify_field (value_type (bounds), value_contents_writeable (bounds),
+		    ada_array_bound (arr, i, 1),
+		    desc_bound_bitpos (bounds_type, i, 1),
+		    desc_bound_bitsize (bounds_type, i, 1));
     }
 
   bounds = ensure_lval (bounds);
 
-  modify_general_field (value_type (descriptor),
-			value_contents_writeable (descriptor),
-                        value_pointer (ensure_lval (arr),
-                                       TYPE_FIELD_TYPE (desc_type, 0)),
-                        fat_pntr_data_bitpos (desc_type),
-                        fat_pntr_data_bitsize (desc_type));
-
-  modify_general_field (value_type (descriptor),
-			value_contents_writeable (descriptor),
-                        value_pointer (bounds,
-                                       TYPE_FIELD_TYPE (desc_type, 1)),
-                        fat_pntr_bounds_bitpos (desc_type),
-                        fat_pntr_bounds_bitsize (desc_type));
+  modify_field (value_type (descriptor),
+		value_contents_writeable (descriptor),
+		value_pointer (ensure_lval (arr),
+			       TYPE_FIELD_TYPE (desc_type, 0)),
+		fat_pntr_data_bitpos (desc_type),
+		fat_pntr_data_bitsize (desc_type));
+
+  modify_field (value_type (descriptor),
+		value_contents_writeable (descriptor),
+		value_pointer (bounds,
+			       TYPE_FIELD_TYPE (desc_type, 1)),
+		fat_pntr_bounds_bitpos (desc_type),
+		fat_pntr_bounds_bitsize (desc_type));
 
   descriptor = ensure_lval (descriptor);
 
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -2183,7 +2183,7 @@ unpack_field_as_long (struct type *type, const gdb_byte *valaddr, int fieldno)
    target byte order; the bitfield starts in the byte pointed to.  FIELDVAL
    is the desired value of the field, in host byte order.  BITPOS and BITSIZE
    indicate which bits (in target bit order) comprise the bitfield.  
-   Requires 0 < BITSIZE <= lbits, 0 <= BITPOS+BITSIZE <= lbits, and
+   Requires 0 < BITSIZE <= lbits, 0 <= BITPOS % 8 + BITSIZE <= lbits, and
    0 <= BITPOS, where lbits is the size of a LONGEST in bits.  */
 
 void
@@ -2193,6 +2193,11 @@ modify_field (struct type *type, gdb_byte *addr,
   enum bfd_endian byte_order = gdbarch_byte_order (get_type_arch (type));
   ULONGEST oword;
   ULONGEST mask = (ULONGEST) -1 >> (8 * sizeof (ULONGEST) - bitsize);
+  int bytesize;
+
+  /* Normalize BITPOS.  */
+  addr += bitpos / 8;
+  bitpos %= 8;
 
   /* If a negative fieldval fits in the field in question, chop
      off the sign extension bits.  */
@@ -2210,16 +2215,20 @@ modify_field (struct type *type, gdb_byte *addr,
       fieldval &= mask;
     }
 
-  oword = extract_unsigned_integer (addr, sizeof oword, byte_order);
+  /* Ensure no bytes outside of the modified ones get accessed as it may cause
+     false valgrind reports.  */
+
+  bytesize = (bitpos + bitsize + 7) / 8;
+  oword = extract_unsigned_integer (addr, bytesize, byte_order);
 
   /* Shifting for bit field depends on endianness of the target machine.  */
   if (gdbarch_bits_big_endian (get_type_arch (type)))
-    bitpos = sizeof (oword) * 8 - bitpos - bitsize;
+    bitpos = bytesize * 8 - bitpos - bitsize;
 
   oword &= ~(mask << bitpos);
   oword |= fieldval << bitpos;
 
-  store_unsigned_integer (addr, sizeof oword, byte_order, oword);
+  store_unsigned_integer (addr, bytesize, byte_order, oword);
 }
 
 /* Pack NUM into BUF using a target format of TYPE.  */