On Thu, 7 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> While looking at why we didn't warn before Richi's commit, I've discovered
> is that the change is that in a different pass we used to call c_strlen too,
> but at that point with a location from system headers, so no warning has
> been printed, but TREE_NO_WARNING has been set anyway and later on when
> c_strlen was called again with a location not from system header, we
> wouldn't warn because TREE_NO_WARNING is set.
> 
> Apparently there are quite a few spots that have similar bug (and quite a
> few spots that do it right).
> 
> The following patch fixes what I found.  Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2019-03-07  Jakub Jelinek  <ja...@redhat.com>
> 
>       PR tree-optimization/89550
>       * builtins.c (c_strlen): Only set TREE_NO_WARNING if warning_at
>       returned true.  Formatting fixes.
>       (expand_builtin_strnlen): Formatting fixes.
>       * tree-vrp.c (vrp_prop::check_mem_ref): Only set TREE_NO_WARNING
>       if warning_at returned true.
>       * tree-cfg.c (pass_warn_function_return::execute): Likewise.
> c-family/
>       * c-common.c (c_common_truthvalue_conversion): Only set
>       TREE_NO_WARNING if warning_at returned true.
>       * c-warn.c (overflow_warning, warn_logical_operator): Likewise.
> c/
>       * c-decl.c (finish_function): Only set TREE_NO_WARNING if warning_at
>       returned true.
>       (c_write_global_declarations_1): Only set TREE_NO_WARNING if pedwarn
>       or warning returned true.
> cp/
>       * semantics.c (maybe_convert_cond): Only set TREE_NO_WARNING if
>       warning_at returned true.
>       * decl2.c (c_parse_final_cleanups): Likewise.
>       * typeck.c (convert_for_assignment): Likewise.
>       * decl.c (finish_function): Likewise.
> 
> --- gcc/builtins.c.jj 2019-03-05 17:21:42.536993265 +0100
> +++ gcc/builtins.c    2019-03-07 12:01:23.114479128 +0100
> @@ -760,15 +760,13 @@ c_strlen (tree src, int only_value, c_st
>       runtime.  */
>    if (eltoff < 0 || eltoff >= maxelts)
>      {
> -     /* Suppress multiple warnings for propagated constant strings.  */
> +      /* Suppress multiple warnings for propagated constant strings.  */
>        if (only_value != 2
> -       && !TREE_NO_WARNING (src))
> -        {
> -       warning_at (loc, OPT_Warray_bounds,
> -                   "offset %qwi outside bounds of constant string",
> -                   eltoff);
> -          TREE_NO_WARNING (src) = 1;
> -        }
> +       && !TREE_NO_WARNING (src)
> +       && warning_at (loc, OPT_Warray_bounds,
> +                      "offset %qwi outside bounds of constant string",
> +                      eltoff))
> +     TREE_NO_WARNING (src) = 1;
>        return NULL_TREE;
>      }
>  
> @@ -3099,7 +3097,7 @@ expand_builtin_strnlen (tree exp, rtx ta
>                        "%K%qD specified bound %E "
>                        "exceeds maximum object size %E",
>                        exp, func, bound, maxobjsize))
> -       TREE_NO_WARNING (exp) = true;
> +     TREE_NO_WARNING (exp) = true;
>  
>        bool exact = true;
>        if (!len || TREE_CODE (len) != INTEGER_CST)
> @@ -3158,7 +3156,7 @@ expand_builtin_strnlen (tree exp, rtx ta
>                    "%K%qD specified bound [%wu, %wu] "
>                    "exceeds maximum object size %E",
>                    exp, func, min.to_uhwi (), max.to_uhwi (), maxobjsize))
> -      TREE_NO_WARNING (exp) = true;
> +    TREE_NO_WARNING (exp) = true;
>  
>    bool exact = true;
>    if (!len || TREE_CODE (len) != INTEGER_CST)
> --- gcc/tree-vrp.c.jj 2019-02-01 09:43:40.264757408 +0100
> +++ gcc/tree-vrp.c    2019-03-07 12:05:05.959845856 +0100
> @@ -4749,7 +4749,8 @@ vrp_prop::check_mem_ref (location_t loca
>        if (warned && DECL_P (arg))
>       inform (DECL_SOURCE_LOCATION (arg), "while referencing %qD", arg);
>  
> -      TREE_NO_WARNING (ref) = 1;
> +      if (warned)
> +     TREE_NO_WARNING (ref) = 1;
>        return;
>      }
>  
> @@ -4762,11 +4763,10 @@ vrp_prop::check_mem_ref (location_t loca
>      {
>        HOST_WIDE_INT tmpidx = extrema[i].to_shwi () / eltsize.to_shwi ();
>  
> -      warning_at (location, OPT_Warray_bounds,
> -               "intermediate array offset %wi is outside array bounds "
> -               "of %qT",
> -               tmpidx,  reftype);
> -      TREE_NO_WARNING (ref) = 1;
> +      if (warning_at (location, OPT_Warray_bounds,
> +                   "intermediate array offset %wi is outside array bounds "
> +                   "of %qT", tmpidx, reftype))
> +     TREE_NO_WARNING (ref) = 1;
>      }
>  }
>  
> --- gcc/tree-cfg.c.jj 2019-02-22 23:02:47.768118220 +0100
> +++ gcc/tree-cfg.c    2019-03-07 12:02:52.662019138 +0100
> @@ -9328,9 +9328,9 @@ pass_warn_function_return::execute (func
>             location = gimple_location (last);
>             if (LOCATION_LOCUS (location) == UNKNOWN_LOCATION)
>               location = fun->function_end_locus;
> -           warning_at (location, OPT_Wreturn_type,
> -                       "control reaches end of non-void function");
> -           TREE_NO_WARNING (fun->decl) = 1;
> +           if (warning_at (location, OPT_Wreturn_type,
> +                           "control reaches end of non-void function"))
> +             TREE_NO_WARNING (fun->decl) = 1;
>             break;
>           }
>       }
> @@ -9360,9 +9360,9 @@ pass_warn_function_return::execute (func
>                   location = gimple_location (prev);
>                 if (LOCATION_LOCUS (location) == UNKNOWN_LOCATION)
>                   location = fun->function_end_locus;
> -               warning_at (location, OPT_Wreturn_type,
> -                           "control reaches end of non-void function");
> -               TREE_NO_WARNING (fun->decl) = 1;
> +               if (warning_at (location, OPT_Wreturn_type,
> +                               "control reaches end of non-void function"))
> +                 TREE_NO_WARNING (fun->decl) = 1;
>                 break;
>               }
>           }
> --- gcc/c-family/c-common.c.jj        2019-02-18 20:48:34.420703158 +0100
> +++ gcc/c-family/c-common.c   2019-03-07 12:09:45.187293329 +0100
> @@ -3546,13 +3546,11 @@ c_common_truthvalue_conversion (location
>  
>      case MODIFY_EXPR:
>        if (!TREE_NO_WARNING (expr)
> -       && warn_parentheses)
> -     {
> -       warning_at (location, OPT_Wparentheses,
> -                   "suggest parentheses around assignment used as "
> -                   "truth value");
> -       TREE_NO_WARNING (expr) = 1;
> -     }
> +       && warn_parentheses
> +       && warning_at (location, OPT_Wparentheses,
> +                      "suggest parentheses around assignment used as "
> +                      "truth value"))
> +     TREE_NO_WARNING (expr) = 1;
>        break;
>  
>      case CONST_DECL:
> --- gcc/c-family/c-warn.c.jj  2019-02-11 18:04:18.143588036 +0100
> +++ gcc/c-family/c-warn.c     2019-03-07 12:11:44.097354618 +0100
> @@ -143,12 +143,16 @@ overflow_warning (location_t loc, tree v
>        return;
>      }
>  
> +  bool warned;
>    if (expr)
> -    warning_at (loc, OPT_Woverflow, warnfmt, expr, TREE_TYPE (expr), value);
> +    warned = warning_at (loc, OPT_Woverflow, warnfmt, expr, TREE_TYPE (expr),
> +                      value);
>    else
> -    warning_at (loc, OPT_Woverflow, warnfmt, TREE_TYPE (value), value);
> +    warned = warning_at (loc, OPT_Woverflow, warnfmt, TREE_TYPE (value),
> +                      value);
>  
> -  TREE_NO_WARNING (value) = 1;
> +  if (warned)
> +    TREE_NO_WARNING (value) = 1;
>  }
>  
>  /* Helper function for walk_tree.  Unwrap C_MAYBE_CONST_EXPRs in an 
> expression
> @@ -216,13 +220,17 @@ warn_logical_operator (location_t locati
>         && !integer_zerop (folded_op_right)
>         && !integer_onep (folded_op_right))
>       {
> +       bool warned;
>         if (or_op)
> -         warning_at (location, OPT_Wlogical_op, "logical %<or%>"
> -                     " applied to non-boolean constant");
> +         warned
> +           = warning_at (location, OPT_Wlogical_op,
> +                         "logical %<or%> applied to non-boolean constant");
>         else
> -         warning_at (location, OPT_Wlogical_op, "logical %<and%>"
> -                     " applied to non-boolean constant");
> -       TREE_NO_WARNING (op_left) = true;
> +         warned
> +           = warning_at (location, OPT_Wlogical_op,
> +                         "logical %<and%> applied to non-boolean constant");
> +       if (warned)
> +         TREE_NO_WARNING (op_left) = true;
>         return;
>       }
>      }
> --- gcc/c/c-decl.c.jj 2019-02-15 08:37:06.893222441 +0100
> +++ gcc/c/c-decl.c    2019-03-07 12:08:41.932324639 +0100
> @@ -9664,12 +9664,10 @@ finish_function (void)
>        && !C_FUNCTION_IMPLICIT_INT (fndecl)
>        /* Normally, with -Wreturn-type, flow will complain, but we might
>           optimize out static functions.  */
> -      && !TREE_PUBLIC (fndecl))
> -    {
> -      warning (OPT_Wreturn_type,
> -            "no return statement in function returning non-void");
> -      TREE_NO_WARNING (fndecl) = 1;
> -    }
> +      && !TREE_PUBLIC (fndecl)
> +      && warning (OPT_Wreturn_type,
> +               "no return statement in function returning non-void"))
> +    TREE_NO_WARNING (fndecl) = 1;
>  
>    /* Complain about parameters that are only set, but never otherwise used.  
> */
>    if (warn_unused_but_set_parameter)
> @@ -11486,17 +11484,19 @@ c_write_global_declarations_1 (tree glob
>       {
>         if (C_DECL_USED (decl))
>           {
> -           pedwarn (input_location, 0, "%q+F used but never defined", decl);
> -           TREE_NO_WARNING (decl) = 1;
> +           if (pedwarn (input_location, 0, "%q+F used but never defined",
> +                        decl))
> +             TREE_NO_WARNING (decl) = 1;
>           }
>         /* For -Wunused-function warn about unused static prototypes.  */
>         else if (warn_unused_function
>                  && ! DECL_ARTIFICIAL (decl)
>                  && ! TREE_NO_WARNING (decl))
>           {
> -           warning (OPT_Wunused_function,
> -                    "%q+F declared %<static%> but never defined", decl);
> -           TREE_NO_WARNING (decl) = 1;
> +           if (warning (OPT_Wunused_function,
> +                        "%q+F declared %<static%> but never defined",
> +                        decl))
> +             TREE_NO_WARNING (decl) = 1;
>           }
>       }
>  
> --- gcc/cp/semantics.c.jj     2019-03-06 19:45:40.367751642 +0100
> +++ gcc/cp/semantics.c        2019-03-07 12:14:50.144321301 +0100
> @@ -657,12 +657,11 @@ maybe_convert_cond (tree cond)
>  
>    if (TREE_CODE (cond) == MODIFY_EXPR
>        && !TREE_NO_WARNING (cond)
> -      && warn_parentheses)
> -    {
> -      warning_at (cp_expr_loc_or_loc (cond, input_location), 
> OPT_Wparentheses,
> -               "suggest parentheses around assignment used as truth value");
> -      TREE_NO_WARNING (cond) = 1;
> -    }
> +      && warn_parentheses
> +      && warning_at (cp_expr_loc_or_loc (cond, input_location),
> +                  OPT_Wparentheses, "suggest parentheses around "
> +                                    "assignment used as truth value"))
> +    TREE_NO_WARNING (cond) = 1;
>  
>    return condition_conversion (cond);
>  }
> --- gcc/cp/decl2.c.jj 2019-02-21 13:04:01.833547350 +0100
> +++ gcc/cp/decl2.c    2019-03-07 12:12:41.467419247 +0100
> @@ -5022,13 +5022,11 @@ c_parse_final_cleanups (void)
>         /* Don't complain if the template was defined.  */
>         && !(DECL_TEMPLATE_INSTANTIATION (decl)
>              && DECL_INITIAL (DECL_TEMPLATE_RESULT
> -                             (template_for_substitution (decl)))))
> -     {
> -       warning_at (DECL_SOURCE_LOCATION (decl), 0,
> -                   "inline function %qD used but never defined", decl);
> -       /* Avoid a duplicate warning from check_global_declaration.  */
> -       TREE_NO_WARNING (decl) = 1;
> -     }
> +                             (template_for_substitution (decl))))
> +       && warning_at (DECL_SOURCE_LOCATION (decl), 0,
> +                      "inline function %qD used but never defined", decl))
> +     /* Avoid a duplicate warning from check_global_declaration.  */
> +     TREE_NO_WARNING (decl) = 1;
>      }
>  
>    /* So must decls that use a type with no linkage.  */
> --- gcc/cp/typeck.c.jj        2019-03-06 19:45:40.330752248 +0100
> +++ gcc/cp/typeck.c   2019-03-07 12:16:02.319144554 +0100
> @@ -9063,12 +9063,11 @@ convert_for_assignment (tree type, tree
>        && TREE_CODE (rhs) == MODIFY_EXPR
>        && !TREE_NO_WARNING (rhs)
>        && TREE_CODE (TREE_TYPE (rhs)) != BOOLEAN_TYPE
> -      && (complain & tf_warning))
> -    {
> -      warning_at (rhs_loc, OPT_Wparentheses,
> -               "suggest parentheses around assignment used as truth value");
> -      TREE_NO_WARNING (rhs) = 1;
> -    }
> +      && (complain & tf_warning)
> +      && warning_at (rhs_loc, OPT_Wparentheses,
> +                  "suggest parentheses around assignment used as "
> +                  "truth value"))
> +    TREE_NO_WARNING (rhs) = 1;
>  
>    if (complain & tf_warning)
>      warn_for_address_or_pointer_of_packed_member (type, rhs);
> --- gcc/cp/decl.c.jj  2019-03-02 09:05:44.412524821 +0100
> +++ gcc/cp/decl.c     2019-03-07 12:13:36.500521990 +0100
> @@ -16169,9 +16169,9 @@ finish_function (bool inline_p)
>                                           global_dc->option_state))
>           add_return_star_this_fixit (&richloc, fndecl);
>       }
> -      warning_at (&richloc, OPT_Wreturn_type,
> -               "no return statement in function returning non-void");
> -      TREE_NO_WARNING (fndecl) = 1;
> +      if (warning_at (&richloc, OPT_Wreturn_type,
> +       "no return statement in function returning non-void"))
> +     TREE_NO_WARNING (fndecl) = 1;
>      }
>  
>    /* Store the end of the function, so that we get good line number
> 
>       Jakub
> 
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Reply via email to