From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15796 invoked by alias); 17 Aug 2012 09:35:34 -0000 Received: (qmail 15569 invoked by uid 22791); 17 Aug 2012 09:35:30 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,MAY_BE_FORGED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD,SPF_HELO_PASS,TW_CP,TW_XZ X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 17 Aug 2012 09:34:53 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q7H9Yr6C002395 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 17 Aug 2012 05:34:53 -0400 Received: from spoyarek (dhcp223-8.pnq.redhat.com [10.65.223.8] (may be forged)) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q7H9YmAo011888; Fri, 17 Aug 2012 05:34:51 -0400 Date: Fri, 17 Aug 2012 09:35:00 -0000 From: Siddhesh Poyarekar To: Jan Kratochvil Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] bitpos: Additional checks to ensure that a type fits into size_t Message-ID: <20120817145350.0802572c@spoyarek> In-Reply-To: <20120811111335.459a602c@spoyarek> References: <20120805005411.5fc2c25a@spoyarek> <20120811111335.459a602c@spoyarek> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="MP_/qO9J+WWVEMPlvfLDMzWog0T" Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2012-08/txt/msg00476.txt.bz2 --MP_/qO9J+WWVEMPlvfLDMzWog0T Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Content-Disposition: inline Content-length: 1744 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. --MP_/qO9J+WWVEMPlvfLDMzWog0T Content-Type: text/x-patch Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename=bitpos-expand-ensure-sizet-3.patch Content-length: 5971 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. */ --MP_/qO9J+WWVEMPlvfLDMzWog0T--