From patchwork Thu Jun 14 09:55:02 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 164875 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 7AFEAB7013 for ; Thu, 14 Jun 2012 19:55:32 +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=1340272535; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Message-ID:Date:From:User-Agent:MIME-Version:To:Subject: Content-Type:Mailing-List:Precedence:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:Sender:Delivered-To; bh=mJhsLyz Y+Z28fSybM0sTK6RZjJs=; b=mIOzhjO6Rr6zQmWPTKfDU/RPfdwZ49/pBJjwjuR +CXfVsr/wDls+G/7ZGTibaYDy+Q+Hqp23hFb71a7EB6TW3RpgEtINPKR4EGKJLop 5WIwdge2mfemr3zDHkxu03TMo0q+bFKTl/zUhZQVlO1CyQ9bkNDiXCORdWAZYEH+ TY7U= 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:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=m+cp3QmUCA9/4ud/K5XTKrZWW3XgUNwLrR0DXIyvDEOD+7AhAWzK+lwm5t1g9c 6gfrqQDJk1dNCEVghYdcebbmjPIarOtu8u8Hdh8WrRiL/0arOej2FW1wA2cj161c SytIR6okc8SFCmY36BuyfartHgrlnQyE0whq52gMwc44I=; Received: (qmail 12894 invoked by alias); 14 Jun 2012 09:55:27 -0000 Received: (qmail 12882 invoked by uid 22791); 14 Jun 2012 09:55:25 -0000 X-SWARE-Spam-Status: No, hits=-6.8 required=5.0 tests=AWL, BAYES_00, KHOP_RCVD_UNTRUST, 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; Thu, 14 Jun 2012 09:55:05 +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 q5E9t5gh022094 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Thu, 14 Jun 2012 05:55:05 -0400 Received: from dhcp-5-241.str.redhat.com (dhcp-5-241.str.redhat.com [10.32.5.241]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q5E9t3oN027289 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Thu, 14 Jun 2012 05:55:04 -0400 Message-ID: <4FD9B4F6.4030200@redhat.com> Date: Thu, 14 Jun 2012 11:55:02 +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: Fix PR c++/19351 (operator new[] overflow) 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 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 Bootstrapped and tested on x86_64-linux-gnu. 2012-06-14 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-14 Florian Weimer * g++.dg/init/new37.C: New test. * 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 bdca799..5103ed9 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) error_at (EXPR_LOC_OR_HERE (inner_nelts), @@ -2256,8 +2274,8 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, if (outer_nelts_from_type && !TREE_CONSTANT (maybe_constant_value (outer_nelts)) && (complain & tf_warning_or_error)) - pedwarn(EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla, - "ISO C++ does not support variable-length array types"); + pedwarn (EXPR_LOC_OR_HERE (outer_nelts), OPT_Wvla, + "ISO C++ does not support variable-length array types"); if (TREE_CODE (elt_type) == VOID_TYPE) { @@ -2311,7 +2329,61 @@ 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. */ + if (max_outer_nelts.high) + { + unsigned shift = HOST_BITS_PER_WIDE_INT - 7 + - clz_hwi (max_outer_nelts.high); + max_outer_nelts.high + = (max_outer_nelts.high >> shift) << shift; + max_outer_nelts.low = 0; + } + else + { + unsigned shift = HOST_BITS_PER_WIDE_INT - 7 + - clz_hwi (max_outer_nelts.low); + max_outer_nelts.low + = (max_outer_nelts.low >> shift) << shift; + } + 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; @@ -2374,10 +2446,11 @@ 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); - } + /* 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. */ @@ -2408,13 +2481,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/new37.C b/gcc/testsuite/g++.dg/init/new37.C new file mode 100644 index 0000000..1645b3d --- /dev/null +++ b/gcc/testsuite/g++.dg/init/new37.C @@ -0,0 +1,63 @@ +// { dg-do compile } + +void +nonconst(int n) +{ + new (long[n][n]); // { dg-error "variable length|array size" } + new long[n][n]; // { dg-error "variable length|array size" } +} + +template +void * +callnew(int n) +{ + return new long[n][T::n]; +} + +template +void * +callnew_fail_1(int n) +{ + return new long[n][T::n]; // { dg-error "variable length|array size" } +} + +template +void * +callnew_fail_2() +{ + return new long[T::n]; // { dg-error "size in array new" } +} + +template +void * +callnew_fail_3() +{ + return new T[2][T::n]; // { dg-error "size of array has non-integral type" } +} + +struct T1 { + static int n; +}; + +struct T2 { + static const double n = 2; // { dg-error "non-integral type" } +}; + +struct T3 { + static const int n = 2; +}; + +struct T4 { + enum { n = 3 }; +}; + +void +test_callnew(int n) +{ + new long[0.2]; // { dg-error "integral or enumeration type" } + callnew_fail_1(n); + callnew_fail_2(); + callnew_fail_3(); + callnew(n); + callnew(n); +} 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; +} +