From patchwork Mon Feb 21 21:05:08 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 83876 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 955EBB717A for ; Tue, 22 Feb 2011 08:05:24 +1100 (EST) Received: (qmail 9713 invoked by alias); 21 Feb 2011 21:05:22 -0000 Received: (qmail 9702 invoked by uid 22791); 21 Feb 2011 21:05:21 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL, BAYES_00, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from ka.mail.enyo.de (HELO ka.mail.enyo.de) (87.106.162.201) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 21 Feb 2011 21:05:15 +0000 Received: from [172.17.135.4] (helo=deneb.enyo.de) by ka.mail.enyo.de with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) id 1PrcwP-00060t-0A; Mon, 21 Feb 2011 22:05:09 +0100 Received: from fw by deneb.enyo.de with local (Exim 4.72) (envelope-from ) id 1PrcwO-0002Lc-Pf; Mon, 21 Feb 2011 22:05:08 +0100 From: Florian Weimer To: Mark Mitchell Cc: Richard Guenther , gcc-patches@gcc.gnu.org, Jason Merrill Subject: Re: [PR19351, C++] Fix heap overflow in operator new[] References: <873a1eydec.fsf@mid.deneb.enyo.de> <87d3noemb8.fsf@mid.deneb.enyo.de> <87sjw7igbw.fsf@mid.deneb.enyo.de> <4D4DCD34.4050201@codesourcery.com> <87bp2pfshg.fsf@mid.deneb.enyo.de> <4D4F8CD3.1040405@codesourcery.com> <87tygg1hqs.fsf@mid.deneb.enyo.de> <4D4F924A.3010504@codesourcery.com> <877hdbd2cn.fsf@mid.deneb.enyo.de> Date: Mon, 21 Feb 2011 22:05:08 +0100 In-Reply-To: <877hdbd2cn.fsf@mid.deneb.enyo.de> (Florian Weimer's message of "Mon, 07 Feb 2011 21:19:36 +0100") Message-ID: <87ipwd6quz.fsf@mid.deneb.enyo.de> MIME-Version: 1.0 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 * Florian Weimer: > (I hope I'll have the patch with the option ready soon, perhaps > tomorrow.) Sorry, took much longer than expected. The default is currently on. I have run "make check-c++" with no new failures on x86_64-gnu-linux twice, with the operator new[] check enabled and disabled; there were no new failures. If the check is disabled, trunk and patch produce identical assembler code for the test case. I still need some guidance where to put the test case. There don't seem to be any direct tests for operator new[] funcionality. 2011-02-21 Florian Weimer * c.opt (foperator-new-overflow-check): New. 2011-02-21 Florian Weimer PR c++/19351 * init.c (maybe_build_size_mult_saturated, build_new_size_expr): Add. (build_new_1): Use these new functions in size calculations. * call.c (build_operator_new_call): Add size_with_cookie argument. * cp-tree.h (build_operator_new_call): Likewise. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index bb928fa..78e4020 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -898,6 +898,10 @@ foperator-names C++ ObjC++ Recognize C++ keywords like \"compl\" and \"xor\" +foperator-new-overflow-check +C++ ObjC++ Var(flag_new_overflow_check) Init(1) +Check for overflow during operator new[] + foptional-diags C++ ObjC++ Ignore Does nothing. Preserved for backward compatibility. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index 8dccbbe..15bc570 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -3635,7 +3635,7 @@ build_new_function_call (tree fn, VEC(tree,gc) **args, bool koenig_p, tree build_operator_new_call (tree fnname, VEC(tree,gc) **args, - tree *size, tree *cookie_size, + tree *size, tree size_with_cookie, tree *cookie_size, tree *fn) { tree fns; @@ -3706,7 +3706,7 @@ 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_with_cookie; /* 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 238d0cf..4598f10 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -4614,7 +4614,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 *); extern tree build_new_method_call (tree, tree, VEC(tree,gc) **, tree, int, tree *, tsubst_flags_t); diff --git a/gcc/cp/init.c b/gcc/cp/init.c index e590118..87a90a4 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -1913,6 +1913,43 @@ diagnose_uninitialized_cst_or_ref_member (tree type, bool using_new, bool compla return diagnose_uninitialized_cst_or_ref_member_1 (type, type, using_new, complain); } +/* Multiplies MUL1 with MUL2, and adds ADD. Returns + std::limits::max() if the result cannot be be represented + as a size_t value. If ADD is null_tree, treat it as a zero + constant. Uses saturated arithmetic only if + flag_new_overflow_check is true. + */ +static tree +maybe_build_size_mult_saturated (tree mul1, tree mul2, tree add) +{ + tree max_mul1, result; + result = size_binop (MULT_EXPR, mul1, mul2); + if (add != NULL_TREE) + result = size_binop (PLUS_EXPR, result, add); + if (!flag_new_overflow_check) + return result; + max_mul1 = TYPE_MAX_VALUE (size_type_node); + if (add != NULL_TREE) + max_mul1 = build2 (MINUS_EXPR, size_type_node, max_mul1, add); + max_mul1 = build2 (TRUNC_DIV_EXPR, size_type_node, max_mul1, mul2); + return build3 (COND_EXPR, size_type_node, + build2 (EQ_EXPR, size_type_node, mul2, size_zero_node), + add == NULL_TREE ? size_zero_node : add, + build3 (COND_EXPR, size_type_node, + build2 (LE_EXPR, size_type_node, mul1, max_mul1), + result, TYPE_MAX_VALUE (size_type_node))); +} + +static tree +build_new_size_expr (tree elt_type, tree nelts, tree cookie_size) +{ + tree elt_size = size_in_bytes (elt_type); + if (nelts == NULL_TREE) + return elt_size; + return maybe_build_size_mult_saturated + (convert (sizetype, nelts), elt_size, cookie_size); +} + /* Generate code for a new-expression, including calling the "operator new" function, initializing the object, and, if an exception occurs during construction, cleaning up. The arguments are as for @@ -1982,10 +2019,10 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, for (elt_type = type; TREE_CODE (elt_type) == ARRAY_TYPE; elt_type = TREE_TYPE (elt_type)) - nelts = cp_build_binary_op (input_location, - MULT_EXPR, nelts, - array_type_nelts_top (elt_type), - complain); + nelts = maybe_build_size_mult_saturated + (convert (sizetype, nelts), + convert (sizetype, array_type_nelts_top (elt_type)), + NULL_TREE); if (TREE_CODE (elt_type) == VOID_TYPE) { @@ -2037,10 +2074,6 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, return error_mark_node; } - size = size_in_bytes (elt_type); - if (array_p) - size = size_binop (MULT_EXPR, size, convert (sizetype, nelts)); - alloc_fn = NULL_TREE; /* If PLACEMENT is a single simple pointer type not passed by @@ -2104,8 +2137,8 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, 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 = build_new_size_expr (elt_type, nelts, cookie_size); /* Create the argument list. */ VEC_safe_insert (tree, gc, *placement, 0, size); /* Do name-lookup to find the appropriate operator. */ @@ -2136,14 +2169,24 @@ build_new_1 (VEC(tree,gc) **placement, tree type, tree nelts, { /* Use a global operator new. */ /* See if a cookie might be required. */ + tree size_with_cookie; + size = build_new_size_expr (elt_type, nelts, NULL_TREE); if (array_p && TYPE_VEC_NEW_USES_COOKIE (elt_type)) - cookie_size = targetm.cxx.get_cookie_size (elt_type); + { + cookie_size = targetm.cxx.get_cookie_size (elt_type); + size_with_cookie = build_new_size_expr + (elt_type, nelts, cookie_size); + } else - cookie_size = NULL_TREE; + { + cookie_size = NULL_TREE; + size_with_cookie = size; + } - alloc_call = build_operator_new_call (fnname, placement, - &size, &cookie_size, - &alloc_fn); + alloc_call = build_operator_new_call + (fnname, placement, + &size, size_with_cookie, &cookie_size, + &alloc_fn); } } diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 5295c39..c066693 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -1998,6 +1998,18 @@ Do not treat the operator name keywords @code{and}, @code{bitand}, @code{bitor}, @code{compl}, @code{not}, @code{or} and @code{xor} as synonyms as keywords. +@item -foperator-new-overflow-check +@itemx -fno-operator-new-overflow-check +@opindex foperator-new-overflow-check +@opindex fno-operator-new-overflow-check +If @option{-foperator-new-overflow-check} is specified, @code{operator +new[]} expressions will throw @code{std::bad_alloc} if the required size +cannot be represented as a value of type @code{size_t}. With +@option{-fno-operator-new-overflow-check}, no such checks are performed +and @code{operator new[]} may return a pointer to a memory area which +is too small for the requested number of elements. The default is to +perform the check, which increases code size slightly. + @item -fno-optional-diags @opindex fno-optional-diags Disable diagnostics that the standard says a compiler does not need to