[Bug c/95141] [8/9/10/11 Regression] Incorrect integer overflow warning message for bitand expression

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95141

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #2)
> So there's already (OVF) at
> 
> ((long unsigned int) IA1 & 158(OVF)) & 1
> 
> but we only check
> 
> 375           if (TREE_OVERFLOW_P (ret)
> 376               && !TREE_OVERFLOW_P (op0)
> 377               && !TREE_OVERFLOW_P (op1))
> 378             overflow_warning (EXPR_LOC_OR_LOC (expr, input_location),
> ret, expr);
> 
> which doesn't catch the pre-existing overflow on op0 which then propagates.
> 
> The original overflow is introduced folding short 158 to signed char -98(OVF)
> via convert.c:do_narrow:
> 
> 437           /* We should do away with all this once we have a proper
> 438              type promotion/demotion pass, see PR45397.  */
> 439           expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
> 440                                         convert (typex, arg0),
> 441                                         convert (typex, arg1));
> 
> specifically the convert (typex, arg1).
> 
> Now TREE_OVERFLOW in general is quite a fragile thing, but it's tempting to
> adjust the overflow_warning guard for this case ...
> 
> The do_narrow code also specifically looks for overflow cases that matter
> and does not perform narrowing then so clearing TREE_OVERFLOW there would
> also look reasonable.
> 
> Thus like the following?  Should be cheaper than adding walk_tree to the
> diagnostic guard.
> 
> diff --git a/gcc/convert.c b/gcc/convert.c
> index 42509c518a9..ed00ded1a89 100644
> --- a/gcc/convert.c
> +++ b/gcc/convert.c
> @@ -436,9 +436,16 @@ do_narrow (location_t loc,
>         }
>        /* We should do away with all this once we have a proper
>          type promotion/demotion pass, see PR45397.  */
> +      /* Above we checked for all cases where overflow matters, avoid
> +        geneating overflowed constants here which otherwise propagate
> +        and cause spurious warnings, see PR95141.  */
> +      tree converted_arg1 = convert (typex, arg1);
> +      if (TREE_OVERFLOW_P (converted_arg1)
> +         && !TREE_OVERFLOW_P (arg1))
> +       converted_arg1 = drop_tree_overflow (converted_arg1);
>        expr = maybe_fold_build2_loc (dofold, loc, ex_form, typex,
>                                     convert (typex, arg0),
> -                                   convert (typex, arg1));
> +                                   converted_arg1);
>        return convert (type, expr);
>      }

Regresses

FAIL: gcc.dg/overflow-warn-5.c  (test for warnings, line 6)

which looks like a useful warning to preserve.  Lame "walk-tree" variant
catching this case:

diff --git a/gcc/c/c-fold.c b/gcc/c/c-fold.c
index 63becfeaf2c..bd21d247051 100644
--- a/gcc/c/c-fold.c
+++ b/gcc/c/c-fold.c
@@ -374,6 +374,7 @@ c_fully_fold_internal (tree expr, bool in_init, bool
*maybe_const_operands,
        ret = fold (expr);
       if (TREE_OVERFLOW_P (ret)
          && !TREE_OVERFLOW_P (op0)
+         && !(BINARY_CLASS_P (op0) && TREE_OVERFLOW_P (TREE_OPERAND (op0, 1)))
          && !TREE_OVERFLOW_P (op1))
        overflow_warning (EXPR_LOC_OR_LOC (expr, input_location), ret, expr);
       if (code == LSHIFT_EXPR