Re: [PATCH 2/3] bitpos: Additional checks to ensure that a type fits into size_t

[Siddhesh Poyarekar <siddhesh at redhat dot com>]

Hi,

Here's another update to the ensure_size_t patch. The following changes
have been made to the patch:

* The function is now called ulongest_fits_host_or_error and is now
  in utils.c. I have named it so to give room for a future
  'longest_fits_host_orr_error' if needed. They have to be different
  because the error message will have to be printed differently
  (plongest vs pulongest). The name also looks neater in general.
* I have reverted the size_t checks from most tdep files in favour of
  promoting the value_contents call ahead. In fact, barring
  h8300h_return_value, all of them have an earlier value_contents call,
  making the check unnecessary.

This change applies on top of the bitpos patch and I have verified that
the change does not cause any regressions.

Regards,
Siddhesh

gdb/ChangeLog

	* alpha-tdep.c (alpha_push_dummy_call) Check for underflow in
	SP.
	* cp-valprint (cp_print_value): Ensure BASECLASS fits into
	size_t.
	* dwarf2loc.c (read_pieced_value): Ensure that THIS_SIZE fits
	into size_t.
	(write_pieced_value): Likewise.
	* h8300-tdep.c (h8300h_return_value): Ensure that TYPE fits
	into size_t.
	* p-valprint (pascal_object_print_value): Ensure BASECLASS fits
	into size_t.
	* utils.c (ulongest_fits_host_or_error): New function to find if
	a ULONGEST number fits into size_t.
	* utils.h: Declare ulongest_fits_host_or_error.
	* valops.c (search_struct_method): Ensure BASECLASS fits into
	size_t.
	* value.c (allocate_value_lazy): Ensure TYPE fits into size_t.
	(allocate_value_contents): Likewise.
	(set_value_enclosing_type): Ensure NEW_ENCL_TYPE fits into
	size_t.
	* xstormy16-tdep.c (xstormy16_push_dummy_call): Call
	value_contents earlier to ensure that the argument fits into
	host memory.

diff --git a/gdb/alpha-tdep.c b/gdb/alpha-tdep.c
index 7a6f38d..0ce378f 100644
--- a/gdb/alpha-tdep.c
+++ b/gdb/alpha-tdep.c
@@ -414,6 +414,13 @@ alpha_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
     accumulate_size = 0;
   else
     accumulate_size -= sizeof(arg_reg_buffer);
+
+  /* Check for underflow.  */
+  if (sp - accumulate_size > sp)
+    error (_("Insufficient memory in GDB host for arguments, "
+	     "need %s bytes, but less than %s bytes available."),
+	   plongest (accumulate_size), plongest (CORE_ADDR_MAX - sp));
+
   sp -= accumulate_size;
 
   /* Keep sp aligned to a multiple of 16 as the ABI requires.  */
diff --git a/gdb/cp-valprint.c b/gdb/cp-valprint.c
index 8bc329e..2373419 100644
--- a/gdb/cp-valprint.c
+++ b/gdb/cp-valprint.c
@@ -558,6 +558,8 @@ cp_print_value (struct type *type, struct type *real_type,
 		  gdb_byte *buf;
 		  struct cleanup *back_to;
 
+		  ulongest_fits_host_or_error (TYPE_LENGTH (baseclass));
+
 		  buf = xmalloc (TYPE_LENGTH (baseclass));
 		  back_to = make_cleanup (xfree, buf);
 
diff --git a/gdb/dwarf2loc.c b/gdb/dwarf2loc.c
index bef4355..7106805 100644
--- a/gdb/dwarf2loc.c
+++ b/gdb/dwarf2loc.c
@@ -1593,6 +1593,8 @@ read_pieced_value (struct value *v)
 
       this_size = (this_size_bits + source_offset_bits % 8 + 7) / 8;
       source_offset = source_offset_bits / 8;
+      ulongest_fits_host_or_error (this_size);
+
       if (buffer_size < this_size)
 	{
 	  buffer_size = this_size;
@@ -1784,6 +1786,7 @@ write_pieced_value (struct value *to, struct value *from)
 	}
       else
 	{
+	  ulongest_fits_host_or_error (this_size);
 	  if (buffer_size < this_size)
 	    {
 	      buffer_size = this_size;
diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index ee5aa2d..c2e786f 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -925,6 +925,7 @@ h8300h_return_value (struct gdbarch *gdbarch, struct value *function,
 	  ULONGEST addr;
 
 	  regcache_raw_read_unsigned (regcache, E_R0_REGNUM, &addr);
+	  ulongest_fits_host_or_error (TYPE_LENGTH (type));
 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
 	}
 
diff --git a/gdb/p-valprint.c b/gdb/p-valprint.c
index 56f9e30..c153669 100644
--- a/gdb/p-valprint.c
+++ b/gdb/p-valprint.c
@@ -791,6 +791,7 @@ pascal_object_print_value (struct type *type, const gdb_byte *valaddr,
 	      gdb_byte *buf;
 	      struct cleanup *back_to;
 
+	      ulongest_fits_host_or_error (TYPE_LENGTH (baseclass));
 	      buf = xmalloc (TYPE_LENGTH (baseclass));
 	      back_to = make_cleanup (xfree, buf);
 
diff --git a/gdb/utils.c b/gdb/utils.c
index 607d7bc..f6602b5 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -3141,6 +3141,18 @@ host_address_to_string (const void *addr)
   return str;
 }
 
+/* Ensure that the input NUM is not larger than the maximum capacity of the
+   host system.  We choose SIZE_MAX / 8 as a conservative estimate of the size
+   of a resource that a system may allocate.  */
+void
+ulongest_fits_host_or_error (ULONGEST num)
+{
+  if (num > SIZE_MAX / 8)
+    error (_("Insufficient memory in host GDB for object of size %s bytes, "
+	     "maximum allowed %s bytes."), pulongest (num),
+	   pulongest (SIZE_MAX / 8));
+}
+
 char *
 gdb_realpath (const char *filename)
 {
diff --git a/gdb/utils.h b/gdb/utils.h
index 4bb6ac8..d7d2e1a 100644
--- a/gdb/utils.h
+++ b/gdb/utils.h
@@ -23,6 +23,8 @@
 
 #include "cleanups.h"
 
+extern void ulongest_fits_host_or_error (ULONGEST num);
+
 extern void initialize_utils (void);
 
 /* String utilities.  */
diff --git a/gdb/valops.c b/gdb/valops.c
index 7c0a478..1708a4b 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2275,6 +2275,7 @@ search_struct_method (const char *name, struct value **arg1p,
 	      struct cleanup *back_to;
 	      CORE_ADDR address;
 
+	      ulongest_fits_host_or_error (TYPE_LENGTH (baseclass));
 	      tmp = xmalloc (TYPE_LENGTH (baseclass));
 	      back_to = make_cleanup (xfree, tmp);
 	      address = value_address (*arg1p);
diff --git a/gdb/value.c b/gdb/value.c
index 09ecb70..7c3bdb4 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -662,6 +662,7 @@ allocate_value_lazy (struct type *type)
      description correctly.  */
   check_typedef (type);
 
+  ulongest_fits_host_or_error (TYPE_LENGTH (type));
   val = (struct value *) xzalloc (sizeof (struct value));
   val->contents = NULL;
   val->next = all_values;
@@ -693,6 +694,8 @@ allocate_value_lazy (struct type *type)
 void
 allocate_value_contents (struct value *val)
 {
+  ulongest_fits_host_or_error (TYPE_LENGTH (val->enclosing_type));
+
   if (!val->contents)
     val->contents = (gdb_byte *) xzalloc (TYPE_LENGTH (val->enclosing_type));
 }
@@ -2602,8 +2605,12 @@ void
 set_value_enclosing_type (struct value *val, struct type *new_encl_type)
 {
   if (TYPE_LENGTH (new_encl_type) > TYPE_LENGTH (value_enclosing_type (val))) 
-    val->contents =
-      (gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    {
+      ulongest_fits_host_or_error (TYPE_LENGTH (new_encl_type));
+
+      val->contents =
+	(gdb_byte *) xrealloc (val->contents, TYPE_LENGTH (new_encl_type));
+    }
 
   val->enclosing_type = new_encl_type;
 }
diff --git a/gdb/xstormy16-tdep.c b/gdb/xstormy16-tdep.c
index d73e2d6..1667c14 100644
--- a/gdb/xstormy16-tdep.c
+++ b/gdb/xstormy16-tdep.c
@@ -282,11 +282,15 @@ xstormy16_push_dummy_call (struct gdbarch *gdbarch,
   for (j = nargs - 1; j >= i; j--)
     {
       char *val;
+      const gdb_byte *bytes = value_contents (args[j]);
+      struct type *t = value_enclosing_type (args[j]);
 
-      typelen = TYPE_LENGTH (value_enclosing_type (args[j]));
+      typelen = TYPE_LENGTH (t);
       slacklen = typelen & 1;
+
+      /* FIXME: This alloca is dangerous.  */
       val = alloca (typelen + slacklen);
-      memcpy (val, value_contents (args[j]), typelen);
+      memcpy (val, bytes, typelen);
       memset (val + typelen, 0, slacklen);
 
       /* Now write this data to the stack.  The stack grows upwards.  */