This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR19351, C++] Fix heap overflow in operator new[]


Sorry it's taken so long to review this.

On 02/21/2011 04:05 PM, Florian Weimer wrote:
 build_operator_new_call (tree fnname, VEC(tree,gc) **args,
-                        tree *size, tree *cookie_size,
+                        tree *size, tree size_with_cookie, tree *cookie_size,

We don't need both size_with_cookie and cookie_size here. Let's change the cookie_size parameter to be size_with_cookie instead (and adjust the places in build_new_1 that check for cookie_size being set or not).


+   flag_new_overflow_check is true.
+ */

Close comment on the last line of text rather than the next line.


+  if (!flag_new_overflow_check)
+    return result;

Let's check for constant results here. If we have a TREE_CONSTANT result that overflows, we can handle it even if we aren't emitting the checks for non-constant values. And if we have a TREE_CONSTANT result that doesn't overflow, we don't have to bother building up the complicated trees in order to fold them away.


+  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)));

Go ahead and fold here. The C++ front end has not yet been converted to delay folding like the C front end.


+static tree
+build_new_size_expr (tree elt_type, tree nelts, tree cookie_size)

This function needs a comment.


+    nelts = maybe_build_size_mult_saturated
+      (convert (sizetype, nelts),
+       convert (sizetype, array_type_nelts_top (elt_type)),
+       NULL_TREE);

It seems unfortunate to do this runtime checking more than once for multi-dimensional new. Since the element size and all but one of the array dimensions are going to be constant in most cases, let's try to multiply the element size with the constant dimensions before we bring in the potentially non-constant dimension.


    I still need some guidance where to put the test case.  There don't
    seem to be any direct tests for operator new[] funcionality.

Let's put it in g++.dg/init.


Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]