[PATCH] Fix ICE in build_classic_dist_vector_1 and memory leaks in tree-data-ref (PR tree-optimization/32573, middle-end/32946)
Jakub Jelinek
jakub@redhat.com
Tue Aug 21 15:49:00 GMT 2007
Hi!
The attached testcase ICEs in build_classic_dist_vector_1.
The problem is that subscript_dependence_tester_1 earlier called
finalize_ddr_dependent, but build_classic_dist_vector didn't check
its return value and happily called build_classic_dist_vector_1
on it. As finalize_ddr_dependent doesn't clear DDR_SUBSCRIPTS pointer after
freeing it, this didn't crash right away, but as DDR_NUM_SUBSCRIPTS
contained some random huge number it ICEd shortly after.
While looking into this, I found also that tree-data-ref.c is leaking
memory, DDR_DIST_VECTS and DDR_DIR_VECTS are heap allocated vectors
that weren't ever freed.
The following fix for both PRs comes from my understanding (correct
me if I'm wrong) that if DDR_ARE_DEPENDENT (ddr) != NULL, then
DDR_DIR_VECTS and DDR_DIST_VECTS aren't used (from what I saw in the
users of the tree-data-ref.c computed data it seemed like that).
If subscript_dependence_tester_1 fails, then DDR_ARE_DEPENDENT (ddr) != NULL
and therefore it is IMHO safe to return false right away, no need to
build_classic_dir_vector. build_classic_dist_vector_1 can also
return false because it set DDR_ARE_DEPENDENT to non-NULL
(finalize_ddr_dependent), but also because of
non_affine_dependence_relation. The debug message in that function mentions
that deprel can't be represented by a distance vector, so I'd guess
that we indeed shouldn't add some dist vectors to DDR_DIST_VECTS and
leave others out. If that's incorrect assumption the patch needs to
be changed. When DDR_ARE_DEPENDENT is non-NULL on entry to
build_classic_dist_vector, IMHO we shouldn't populate DDR_DIR_VECTS
for reasons stated above.
The rest of the changes is to avoid the memory leaks. I think
initializing 3 pointers in initialize_data_dependence_relation early
isn't a performance issue and allows more robust freeing of memory
(otherwise finalize_ddr_dependent would also need to free DDR_DIST_VECTS
and DDR_DIR_VECTS and we'd need to make sure that DDR_ARE_DEPENDENT
is never changed directly after initialize_data_dependence_relation
(currently add_other_self_distances and init_omega_for_ddr_1
set it directly) and that we never add anything to the vectors
after DDR_ARE_DEPENDENT is set to non-NULL. Clearing DDR_SUBSCRIPTS
in finalize_ddr_dependence is just to make incorrect code crash right
away rather then referencing uninitialized data.
Questionable things I left out:
1) shouldn't add_other_self_distances and init_omega_for_ddr_1
use finalize_ddr_dependence to set DDR_ARE_DEPENDENT?
2) shouldn't build_classic_dir_vector be a nop if DDR_ARE_DEPENDENT (ddr) !=
NULL? DDR_DIST_VECTS will likely be NULL in that case with this patch
(wouldn't necessarily be the case otherwise), but anyway.
Tested on x86_64-linux, ok for 4.3?
Just quickly skimmed 4.2 and it does have the same issues.
2007-08-21 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/32573
PR middle-end/32946
* tree-data-ref.c (initialize_data_dependence_relation): Clear
DDR_SUBSCRIPTS, DDR_DIR_VECTS and DDR_DIST_VECTS at the beginning.
(finalize_ddr_dependent): Clear DDR_SUBSCRIPTS after freeing it.
(build_classic_dist_vector): Return false rather than true if
DDR_ARE_DEPENDENT is non-NULL at the beginning. Return false
if either subscript_dependence_tester_1 or build_classic_dist_vector_1
returned false. Don't call save_dist_v before calling
build_classic_dist_vector_1.
(free_dependence_relation): Don't guard freeing DDR_SUBSCRIPTS
with NULL DDR_ARE_DEPENDENT. Also free DDR_DIST_VECTS and/or
DDR_DIR_VECTS vectors.
* gcc.dg/pr32573.c: New test.
--- gcc/tree-data-ref.c.jj 2007-08-20 09:49:24.000000000 +0200
+++ gcc/tree-data-ref.c 2007-08-21 15:52:10.000000000 +0200
@@ -1229,6 +1229,9 @@ initialize_data_dependence_relation (str
DDR_B (res) = b;
DDR_LOOP_NEST (res) = NULL;
DDR_REVERSED_P (res) = false;
+ DDR_SUBSCRIPTS (res) = NULL;
+ DDR_DIR_VECTS (res) = NULL;
+ DDR_DIST_VECTS (res) = NULL;
if (a == NULL || b == NULL)
{
@@ -1268,8 +1271,6 @@ initialize_data_dependence_relation (str
DDR_SUBSCRIPTS (res) = VEC_alloc (subscript_p, heap, DR_NUM_DIMENSIONS (a));
DDR_LOOP_NEST (res) = loop_nest;
DDR_INNER_LOOP (res) = 0;
- DDR_DIR_VECTS (res) = NULL;
- DDR_DIST_VECTS (res) = NULL;
for (i = 0; i < DR_NUM_DIMENSIONS (a); i++)
{
@@ -1333,6 +1334,7 @@ finalize_ddr_dependent (struct data_depe
DDR_ARE_DEPENDENT (ddr) = chrec;
free_subscripts (DDR_SUBSCRIPTS (ddr));
+ DDR_SUBSCRIPTS (ddr) = NULL;
}
/* The dependence relation DDR cannot be represented by a distance
@@ -2961,7 +2963,7 @@ build_classic_dist_vector (struct data_d
lambda_vector dist_v;
if (DDR_ARE_DEPENDENT (ddr) != NULL_TREE)
- return true;
+ return false;
if (same_access_functions (ddr))
{
@@ -3013,11 +3015,13 @@ build_classic_dist_vector (struct data_d
if (!lambda_vector_lexico_pos (dist_v, DDR_NB_LOOPS (ddr)))
{
lambda_vector save_v = lambda_vector_new (DDR_NB_LOOPS (ddr));
- subscript_dependence_tester_1 (ddr, DDR_B (ddr), DDR_A (ddr),
- loop_nest);
+ if (!subscript_dependence_tester_1 (ddr, DDR_B (ddr), DDR_A (ddr),
+ loop_nest))
+ return false;
compute_subscript_distance (ddr);
- build_classic_dist_vector_1 (ddr, DDR_B (ddr), DDR_A (ddr),
- save_v, &init_b, &index_carry);
+ if (!build_classic_dist_vector_1 (ddr, DDR_B (ddr), DDR_A (ddr),
+ save_v, &init_b, &index_carry))
+ return false;
save_dist_v (ddr, save_v);
DDR_REVERSED_P (ddr) = true;
@@ -3047,21 +3051,26 @@ build_classic_dist_vector (struct data_d
{
lambda_vector save_v = lambda_vector_new (DDR_NB_LOOPS (ddr));
lambda_vector_copy (dist_v, save_v, DDR_NB_LOOPS (ddr));
- save_dist_v (ddr, save_v);
if (DDR_NB_LOOPS (ddr) > 1)
{
lambda_vector opposite_v = lambda_vector_new (DDR_NB_LOOPS (ddr));
- subscript_dependence_tester_1 (ddr, DDR_B (ddr), DDR_A (ddr),
- loop_nest);
+ if (!subscript_dependence_tester_1 (ddr, DDR_B (ddr),
+ DDR_A (ddr), loop_nest))
+ return false;
compute_subscript_distance (ddr);
- build_classic_dist_vector_1 (ddr, DDR_B (ddr), DDR_A (ddr),
- opposite_v, &init_b, &index_carry);
+ if (!build_classic_dist_vector_1 (ddr, DDR_B (ddr), DDR_A (ddr),
+ opposite_v, &init_b,
+ &index_carry))
+ return false;
+ save_dist_v (ddr, save_v);
add_outer_distances (ddr, dist_v, index_carry);
add_outer_distances (ddr, opposite_v, index_carry);
}
+ else
+ save_dist_v (ddr, save_v);
}
}
else
@@ -4288,8 +4297,12 @@ free_dependence_relation (struct data_de
if (ddr == NULL)
return;
- if (DDR_ARE_DEPENDENT (ddr) == NULL_TREE && DDR_SUBSCRIPTS (ddr))
+ if (DDR_SUBSCRIPTS (ddr))
free_subscripts (DDR_SUBSCRIPTS (ddr));
+ if (DDR_DIST_VECTS (ddr))
+ VEC_free (lambda_vector, heap, DDR_DIST_VECTS (ddr));
+ if (DDR_DIR_VECTS (ddr))
+ VEC_free (lambda_vector, heap, DDR_DIR_VECTS (ddr));
free (ddr);
}
--- gcc/testsuite/gcc.dg/pr32573.c.jj 2007-08-21 16:05:41.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr32573.c 2007-08-21 16:04:11.000000000 +0200
@@ -0,0 +1,30 @@
+/* PR tree-optimization/32573 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+int
+foo (void *x, long long *y)
+{
+ char a[256];
+ int i = 0;
+ long long b;
+ int c;
+ int d = 0;
+ int e = 0;
+ unsigned f = 0;
+ b = bar (x);
+ c = (unsigned) b;
+ while (d < b && d < 65557)
+ {
+ f = *(unsigned *) &a[0];
+ for (i = c - 4; i > 0; i--)
+ if (a[i + 0] == 0x50
+ && a[i + 1] == 0x4B
+ && a[i + 3] == 0x06)
+ {
+ e = 1;
+ break;
+ }
+ }
+ return !e;
+}
Jakub
More information about the Gcc-patches
mailing list