RFA: PATCH to check for overflow in make_tree_vec_stat

RFA: PATCH to check for overflow in make_tree_vec_stat

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 312 bytes --]

A patch I've been putting together ran into strange memory corruption
issues which turned out to be because the calculation in
make_tree_vec_stat was overflowing and allocating a small TREE_VEC
instead of a large one.  This assert should work as a simple sanity
check.

Tested x86_64-pc-linux-gnu, OK for trunk?

[-- Attachment #2: vec-overflow.diff --]
[-- Type: text/plain, Size: 633 bytes --]

commit 59ccf3b1dd5aaf9611a133ad55d950de525e862d
Author: Jason Merrill <jason@redhat.com>
Date:   Thu May 18 15:23:53 2017 -0400

            * tree.c (make_tree_vec_stat): Check for overflow.

diff --git a/gcc/tree.c b/gcc/tree.c
index 7506725..327332b 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -2270,6 +2270,9 @@ make_tree_vec_stat (int len MEM_STAT_DECL)
   tree t;
   int length = (len - 1) * sizeof (tree) + sizeof (struct tree_vec);
 
+  /* Cheap check for overflow.  */
+  gcc_assert (length > len);
+
   record_node_allocation_statistics (TREE_VEC, length);
 
   t = ggc_alloc_cleared_tree_node_stat (length PASS_MEM_STAT);

Re: RFA: PATCH to check for overflow in make_tree_vec_stat

[Richard Biener]

On Fri, May 19, 2017 at 4:32 AM, Jason Merrill <jason@redhat.com> wrote:
> A patch I've been putting together ran into strange memory corruption
> issues which turned out to be because the calculation in
> make_tree_vec_stat was overflowing and allocating a small TREE_VEC
> instead of a large one.  This assert should work as a simple sanity
> check.

Hmm, looks like 'length' should be size_t?  Then nothing can overflow anymore
(on hosts with size_t 64bit and int 32bit)

> Tested x86_64-pc-linux-gnu, OK for trunk?

Thanks,
Richard.

Re: RFA: PATCH to check for overflow in make_tree_vec_stat

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

On Fri, May 19, 2017 at 5:14 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Fri, May 19, 2017 at 4:32 AM, Jason Merrill <jason@redhat.com> wrote:
>> A patch I've been putting together ran into strange memory corruption
>> issues which turned out to be because the calculation in
>> make_tree_vec_stat was overflowing and allocating a small TREE_VEC
>> instead of a large one.  This assert should work as a simple sanity
>> check.
>
> Hmm, looks like 'length' should be size_t?  Then nothing can overflow anymore
> (on hosts with size_t 64bit and int 32bit)

Sure.  I imagine that anything trying to create a TREE_VEC that large
is going to have a bad time for other reasons, but those will probably
be more obvious.

Applying this:

Jason

[-- Attachment #2: vec-size_t.diff --]
[-- Type: text/plain, Size: 1108 bytes --]

commit 2a5c0a40193350e57407165d17afe427a9f42325
Author: Jason Merrill <jason@redhat.com>
Date:   Thu May 18 15:23:53 2017 -0400

            * tree.c (make_tree_vec_stat, grow_tree_vec_stat): Use size_t.

diff --git a/gcc/tree.c b/gcc/tree.c
index 7506725..db31620 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -2268,7 +2268,7 @@ tree
 make_tree_vec_stat (int len MEM_STAT_DECL)
 {
   tree t;
-  int length = (len - 1) * sizeof (tree) + sizeof (struct tree_vec);
+  size_t length = (len - 1) * sizeof (tree) + sizeof (struct tree_vec);
 
   record_node_allocation_statistics (TREE_VEC, length);
 
@@ -2290,8 +2290,8 @@ grow_tree_vec_stat (tree v, int len MEM_STAT_DECL)
   int oldlen = TREE_VEC_LENGTH (v);
   gcc_assert (len > oldlen);
 
-  int oldlength = (oldlen - 1) * sizeof (tree) + sizeof (struct tree_vec);
-  int length = (len - 1) * sizeof (tree) + sizeof (struct tree_vec);
+  size_t oldlength = (oldlen - 1) * sizeof (tree) + sizeof (struct tree_vec);
+  size_t length = (len - 1) * sizeof (tree) + sizeof (struct tree_vec);
 
   record_node_allocation_statistics (TREE_VEC, length - oldlength);