This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR19351, C++] Fix heap overflow in operator new[]
* 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 <fw@deneb.enyo.de>
* c.opt (foperator-new-overflow-check): New.
2011-02-21 Florian Weimer <fw@deneb.enyo.de>
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<size_t>::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