From patchwork Tue Jun 26 14:29:46 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 167410 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id CE3BAB700D for ; Wed, 27 Jun 2012 00:30:18 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1341325820; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: References:In-Reply-To:Content-Type:Mailing-List:Precedence: List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=La5HWzaiF1SAnAUMj8bcSA+V3SI=; b=LaoeKIQZzrNyk7e 6nY93EtT1N7h3wYW6RnuUQC+M//X7Kl2OWAi9cEoh5RZWyVKH47+agcN8Wq9f/pX h60zo5VH9EJBUYciIkJK2rOYdnxQkgqur3NhhiFM0Vx0wJqJ7Gu3VUZB+/mPXvOA TY7lqEE3uS5biBtvSqZH1L6oCBDM= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:References:In-Reply-To:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=ZIt1bCYgmQOnKHJmGvVQBQ3VFY1KfeylvhBfYCSljH85vi5rwM91HLW7nbAFJP rDueTNpe3j97zfOnoLfzWjuOjhTwbrOvSfKS3eF/FkC5MImSYZUAmWZF+8WrKeO8 q3zFXX0BJ3smIrOhgq6aGdWEG7sw/E/TkAhecACwiP+eE=; Received: (qmail 21230 invoked by alias); 26 Jun 2012 14:30:10 -0000 Received: (qmail 21132 invoked by uid 22791); 26 Jun 2012 14:30:04 -0000 X-SWARE-Spam-Status: No, hits=-7.6 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, KHOP_THREADED, RCVD_IN_DNSWL_HI, RCVD_IN_HOSTKARMA_W, SPF_HELO_PASS, TW_CL, T_RP_MATCHES_RCVD 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; Tue, 26 Jun 2012 14:29:48 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q5QETmpE005781 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Tue, 26 Jun 2012 10:29:48 -0400 Received: from dhcp-5-241.str.redhat.com (dhcp-5-241.str.redhat.com [10.32.5.241]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q5QETknr023720 (version=TLSv1/SSLv3 cipher=DHE-RSA-CAMELLIA256-SHA bits=256 verify=NO) for ; Tue, 26 Jun 2012 10:29:47 -0400 Message-ID: <4FE9C75A.8030000@redhat.com> Date: Tue, 26 Jun 2012 16:29:46 +0200 From: Florian Weimer User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: gcc-patches@gcc.gnu.org Subject: Re: Fix PR c++/19351 (operator new[] overflow) References: <4FD9B4F6.4030200@redhat.com> In-Reply-To: <4FD9B4F6.4030200@redhat.com> X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On 06/14/2012 11:55 AM, Florian Weimer wrote: > This is another attempt at ensuring that operator new[] always returns a > block of sufficient size. > > This is on top of my previous patch rejecting VLA allocations: > http://gcc.gnu.org/ml/gcc-patches/2012-06/msg00616.html I've committed the patch referenced above, and rebased this patch. I also made two additional changes: The Hamming weight reduction (to help encoding the maximum size constant on architectures like ARM) was unnecessarily aggressive, and I had forgotten to disable cookies for non-cookie types, leading to failures when testing Java. Bootstrapped and tested on x86_86-unknown-linux-gnu, with no new regressions (this time including Java). Okay for trunk? 2012-06-26 Florian Weimer PR c++/19351 * call.c (build_operator_new_call): Add size_check argument and evaluate it. * cp-tree.h (build_operator_new_call): Adjust declaration. * init.c (build_new_1): Compute array size check and apply it. 2012-06-26 Florian Weimer * g++.dg/init/new38.C: New test. * g++.dg/init/new39.C: New test. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 09965b3..806e0ba 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -3943,15 +3943,19 @@ build_new_function_call (tree fn, VEC(tree,gc) **args, bool koenig_p, total number of bytes required by the allocation, and is updated if that is changed here. *COOKIE_SIZE is non-NULL if a cookie should be used. If this function determines that no cookie should be - used, after all, *COOKIE_SIZE is set to NULL_TREE. If FN is - non-NULL, it will be set, upon return, to the allocation function - called. */ + used, after all, *COOKIE_SIZE is set to NULL_TREE. If SIZE_CHECK + is not NULL_TREE, it is evaluated before calculating the final + array size, and if it fails, the array size is replaced with + (size_t)-1 (usually triggering a std::bad_alloc exception). If FN + is non-NULL, it will be set, upon return, to the allocation + function called. */ tree build_operator_new_call (tree fnname, VEC(tree,gc) **args, - tree *size, tree *cookie_size, + tree *size, tree *cookie_size, tree size_check, tree *fn, tsubst_flags_t complain) { + tree original_size = *size; tree fns; struct z_candidate *candidates; struct z_candidate *cand; @@ -3959,6 +3963,10 @@ build_operator_new_call (tree fnname, VEC(tree,gc) **args, if (fn) *fn = NULL_TREE; + /* Set to (size_t)-1 if the size check fails. */ + if (size_check != NULL_TREE) + *size = fold_build3 (COND_EXPR, sizetype, size_check, + original_size, TYPE_MAX_VALUE (sizetype)); VEC_safe_insert (tree, gc, *args, 0, *size); *args = resolve_args (*args, complain); if (*args == NULL) @@ -4022,7 +4030,11 @@ build_operator_new_call (tree fnname, VEC(tree,gc) **args, if (use_cookie) { /* Update the total size. */ - *size = size_binop (PLUS_EXPR, *size, *cookie_size); + *size = size_binop (PLUS_EXPR, original_size, *cookie_size); + /* Set to (size_t)-1 if the size check fails. */ + gcc_assert (size_check != NULL_TREE); + *size = fold_build3 (COND_EXPR, sizetype, size_check, + *size, TYPE_MAX_VALUE (sizetype)); /* Update the argument list to reflect the adjusted size. */ VEC_replace (tree, *args, 0, *size); } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index a4b7ae3..9033108 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -4879,7 +4879,7 @@ extern tree build_user_type_conversion (tree, tree, int, extern tree build_new_function_call (tree, VEC(tree,gc) **, bool, tsubst_flags_t); extern tree build_operator_new_call (tree, VEC(tree,gc) **, tree *, - tree *, tree *, + tree *, tree, tree *, tsubst_flags_t); extern tree build_new_method_call (tree, tree, VEC(tree,gc) **, tree, int, tree *, diff --git a/gcc/cp/init.c b/gcc/cp/init.c index 5a81643..2f2ef69 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -2175,7 +2175,10 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, tree pointer_type; tree non_const_pointer_type; tree outer_nelts = NULL_TREE; + /* For arrays, a bounds checks on the NELTS parameter. */ + tree outer_nelts_check = NULL_TREE; bool outer_nelts_from_type = false; + double_int inner_nelts_count = double_int_one; tree alloc_call, alloc_expr; /* The address returned by the call to "operator new". This node is a VAR_DECL and is therefore reusable. */ @@ -2228,7 +2231,22 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, { tree inner_nelts = array_type_nelts_top (elt_type); tree inner_nelts_cst = maybe_constant_value (inner_nelts); - if (!TREE_CONSTANT (inner_nelts_cst)) + if (TREE_CONSTANT (inner_nelts_cst) + && TREE_CODE (inner_nelts_cst) == INTEGER_CST) + { + double_int result; + if (mul_double (TREE_INT_CST_LOW (inner_nelts_cst), + TREE_INT_CST_HIGH (inner_nelts_cst), + inner_nelts_count.low, inner_nelts_count.high, + &result.low, &result.high)) + { + if (complain & tf_error) + error ("integer overflow in array size"); + nelts = error_mark_node; + } + inner_nelts_count = result; + } + else { if (complain & tf_error) { @@ -2318,7 +2336,56 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, size = size_in_bytes (elt_type); if (array_p) - size = size_binop (MULT_EXPR, size, convert (sizetype, nelts)); + { + /* Maximum available size in bytes. Half of the address space + minus the cookie size. */ + double_int max_size + = double_int_lshift (double_int_one, TYPE_PRECISION (sizetype) - 1, + HOST_BITS_PER_DOUBLE_INT, false); + /* Size of the inner array elements. */ + double_int inner_size; + /* Maximum number of outer elements which can be allocated. */ + double_int max_outer_nelts; + tree max_outer_nelts_tree; + + gcc_assert (TREE_CODE (size) == INTEGER_CST); + cookie_size = targetm.cxx.get_cookie_size (elt_type); + gcc_assert (TREE_CODE (cookie_size) == INTEGER_CST); + gcc_checking_assert (double_int_ucmp + (TREE_INT_CST (cookie_size), max_size) < 0); + /* Unconditionally substract the array size. This decreases the + maximum object size and is safe even if we choose not to use + a cookie after all. */ + max_size = double_int_sub (max_size, TREE_INT_CST (cookie_size)); + if (mul_double (TREE_INT_CST_LOW (size), TREE_INT_CST_HIGH (size), + inner_nelts_count.low, inner_nelts_count.high, + &inner_size.low, &inner_size.high) + || double_int_ucmp (inner_size, max_size) > 0) + { + if (complain & tf_error) + error ("size of array is too large"); + return error_mark_node; + } + max_outer_nelts = double_int_udiv (max_size, inner_size, TRUNC_DIV_EXPR); + /* Only keep the top-most seven bits, to simplify encoding the + constant in the instruction stream. */ + { + unsigned shift = HOST_BITS_PER_DOUBLE_INT - 7 + - (max_outer_nelts.high ? clz_hwi (max_outer_nelts.high) + : (HOST_BITS_PER_WIDE_INT + clz_hwi (max_outer_nelts.low))); + max_outer_nelts + = double_int_lshift (double_int_rshift + (max_outer_nelts, shift, + HOST_BITS_PER_DOUBLE_INT, false), + shift, HOST_BITS_PER_DOUBLE_INT, false); + } + max_outer_nelts_tree = double_int_to_tree (sizetype, max_outer_nelts); + + size = size_binop (MULT_EXPR, size, convert (sizetype, nelts)); + outer_nelts_check = fold_build2 (LE_EXPR, boolean_type_node, + outer_nelts, + max_outer_nelts_tree); + } alloc_fn = NULL_TREE; @@ -2381,10 +2448,13 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, /* Use a class-specific operator new. */ /* If a cookie is required, add some extra space. */ if (array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type)) - { - cookie_size = targetm.cxx.get_cookie_size (elt_type); - size = size_binop (PLUS_EXPR, size, cookie_size); - } + size = size_binop (PLUS_EXPR, size, cookie_size); + else + cookie_size = NULL_TREE; + /* Perform the overflow check. */ + if (outer_nelts_check != NULL_TREE) + size = fold_build3 (COND_EXPR, sizetype, outer_nelts_check, + size, TYPE_MAX_VALUE (sizetype)); /* Create the argument list. */ VEC_safe_insert (tree, gc, *placement, 0, size); /* Do name-lookup to find the appropriate operator. */ @@ -2415,13 +2485,12 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, { /* Use a global operator new. */ /* See if a cookie might be required. */ - if (array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type)) - cookie_size = targetm.cxx.get_cookie_size (elt_type); - else + if (!(array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type))) cookie_size = NULL_TREE; alloc_call = build_operator_new_call (fnname, placement, &size, &cookie_size, + outer_nelts_check, &alloc_fn, complain); } } diff --git a/gcc/testsuite/g++.dg/init/new38.C b/gcc/testsuite/g++.dg/init/new38.C new file mode 100644 index 0000000..1672f22 --- /dev/null +++ b/gcc/testsuite/g++.dg/init/new38.C @@ -0,0 +1,54 @@ +// { dg-do compile } + +void +large_array_char(int n) +{ + new char[n] + [1ULL << (sizeof(void *) * 4)] + [1ULL << (sizeof(void *) * 4)]; // { dg-error "size of array" } +} + +template +void +large_array_char_template(int n) +{ + new char[n] + [1ULL << (sizeof(void *) * 4)] + [1ULL << (sizeof(void *) * 4)]; // { dg-error "size of array" } +} + + +template +void +large_array_template1(int n) +{ + new T[n] // { dg-error "size of array is too large" } + [(1ULL << (sizeof(void *) * 4)) / sizeof(T)] + [1ULL << (sizeof(void *) * 4)]; +} + +template +void +large_array_template2(int n) +{ + new T[n] // { dg-error "size of array is too large" } + [(1ULL << (sizeof(void *) * 4)) / sizeof(T)] + [1ULL << (sizeof(void *) * 4)]; +} + +template +void +large_array_template3(int n) +{ + new T[n] // { dg-error "size of array is too large" } + [(1ULL << (sizeof(void *) * 4)) / sizeof(T)] + [1ULL << (sizeof(void *) * 4)]; +} + +void +call_large_array_template(int n) +{ + large_array_template1(n); + large_array_template2(n); + large_array_template3(n); +} diff --git a/gcc/testsuite/g++.dg/init/new39.C b/gcc/testsuite/g++.dg/init/new39.C new file mode 100644 index 0000000..f274ebb --- /dev/null +++ b/gcc/testsuite/g++.dg/init/new39.C @@ -0,0 +1,68 @@ +// Testcase for overflow handling in operator new[]. +// { dg-do run } + +#include +#include + +struct without_new { + char bar[256]; +}; + +struct with_new { + char bar[256]; + void *operator new[] (size_t sz) + { + if (sz != -1) + abort (); + throw std::bad_alloc(); + } +}; + +template +inline void +test (size_t s) +{ + try { + new T[s]; + abort (); + } catch (std::bad_alloc &) { + } +} + +template +void +test_noopt (size_t s) __attribute__((noinline)); + +template +void +test_noopt (size_t s) +{ + __asm__ (""); + test (s); +} + +template +void +all_tests () +{ + test(-1); + test(size_t(-1) / sizeof (T) + 1); + test(size_t(-1) / sizeof (T) + 2); + test_noopt(-1); + test_noopt(size_t(-1) / sizeof (T) + 1); + test_noopt(size_t(-1) / sizeof (T) + 2); +} + +int +main () +{ + try { + ::operator new(size_t(-1)); + abort (); + } catch (std::bad_alloc &) { + } + all_tests (); + all_tests (); + return 0; +} +