[patch] ASAN fix for gdb.cp/inherit.exp and gdb.cp/virtbase.exp

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

Hi,

unfortunately still many other cases crash with ASAN but at least two more
testcases get fixed for future ASAN compatibility of GDB so that more bugs
could get caught by ASAN before they get checked in and being chased later.

I was checking what happens for gdb.cp/virtbase.exp:

../gdb -nx -q gdb.cp/virtbase -ex 'b 131' -ex r -ex 'print *virtual_middle_b'
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x61a00003a177 at pc 0x99d65f bp 0x7fff479f7180 sp 0x7fff479f7170
READ of size 1 at 0x61a00003a177 thread T0
    #0 0x99d65e in value_from_contents .../gdb/value.c:3502
    #1 0x99d3f4 in value_from_contents_and_address .../gdb/value.c:3484
    #2 0x88cb59 in gdbpy_apply_val_pretty_printer python/py-prettyprint.c:717
    #3 0xa84803 in apply_ext_lang_val_pretty_printer .../gdb/extension.c:498
    #4 0x9d69a0 in value_print .../gdb/valprint.c:877
0x61a00003a177 is located 15 bytes to the right of 1256-byte region [0x61a000039c80,0x61a00003a168)
allocated by thread T0 here:
    #0 0x7fb15cfa6cf5 in calloc (/lib64/libasan.so.1+0x57cf5)
    #1 0xdb52d8 in xcalloc common/common-utils.c:85
    #2 0xdb5324 in xzalloc common/common-utils.c:95
    #3 0x9933d6 in allocate_value_contents .../gdb/value.c:957

struct VirtualBase;
struct VirtualMiddleA : public virtual VirtualBase;
struct VirtualMiddleB : public virtual VirtualBase;
struct Virtual : public virtual VirtualMiddleA, public virtual VirtualMiddleB;
sizeof
  1224 VirtualMiddleA
    32 VirtualMiddleB *virtual_middle_b
  1256 Virtual virtual_o

VirtualMiddleB *virtual_middle_b
1240 embedded_offset
1240 = virtual_middle_b - &virtual_o

In short Virtual really contains VirtualBase only once (obviously).
Upcasting Virtual * to VirtualMiddleB * will create object with sizeof(*ptr)
of standalone VirtualMiddleB (which is 32). But if we copy all the data of
*(VirtualMiddleB *)ptr of this cast we will run out of Virtual virtual_o size
and ASAN correctly complains.

This is because VirtualBase is located before VirtualMiddleB *ptr itself.
Only the special vptr in VirtualMiddleB * will find the proper location of
VirtualBase for this upcasted VirtualMiddleB *.

The attached patch just naively avoids the ASAN crash and nothing more.
So I find it safe.  Maybe one could be more proactive with memory tainting:

 * We could taint the currently memset-zero memory (memset from this patch,
   that is the bytes at 1256..1240+32 from the example above) such as just by
   removing the memset to make the memory uninitialized for ASAN.
   But unfortunately that struct value * could be naively copied into another
   struct value * and then the uninitialized bytes will get hit.
   [ I did not try if it works in practice or not as I find it broken by
   design. ]

 * Provide some adjusted/derived struct type of VirtualMiddleB for this
   special kind of upcasted VirtualMiddleB so that its TYPE_LENGTH is shorter.
   Given the state of the inferior type system I did not try it.

No regressions on {x86_64,x86_64-m32,i686}-fedorarawhide-linux-gnu.


Thanks,
Jan

gdb/
2014-08-21  Jan Kratochvil  <jan.kratochvil@redhat.com>

	Fix gdb.cp/inherit.exp and gdb.cp/virtbase.exp with ASAN.
	* python/py-prettyprint.c (gdbpy_apply_val_pretty_printer): Use
	value_from_contents_and_address_safe.
	* value.c (value_enclosing_type): Make the parameter const.
	(value_from_contents_and_address): Rename to ...
	(value_from_contents_and_address_safe): ... here, add parameter
	valaddr_len.
	(value_from_contents_and_address): New wrapper.
	(value_from_contents): Rename to ...
	(value_from_contents_safe): ... here, add parameter contents_len.
	(value_from_contents): New wrapper.
	* value.h (value_enclosing_type): Make the parameter const.
	(value_from_contents_and_address_safe, value_from_contents_safe): New
	prototype.

diff --git a/gdb/python/py-prettyprint.c b/gdb/python/py-prettyprint.c
index a7cd337..febf7a7 100644
--- a/gdb/python/py-prettyprint.c
+++ b/gdb/python/py-prettyprint.c
@@ -701,6 +701,7 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
   struct cleanup *cleanups;
   enum ext_lang_rc result = EXT_LANG_RC_NOP;
   enum string_repr_result print_result;
+  size_t valaddr_len = TYPE_LENGTH (check_typedef (value_enclosing_type (val)));
 
   /* No pretty-printer support for unavailable values.  */
   if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
@@ -713,9 +714,14 @@ gdbpy_apply_val_pretty_printer (const struct extension_language_defn *extlang,
 
   /* Instantiate the printer.  */
   if (valaddr)
-    valaddr += embedded_offset;
-  value = value_from_contents_and_address (type, valaddr,
-					   address + embedded_offset);
+    {
+      valaddr += embedded_offset;
+
+      gdb_assert (embedded_offset <= valaddr_len);
+      valaddr_len -= embedded_offset;
+    }
+  value = value_from_contents_and_address_safe (type, valaddr, valaddr_len,
+						address + embedded_offset);
 
   set_value_component_location (value, val);
   /* set_value_component_location resets the address, so we may
diff --git a/gdb/value.c b/gdb/value.c
index 077d234..b291ac4 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1096,7 +1096,7 @@ value_contents_all_raw (struct value *value)
 }
 
 struct type *
-value_enclosing_type (struct value *value)
+value_enclosing_type (const struct value *value)
 {
   return value->enclosing_type;
 }
@@ -3464,15 +3464,18 @@ value_from_contents_and_address_unresolved (struct type *type,
   return v;
 }
 
-/* Create a value of type TYPE whose contents come from VALADDR, if it
-   is non-null, and whose memory address (in the inferior) is
-   ADDRESS.  The type of the created value may differ from the passed
+/* Create a value of type TYPE whose contents come from VALADDR
+   of size VALADDR_LEN, if VALADDR is non-null, and whose memory address
+   (in the inferior) is ADDRESS.  VALADDR_LEN may be larger than
+   required for TYPE.  VALADDR_LEN is ignored if VALADDR is NULL.
+   The type of the created value may differ from the passed
    type TYPE.  Make sure to retrieve values new type after this call.  */
 
 struct value *
-value_from_contents_and_address (struct type *type,
-				 const gdb_byte *valaddr,
-				 CORE_ADDR address)
+value_from_contents_and_address_safe (struct type *type,
+				      const gdb_byte *valaddr,
+				      size_t valaddr_len,
+				      CORE_ADDR address)
 {
   struct type *resolved_type = resolve_dynamic_type (type, address);
   struct type *resolved_type_no_typedef = check_typedef (resolved_type);
@@ -3481,7 +3484,7 @@ value_from_contents_and_address (struct type *type,
   if (valaddr == NULL)
     v = allocate_value_lazy (resolved_type);
   else
-    v = value_from_contents (resolved_type, valaddr);
+    v = value_from_contents_safe (resolved_type, valaddr, valaddr_len);
   if (TYPE_DATA_LOCATION (resolved_type_no_typedef) != NULL
       && TYPE_DATA_LOCATION_KIND (resolved_type_no_typedef) == PROP_CONST)
     address = TYPE_DATA_LOCATION_ADDR (resolved_type_no_typedef);
@@ -3490,19 +3493,44 @@ value_from_contents_and_address (struct type *type,
   return v;
 }
 
-/* Create a value of type TYPE holding the contents CONTENTS.
+/* Like value_from_contents_and_address_safe
+   but do not require VALADDR length.  */
+
+struct value *
+value_from_contents_and_address (struct type *type, const gdb_byte *valaddr,
+				 CORE_ADDR address)
+{
+  return value_from_contents_and_address_safe (type, valaddr, SIZE_MAX,
+					       address);
+}
+
+/* Create a value of type TYPE holding the contents CONTENTS of size
+   CONTENTS_LEN.  CONTENTS_LEN may be larger than required for TYPE.
    The new value is `not_lval'.  */
 
 struct value *
-value_from_contents (struct type *type, const gdb_byte *contents)
+value_from_contents_safe (struct type *type, const gdb_byte *contents,
+			  size_t contents_len)
 {
   struct value *result;
 
+  contents_len = min (contents_len, TYPE_LENGTH (type));
+
   result = allocate_value (type);
-  memcpy (value_contents_raw (result), contents, TYPE_LENGTH (type));
+  memcpy (value_contents_raw (result), contents, contents_len);
+  memset (value_contents_raw (result) + contents_len, 0,
+	  TYPE_LENGTH (type) - contents_len);
   return result;
 }
 
+/* Like value_from_contents_safe but do not require CONTENTS length.  */
+
+struct value *
+value_from_contents (struct type *type, const gdb_byte *contents)
+{
+  return value_from_contents_safe (type, contents, SIZE_MAX);
+}
+
 struct value *
 value_from_double (struct type *type, DOUBLEST num)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 5d4949c..a370681 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -181,7 +181,7 @@ extern int deprecated_value_modifiable (struct value *value);
    `type', and `embedded_offset' is zero, so everything works
    normally.  */
 
-extern struct type *value_enclosing_type (struct value *);
+extern struct type *value_enclosing_type (const struct value *);
 extern void set_value_enclosing_type (struct value *val,
 				      struct type *new_type);
 
@@ -640,9 +640,17 @@ extern struct value *value_at_lazy (struct type *type, CORE_ADDR addr);
 
 extern struct value *value_from_contents_and_address_unresolved
      (struct type *, const gdb_byte *, CORE_ADDR);
+extern struct value *
+  value_from_contents_and_address_safe (struct type *type,
+					const gdb_byte *valaddr,
+					size_t valaddr_len,
+					CORE_ADDR address);
 extern struct value *value_from_contents_and_address (struct type *,
 						      const gdb_byte *,
 						      CORE_ADDR);
+extern struct value *value_from_contents_safe (struct type *type,
+					       const gdb_byte *contents,
+					       size_t contents_len);
 extern struct value *value_from_contents (struct type *, const gdb_byte *);
 
 extern struct value *default_value_from_register (struct gdbarch *gdbarch,