[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564
--- Comment #16 from Richard Biener ---
(In reply to Andrew Macleod from comment #15)
> (In reply to Richard Biener from comment #13)
> > (In reply to Jeffrey A. Law from comment #12)
> > > So I think we could solve this with a bit of help from the alias oracle.
> > > We
> > > have the routine ptrs_compare_unequal, but points-to-null is going to get
> > > in the way.
> > >
> > > I think VRP and DOM have enough information to rule out NULL for both
> > > objects in question. So if we could query the points-to information,
> > > ignoring NULL then we could likely solve this particular bug.
> > >
> > > Essentially VRP or DOM would prove NULL isn't in the set of possible
> > > values
> > > at the comparison point. Then we query the alias information ignoring
> > > NULL.
> > > Voila we compute a static result for the comparison of the two pointers
> > > and
> > > the problematical block becomes unreachable and the bogus warning goes
> > > away.
> > >
> > > Richi, any thoughts in viability of such an API?
> >
> > We now treat pt.null conservatively and track non-null-ness derived from
> > range-info in it. That means when VRP/DOM can prove a pointer is always
> > not NULL they can do set_ptr_nonnull (p) on it.
> >
> > This means the
> >
> > /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
> > but those require pt.null to be conservatively correct. */
> >
> > is no longer true and we could finally implement it, like with
> >
> > diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
> > index e7c1c1aa624..5b6d9e0aa6a 100644
> > --- a/gcc/tree-ssa-alias.cc
> > +++ b/gcc/tree-ssa-alias.cc
> > @@ -479,9 +479,25 @@ ptrs_compare_unequal (tree ptr1, tree ptr2)
> > }
> >return !pt_solution_includes (>pt, obj1);
> > }
> > -
> > - /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
> > - but those require pt.null to be conservatively correct. */
> > + else if (TREE_CODE (ptr1) == SSA_NAME)
> > +{
> > + struct ptr_info_def *pi1 = SSA_NAME_PTR_INFO (ptr1);
> > + if (!pi1
> > + || pi1->pt.vars_contains_restrict
> > + || pi1->pt.vars_contains_interposable)
> > + return false;
> > + if (integer_zerop (ptr2) && !pi1->pt.null)
> > + return true;
> > + if (TREE_CODE (ptr2) == SSA_NAME)
> > + {
> > + struct ptr_info_def *pi2 = SSA_NAME_PTR_INFO (ptr2);
> > + if (!pi2
> > + || pi2->pt.vars_contains_restrict
> > + || pi2->pt.vars_contains_interposable)
> > + if (!pi1->pt.null || !pi2->pt.null)
> > + return !pt_solutions_intersect (>pt, >pt);
> > + }
> > +}
> >
> >return false;
> > }
> >
> >
> > but the testcase shows the non-null-ness is only conditional which means
> > we'd have to use a range query above which necessarily falls back to
> > the global ranges given we don't have any context available here. The
> > old EVRP adjusted global ranges during the walk but this is no longer done.
> >
> You mean it lied? because x_1 is not NULL until after _8 = *x_1(D);
> executes. It can still be NULL on that stmt can it not? Did it reset the
> global value afterwards?
Yes and yes, old EVRP turned global ranges into "ranges at the point of
stmt evaluation/folding" (and restored them to the global values later).
Note that EVRP didn't do any sort of iteration for loop handling and it
folded stmts as it analyzed them. Using the global ranges as "lattice"
had the advantage that all folding utilities picked up "local" ranges
for free. IMO it was quite elegant and fast what EVRP did (with it's
obvious limitations of course).
> Contextually ranger knows both are non-null at EVRP time:
> a.0_27 : [irange] int[0:D.] * [1, +INF]
> 2->3 (T) x_1(D) : [irange] int * [1, +INF]
> 2->3 (T) a.0_27 : [irange] int[0:D.] * [1, +INF]
> 2->4 (F) x_1(D) : [irange] int * [1, +INF]
> 2->4 (F) a.0_27 : [irange] int[0:D.] * [1, +INF]
>
> So we know x_1 is non-NULL after the de-reference for the rest of the block
> (and function). It also sets a.0_27 globally to be [1, +INF].
>
> > Note it's enough that one pointer is nonnull, so for your idea the
> > API could be extended with a bool one_ptr_nonnull parameter.
>
> ranger currently sets a.0 globally to be non-null in EVRP.
After EVRP I see
# PT = nonlocal null
unsigned int * x_8(D) = x;
...
:
_1 = *x_8(D);
# RANGE [irange] long unsigned int [0, 4294967295] MASK 0x VALUE 0x0
_2 = (long unsigned int) _1;
# PT = null { D.2781 }
# ALIGN = 8, MISALIGN = 0
# USE = nonlocal escaped
# CLB = nonlocal escaped
a_10 = malloc (_2);
if (a_10 == 0B)
...
:
if (x_8(D) != a_10)
the last test is the one we want to eliminate. The proposed change (with
a missing 'return false' fixed) isn't enough since it just looks at
global ranges where both x_8 and a_10 can be null. In the
[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564
--- Comment #15 from Andrew Macleod ---
(In reply to Richard Biener from comment #13)
> (In reply to Jeffrey A. Law from comment #12)
> > So I think we could solve this with a bit of help from the alias oracle. We
> > have the routine ptrs_compare_unequal, but points-to-null is going to get
> > in the way.
> >
> > I think VRP and DOM have enough information to rule out NULL for both
> > objects in question. So if we could query the points-to information,
> > ignoring NULL then we could likely solve this particular bug.
> >
> > Essentially VRP or DOM would prove NULL isn't in the set of possible values
> > at the comparison point. Then we query the alias information ignoring NULL.
> > Voila we compute a static result for the comparison of the two pointers and
> > the problematical block becomes unreachable and the bogus warning goes away.
> >
> > Richi, any thoughts in viability of such an API?
>
> We now treat pt.null conservatively and track non-null-ness derived from
> range-info in it. That means when VRP/DOM can prove a pointer is always
> not NULL they can do set_ptr_nonnull (p) on it.
>
> This means the
>
> /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
> but those require pt.null to be conservatively correct. */
>
> is no longer true and we could finally implement it, like with
>
> diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
> index e7c1c1aa624..5b6d9e0aa6a 100644
> --- a/gcc/tree-ssa-alias.cc
> +++ b/gcc/tree-ssa-alias.cc
> @@ -479,9 +479,25 @@ ptrs_compare_unequal (tree ptr1, tree ptr2)
> }
>return !pt_solution_includes (>pt, obj1);
> }
> -
> - /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
> - but those require pt.null to be conservatively correct. */
> + else if (TREE_CODE (ptr1) == SSA_NAME)
> +{
> + struct ptr_info_def *pi1 = SSA_NAME_PTR_INFO (ptr1);
> + if (!pi1
> + || pi1->pt.vars_contains_restrict
> + || pi1->pt.vars_contains_interposable)
> + return false;
> + if (integer_zerop (ptr2) && !pi1->pt.null)
> + return true;
> + if (TREE_CODE (ptr2) == SSA_NAME)
> + {
> + struct ptr_info_def *pi2 = SSA_NAME_PTR_INFO (ptr2);
> + if (!pi2
> + || pi2->pt.vars_contains_restrict
> + || pi2->pt.vars_contains_interposable)
> + if (!pi1->pt.null || !pi2->pt.null)
> + return !pt_solutions_intersect (>pt, >pt);
> + }
> +}
>
>return false;
> }
>
>
> but the testcase shows the non-null-ness is only conditional which means
> we'd have to use a range query above which necessarily falls back to
> the global ranges given we don't have any context available here. The
> old EVRP adjusted global ranges during the walk but this is no longer done.
>
You mean it lied? because x_1 is not NULL until after _8 = *x_1(D); executes.
It can still be NULL on that stmt can it not? Did it reset the global value
afterwards?
Contextually ranger knows both are non-null at EVRP time:
a.0_27 : [irange] int[0:D.] * [1, +INF]
2->3 (T) x_1(D) : [irange] int * [1, +INF]
2->3 (T) a.0_27 : [irange] int[0:D.] * [1, +INF]
2->4 (F) x_1(D) : [irange] int * [1, +INF]
2->4 (F) a.0_27 : [irange] int[0:D.] * [1, +INF]
So we know x_1 is non-NULL after the de-reference for the rest of the block
(and function). It also sets a.0_27 globally to be [1, +INF].
> Note it's enough that one pointer is nonnull, so for your idea the
> API could be extended with a bool one_ptr_nonnull parameter.
ranger currently sets a.0 globally to be non-null in EVRP.
[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564 --- Comment #14 from Richard Biener --- gcc.c-torture/execute/pr64242.c is one of the testcases FAILing (guess it's invalid)
[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564
--- Comment #13 from Richard Biener ---
(In reply to Jeffrey A. Law from comment #12)
> So I think we could solve this with a bit of help from the alias oracle. We
> have the routine ptrs_compare_unequal, but points-to-null is going to get
> in the way.
>
> I think VRP and DOM have enough information to rule out NULL for both
> objects in question. So if we could query the points-to information,
> ignoring NULL then we could likely solve this particular bug.
>
> Essentially VRP or DOM would prove NULL isn't in the set of possible values
> at the comparison point. Then we query the alias information ignoring NULL.
> Voila we compute a static result for the comparison of the two pointers and
> the problematical block becomes unreachable and the bogus warning goes away.
>
> Richi, any thoughts in viability of such an API?
We now treat pt.null conservatively and track non-null-ness derived from
range-info in it. That means when VRP/DOM can prove a pointer is always
not NULL they can do set_ptr_nonnull (p) on it.
This means the
/* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
but those require pt.null to be conservatively correct. */
is no longer true and we could finally implement it, like with
diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
index e7c1c1aa624..5b6d9e0aa6a 100644
--- a/gcc/tree-ssa-alias.cc
+++ b/gcc/tree-ssa-alias.cc
@@ -479,9 +479,25 @@ ptrs_compare_unequal (tree ptr1, tree ptr2)
}
return !pt_solution_includes (>pt, obj1);
}
-
- /* ??? We'd like to handle ptr1 != NULL and ptr1 != ptr2
- but those require pt.null to be conservatively correct. */
+ else if (TREE_CODE (ptr1) == SSA_NAME)
+{
+ struct ptr_info_def *pi1 = SSA_NAME_PTR_INFO (ptr1);
+ if (!pi1
+ || pi1->pt.vars_contains_restrict
+ || pi1->pt.vars_contains_interposable)
+ return false;
+ if (integer_zerop (ptr2) && !pi1->pt.null)
+ return true;
+ if (TREE_CODE (ptr2) == SSA_NAME)
+ {
+ struct ptr_info_def *pi2 = SSA_NAME_PTR_INFO (ptr2);
+ if (!pi2
+ || pi2->pt.vars_contains_restrict
+ || pi2->pt.vars_contains_interposable)
+ if (!pi1->pt.null || !pi2->pt.null)
+ return !pt_solutions_intersect (>pt, >pt);
+ }
+}
return false;
}
but the testcase shows the non-null-ness is only conditional which means
we'd have to use a range query above which necessarily falls back to
the global ranges given we don't have any context available here. The
old EVRP adjusted global ranges during the walk but this is no longer done.
Note it's enough that one pointer is nonnull, so for your idea the
API could be extended with a bool one_ptr_nonnull parameter.
[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564 Jeffrey A. Law changed: What|Removed |Added CC||law at gcc dot gnu.org, ||rguenth at gcc dot gnu.org --- Comment #12 from Jeffrey A. Law --- So I think we could solve this with a bit of help from the alias oracle. We have the routine ptrs_compare_unequal, but points-to-null is going to get in the way. I think VRP and DOM have enough information to rule out NULL for both objects in question. So if we could query the points-to information, ignoring NULL then we could likely solve this particular bug. Essentially VRP or DOM would prove NULL isn't in the set of possible values at the comparison point. Then we query the alias information ignoring NULL. Voila we compute a static result for the comparison of the two pointers and the problematical block becomes unreachable and the bogus warning goes away. Richi, any thoughts in viability of such an API?
[Bug middle-end/96564] [11/12/13/14 Regression] New maybe use of uninitialized variable warning since r11-959
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96564 Jakub Jelinek changed: What|Removed |Added Target Milestone|11.4|11.5 --- Comment #11 from Jakub Jelinek --- GCC 11.4 is being released, retargeting bugs to GCC 11.5.
