On Tue, 7 May 2013, Jakub Jelinek wrote:
> Hi!
>
> On the attached testcase we started to give false positive warnings
> when critedge has been changed to only use SSA_NAME_VAR for PHI result
> if all PHI arguments use that SSA_NAME_VAR.
>
> The fn3 function (inlined) contains a retval variable which has
> false positive uninit warning disabled through the int retval = retval;
> trick, so we set TREE_NO_WARNING on the VAR_DECL. Previously
> the PHI result was also result_NN and the uninit pass doesn't warn
> for vars that are used possibly uninitialized and that are TREE_NO_WARNING.
> But since the changes, the PHI result is just an anonymous SSA_NAME and
> later on merged with the err var, which isn't TREE_NO_WARNING, the
> only thing that is TREE_NO_WARNING is the result_NN(D) used in one of the
> PHIs.
>
> The following patch fixes it by not considering TREE_NO_WARNING
> SSA_NAME_VARs as undefined for the purposes of uninit pass (other passes
> still IMHO want the previous ssa_undefined_value_p behavior, which is
> why I haven't changed ssa_undefined_value_p directly).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.8?
Ok.
Thanks,
Richard.
> 2013-05-07 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/57149
> * tree-ssa-uninit.c (uninit_undefined_value_p): New inline.
> (can_skip_redundant_opnd, compute_uninit_opnds_pos,
> collect_phi_def_edges, execute_late_warn_uninitialized): Use
> uninit_undefined_value_p instead of ssa_undefined_value_p.
>
> * gcc.dg/pr57149.c: New test.
>
> --- gcc/tree-ssa-uninit.c.jj 2013-04-26 15:14:19.000000000 +0200
> +++ gcc/tree-ssa-uninit.c 2013-05-07 08:57:37.538444516 +0200
> @@ -101,6 +101,19 @@ ssa_undefined_value_p (tree t)
> && pointer_set_contains (possibly_undefined_names, t)));
> }
>
> +/* Like ssa_undefined_value_p, but don't return true if TREE_NO_WARNING
> + is set on SSA_NAME_VAR. */
> +
> +static inline bool
> +uninit_undefined_value_p (tree t)
> +{
> + if (!ssa_undefined_value_p (t))
> + return false;
> + if (SSA_NAME_VAR (t) && TREE_NO_WARNING (SSA_NAME_VAR (t)))
> + return false;
> + return true;
> +}
> +
> /* Checks if the operand OPND of PHI is defined by
> another phi with one operand defined by this PHI,
> but the rest operands are all defined. If yes,
> @@ -124,7 +137,7 @@ can_skip_redundant_opnd (tree opnd, gimp
> tree op = gimple_phi_arg_def (op_def, i);
> if (TREE_CODE (op) != SSA_NAME)
> continue;
> - if (op != phi_def && ssa_undefined_value_p (op))
> + if (op != phi_def && uninit_undefined_value_p (op))
> return false;
> }
>
> @@ -149,7 +162,7 @@ compute_uninit_opnds_pos (gimple phi)
> {
> tree op = gimple_phi_arg_def (phi, i);
> if (TREE_CODE (op) == SSA_NAME
> - && ssa_undefined_value_p (op)
> + && uninit_undefined_value_p (op)
> && !can_skip_redundant_opnd (op, phi))
> {
> /* Ignore SSA_NAMEs on abnormal edges to setjmp
> @@ -518,7 +531,7 @@ collect_phi_def_edges (gimple phi, basic
> gimple_bb (def), cd_root))
> collect_phi_def_edges (def, cd_root, edges,
> visited_phis);
> - else if (!ssa_undefined_value_p (opnd))
> + else if (!uninit_undefined_value_p (opnd))
> {
> if (dump_file && (dump_flags & TDF_DETAILS))
> {
> @@ -2002,7 +2015,7 @@ execute_late_warn_uninitialized (void)
> {
> tree op = gimple_phi_arg_def (phi, i);
> if (TREE_CODE (op) == SSA_NAME
> - && ssa_undefined_value_p (op))
> + && uninit_undefined_value_p (op))
> {
> worklist.safe_push (phi);
> pointer_set_insert (added_to_worklist, phi);
> --- gcc/testsuite/gcc.dg/pr57149.c.jj 2013-05-07 09:00:02.253621530 +0200
> +++ gcc/testsuite/gcc.dg/pr57149.c 2013-05-07 08:59:42.000000000 +0200
> @@ -0,0 +1,50 @@
> +/* PR tree-optimization/57149 */
> +/* { dg-do compile } */
> +/* { dg-options "-Os -Wuninitialized" } */
> +
> +struct A { struct A *a, *b; };
> +struct D { struct A e; };
> +struct E { unsigned char f; struct { struct A e; } g; };
> +struct F { struct E i[32]; };
> +
> +extern int fn0 (void);
> +extern int fn1 (struct E *, struct D *);
> +
> +static inline __attribute__ ((always_inline)) int
> +fn2 (const struct A *x)
> +{
> + return x->a == x;
> +}
> +
> +static int
> +fn3 (struct E *x)
> +{
> + struct D *l, *m;
> + int retval = retval;
> + if (fn2 (&x->g.e))
> + return 0;
> + for (l = (struct D *) x->g.e.a, m = (struct D *) l->e.a;
> + &l->e != &x->g.e;
> + l = m, m = (struct D *) m->e.a)
> + retval = fn1 (x, l);
> + return retval;
> +}
> +
> +void
> +fn4 (struct F *x, unsigned k)
> +{
> + unsigned i;
> + for (i = 0; i < k; i++)
> + {
> + struct E *y = &x->i[i];
> + int err = -22;
> + err = fn3 (y);
> + if (y->f == 0)
> + {
> + if (err > 0)
> + err = fn0 ();
> + if (err < 0) /* { dg-bogus "may be used uninitialized in this
> function" } */
> + fn0 ();
> + }
> + }
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend
