Re: [PATCH][V2][middle-end/102276]Don't emit switch-unreachable warnings for -ftrivial-auto-var-init (PR102276)

[Richard Biener <rguenther@suse.de>]

On Tue, 1 Mar 2022, Qing Zhao wrote:

> Hi,
> 
> This is the 2nd version of the patch based on the discussion on the initial version. 
> 
> Compared to the previous version, the major changes in this version are:
> 
> 1.  Merged two patches into one;
> 2.  Re-organize the code to emit both Wswitch-unreachable and Wtrivial-auto-var-init
>      Messages inside existing “warn_switch_unreachable_r”;
> 3.  Avoid emitting Wswitch-unreachable for the following 3 cases:
> 
> 1) call to .DEFERRED_INIT
> 2) call to __builtin_clear_padding if the 2nd argument is present and non-zero
> 3) a gimple assign store right after the .DEFERRED_INIT call that has the LHS
>    as RHS
> 
> 
> Bootstraped and regression tested on both x86 and aarch64.
> 
> Okay for committing?

OK.

Thanks,
Richard.

> thanks.
> 
> Qing.
> 
> ========================
> From ca0193f48c8ae11719aa932db065bb24d4d649d7 Mon Sep 17 00:00:00 2001
> From: Qing Zhao <qing.zhao@oracle.com>
> Date: Mon, 28 Feb 2022 21:45:55 +0000
> Subject: [PATCH] Don't emit switch-unreachable warnings for
>  -ftrivial-auto-var-init (PR102276) At the same time, adding
>  -Wtrivial-auto-var-init and update documentation.
> 
> for the following testing case:
> 1 int g(int *);
> 2 int f1()
> 3 {
> 4     switch (0) {
> 5         int x;
> 6         default:
> 7         return g(&x);
> 8     }
> 9 }
> compiling with -O -ftrivial-auto-var-init causes spurious warning:
> warning: statement will never be executed [-Wswitch-unreachable]
> 5 |         int x;
>   |             ^
> This is due to the compiler-generated initialization at the point of
> the declaration.
> 
> We could avoid the warning  to exclude the following cases:
> 
> when
> flag_auto_var_init > AUTO_INIT_UNINITIALIZED
> And
> 1) call to .DEFERRED_INIT
> 2) call to __builtin_clear_padding if the 2nd argument is present and non-zero
> 3) a gimple assign store right after the .DEFERRED_INIT call that has the LHS
>    as RHS
> 
> However, we still need to warn users about the incapability of the option
> -ftrivial-auto-var-init by adding a new warning option -Wtrivial-auto-var-init
> to report cases when it cannot initialize the auto variable. At the same
> time, update documentation for -ftrivial-auto-var-init to connect it with
> the new warning option -Wtrivial-auto-var-init,  and add documentation
> for -Wtrivial-auto-var-init.
> 
> gcc/ChangeLog:
> 
>         * common.opt (-Wtrivial-auto-var-init): New option.
>         * doc/invoke.texi (-Wtrivial-auto-var-init): Document new option.
>         (-ftrivial-auto-var-init): Update option;
>         * gimplify.cc (emit_warn_switch_unreachable): New function.
>         (warn_switch_unreachable_r): Rename to ...
>         (warn_switch_unreachable_and_auto_init_r): This.
>         (maybe_warn_switch_unreachable): Rename to ...
>         (maybe_warn_switch_unreachable_and_auto_init): This.
>         (gimplify_switch_expr): Update calls to renamed function.
> 
> gcc/testsuite/ChangeLog:
> 
>         * gcc.dg/auto-init-pr102276-1.c: New test.
>         * gcc.dg/auto-init-pr102276-2.c: New test.
>         * gcc.dg/auto-init-pr102276-3.c: New test.
>         * gcc.dg/auto-init-pr102276-4.c: New test.
> ---
>  gcc/common.opt                              |   4 +
>  gcc/doc/invoke.texi                         |  14 ++-
>  gcc/gimplify.cc                             | 111 +++++++++++++++-----
>  gcc/testsuite/gcc.dg/auto-init-pr102276-1.c |  38 +++++++
>  gcc/testsuite/gcc.dg/auto-init-pr102276-2.c |  38 +++++++
>  gcc/testsuite/gcc.dg/auto-init-pr102276-3.c |  40 +++++++
>  gcc/testsuite/gcc.dg/auto-init-pr102276-4.c |  40 +++++++
>  7 files changed, 259 insertions(+), 26 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr102276-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr102276-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr102276-3.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr102276-4.c
> 
> diff --git a/gcc/common.opt b/gcc/common.opt
> index c21e5273ae3..8b6513de47c 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -801,6 +801,10 @@ Wtrampolines
>  Common Var(warn_trampolines) Warning
>  Warn whenever a trampoline is generated.
>  
> +Wtrivial-auto-var-init
> +Common Var(warn_trivial_auto_var_init) Warning Init(0)
> +Warn about cases where -ftrivial-auto-var-init cannot initialize an auto variable.
> +
>  Wtype-limits
>  Common Var(warn_type_limits) Warning EnabledBy(Wextra)
>  Warn if a comparison is always true or always false due to the limited range of the data type.
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ec291c06542..63a3c5b7c7e 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -399,7 +399,7 @@ Objective-C and Objective-C++ Dialects}.
>  -Wswitch  -Wno-switch-bool  -Wswitch-default  -Wswitch-enum @gol
>  -Wno-switch-outside-range  -Wno-switch-unreachable  -Wsync-nand @gol
>  -Wsystem-headers  -Wtautological-compare  -Wtrampolines  -Wtrigraphs @gol
> --Wtsan -Wtype-limits  -Wundef @gol
> +-Wtrivial-auto-var-init -Wtsan -Wtype-limits  -Wundef @gol
>  -Wuninitialized  -Wunknown-pragmas @gol
>  -Wunsuffixed-float-constants  -Wunused @gol
>  -Wunused-but-set-parameter  -Wunused-but-set-variable @gol
> @@ -6953,6 +6953,14 @@ This warning is enabled by default for C and C++ programs.
>  Warn when @code{__sync_fetch_and_nand} and @code{__sync_nand_and_fetch}
>  built-in functions are used.  These functions changed semantics in GCC 4.4.
>  
> +@item -Wtrivial-auto-var-init
> +@opindex Wtrivial-auto-var-init
> +@opindex Wno-trivial-auto-var-init
> +Warn when @code{-ftrivial-auto-var-init} cannot initialize the automatic
> +variable.  A common situation is an automatic variable that is declared
> +between the controlling expression and the first case label of a @code{switch}
> +statement.
> +
>  @item -Wunused-but-set-parameter
>  @opindex Wunused-but-set-parameter
>  @opindex Wno-unused-but-set-parameter
> @@ -12314,6 +12322,10 @@ initializer as uninitialized, @option{-Wuninitialized} and
>  warning messages on such automatic variables.
>  With this option, GCC will also initialize any padding of automatic variables
>  that have structure or union types to zeroes.
> +However, the current implementation cannot initialize automatic variables that
> +are declared between the controlling expression and the first case of a
> +@code{switch} statement.  Using @option{-Wtrivial-auto-var-init} to report all
> +such cases.
>  
>  The three values of @var{choice} are:
>  
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index f570daa015a..2364d2e5182 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -2029,13 +2029,55 @@ gimplify_statement_list (tree *expr_p, gimple_seq *pre_p)
>    return GS_ALL_DONE;
>  }
>  
> +
> +/* Emit warning for the unreachable statment STMT if needed.
> +   Return the gimple itself when the warning is emitted, otherwise
> +   return NULL.  */
> +static gimple *
> +emit_warn_switch_unreachable (gimple *stmt)
> +{
> +  if (gimple_code (stmt) == GIMPLE_GOTO
> +      && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
> +      && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
> +  /* Don't warn for compiler-generated gotos.  These occur
> +     in Duff's devices, for example.  */
> +    return NULL;
> +  else if ((flag_auto_var_init > AUTO_INIT_UNINITIALIZED)
> +	   && ((gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> +		|| (gimple_call_builtin_p (stmt, BUILT_IN_CLEAR_PADDING)
> +		    && (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1)))
> +		|| (is_gimple_assign (stmt)
> +		    && gimple_assign_single_p (stmt)
> +		    && (TREE_CODE (gimple_assign_rhs1 (stmt)) == SSA_NAME)
> +		    && gimple_call_internal_p (
> +			 SSA_NAME_DEF_STMT (gimple_assign_rhs1 (stmt)),
> +			 IFN_DEFERRED_INIT))))
> +  /* Don't warn for compiler-generated initializations for
> +     -ftrivial-auto-var-init.
> +     There are 3 cases:
> +	case 1: a call to .DEFERRED_INIT;
> +	case 2: a call to __builtin_clear_padding with the 2nd argument is
> +		present and non-zero;
> +	case 3: a gimple assign store right after the call to .DEFERRED_INIT
> +		that has the LHS of .DEFERRED_INIT as the RHS as following:
> +		  _1 = .DEFERRED_INIT (4, 2, &"i1"[0]);
> +		  i1 = _1.  */
> +    return NULL;
> +  else
> +    warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
> +		"statement will never be executed");
> +  return stmt;
> +}
> +
>  /* Callback for walk_gimple_seq.  */
>  
>  static tree
> -warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
> -			   struct walk_stmt_info *wi)
> +warn_switch_unreachable_and_auto_init_r (gimple_stmt_iterator *gsi_p,
> +					 bool *handled_ops_p,
> +					 struct walk_stmt_info *wi)
>  {
>    gimple *stmt = gsi_stmt (*gsi_p);
> +  bool unreachable_issued = wi->info != NULL;
>  
>    *handled_ops_p = true;
>    switch (gimple_code (stmt))
> @@ -2046,8 +2088,12 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
>  	 worse location info.  */
>        if (gimple_try_eval (stmt) == NULL)
>  	{
> -	  wi->info = stmt;
> -	  return integer_zero_node;
> +	  if (warn_switch_unreachable && !unreachable_issued)
> +	    wi->info = emit_warn_switch_unreachable (stmt);
> +
> +	  /* Stop when auto var init warning is not on.  */
> +	  if (!warn_trivial_auto_var_init)
> +	    return integer_zero_node;
>  	}
>        /* Fall through.  */
>      case GIMPLE_BIND:
> @@ -2064,28 +2110,55 @@ warn_switch_unreachable_r (gimple_stmt_iterator *gsi_p, bool *handled_ops_p,
>  	 there will be non-debug stmts too, and we'll catch those.  */
>        break;
>  
> +    case GIMPLE_LABEL:
> +      /* Stop till the first Label.  */
> +      return integer_zero_node;
>      case GIMPLE_CALL:
>        if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
>  	{
>  	  *handled_ops_p = false;
>  	  break;
>  	}
> +      if (warn_trivial_auto_var_init
> +	  && flag_auto_var_init > AUTO_INIT_UNINITIALIZED
> +	  && gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
> +	{
> +	  /* Get the variable name from the 3rd argument of call.  */
> +	  tree var_name = gimple_call_arg (stmt, 2);
> +	  var_name = TREE_OPERAND (TREE_OPERAND (var_name, 0), 0);
> +	  const char *var_name_str = TREE_STRING_POINTER (var_name);
> +
> +	  warning_at (gimple_location (stmt), OPT_Wtrivial_auto_var_init,
> +		      "%qs cannot be initialized with"
> +		      "%<-ftrivial-auto-var_init%>",
> +		      var_name_str);
> +	  break;
> +       }
> +
>        /* Fall through.  */
>      default:
> -      /* Save the first "real" statement (not a decl/lexical scope/...).  */
> -      wi->info = stmt;
> -      return integer_zero_node;
> +      /* check the first "real" statement (not a decl/lexical scope/...), issue
> +	 warning if needed.  */
> +      if (warn_switch_unreachable && !unreachable_issued)
> +	wi->info = emit_warn_switch_unreachable (stmt);
> +      /* Stop when auto var init warning is not on.  */
> +      if (!warn_trivial_auto_var_init)
> +	return integer_zero_node;
> +      break;
>      }
>    return NULL_TREE;
>  }
>  
> +
>  /* Possibly warn about unreachable statements between switch's controlling
> -   expression and the first case.  SEQ is the body of a switch expression.  */
> +   expression and the first case.  Also warn about -ftrivial-auto-var-init
> +   cannot initialize the auto variable under such situation.
> +   SEQ is the body of a switch expression.  */
>  
>  static void
> -maybe_warn_switch_unreachable (gimple_seq seq)
> +maybe_warn_switch_unreachable_and_auto_init (gimple_seq seq)
>  {
> -  if (!warn_switch_unreachable
> +  if ((!warn_switch_unreachable && !warn_trivial_auto_var_init)
>        /* This warning doesn't play well with Fortran when optimizations
>  	 are on.  */
>        || lang_GNU_Fortran ()
> @@ -2093,21 +2166,9 @@ maybe_warn_switch_unreachable (gimple_seq seq)
>      return;
>  
>    struct walk_stmt_info wi;
> -  memset (&wi, 0, sizeof (wi));
> -  walk_gimple_seq (seq, warn_switch_unreachable_r, NULL, &wi);
> -  gimple *stmt = (gimple *) wi.info;
>  
> -  if (stmt && gimple_code (stmt) != GIMPLE_LABEL)
> -    {
> -      if (gimple_code (stmt) == GIMPLE_GOTO
> -	  && TREE_CODE (gimple_goto_dest (stmt)) == LABEL_DECL
> -	  && DECL_ARTIFICIAL (gimple_goto_dest (stmt)))
> -	/* Don't warn for compiler-generated gotos.  These occur
> -	   in Duff's devices, for example.  */;
> -      else
> -	warning_at (gimple_location (stmt), OPT_Wswitch_unreachable,
> -		    "statement will never be executed");
> -    }
> +  memset (&wi, 0, sizeof (wi));
> +  walk_gimple_seq (seq, warn_switch_unreachable_and_auto_init_r, NULL, &wi);
>  }
>  
>  
> @@ -2640,7 +2701,7 @@ gimplify_switch_expr (tree *expr_p, gimple_seq *pre_p)
>        gimplify_stmt (&SWITCH_BODY (switch_expr), &switch_body_seq);
>  
>        gimplify_ctxp->in_switch_expr = old_in_switch_expr;
> -      maybe_warn_switch_unreachable (switch_body_seq);
> +      maybe_warn_switch_unreachable_and_auto_init (switch_body_seq);
>        maybe_warn_implicit_fallthrough (switch_body_seq);
>        /* Only do this for the outermost GIMPLE_SWITCH.  */
>        if (!gimplify_ctxp->in_switch_expr)
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr102276-1.c b/gcc/testsuite/gcc.dg/auto-init-pr102276-1.c
> new file mode 100644
> index 00000000000..d574926e0c8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr102276-1.c
> @@ -0,0 +1,38 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -ftrivial-auto-var-init=zero" } */
> +
> +int g(int *);
> +int f()
> +{
> +    switch (0) { 
> +        int x;  /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g(&x);
> +    }
> +}
> +
> +int g1(int);
> +int f1()
> +{
> +    switch (0) {
> +        int x; /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g1(x);  /* { dg-warning "is used uninitialized" } */
> +    }
> +}
> +
> +struct S
> +{
> +  char a;
> +  int b;
> +};
> +int g2(int);
> +int f2(int input)
> +{
> +    switch (0) {
> +        struct S x; /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g2(input) + x.b;  /* { dg-warning "is used uninitialized" } */
> +    }
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr102276-2.c b/gcc/testsuite/gcc.dg/auto-init-pr102276-2.c
> new file mode 100644
> index 00000000000..779d3ec3882
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr102276-2.c
> @@ -0,0 +1,38 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -ftrivial-auto-var-init=pattern" } */
> +
> +int g(int *);
> +int f()
> +{
> +    switch (0) { 
> +        int x;  /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g(&x);
> +    }
> +}
> +
> +int g1(int);
> +int f1()
> +{
> +    switch (0) {
> +        int x; /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g1(x);  /* { dg-warning "is used uninitialized" } */
> +    }
> +}
> +
> +struct S
> +{
> +  char a;
> +  int b;
> +};
> +int g2(int);
> +int f2(int input)
> +{
> +    switch (0) {
> +        struct S x; /* { dg-bogus "statement will never be executed" } */
> +        default:
> +        return g2(input) + x.b;  /* { dg-warning "is used uninitialized" } */
> +    }
> +}
> +
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr102276-3.c b/gcc/testsuite/gcc.dg/auto-init-pr102276-3.c
> new file mode 100644
> index 00000000000..f113f46e29d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr102276-3.c
> @@ -0,0 +1,40 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wtrivial-auto-var-init -ftrivial-auto-var-init=zero" } */
> +
> +int g(int *, int *);
> +int f()
> +{
> +    switch (0) {
> +        int x;  /* { dg-warning "cannot be initialized with" } */
> +        int y;  /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g(&x, &y);
> +    }
> +}
> +
> +int g1(int, int);
> +int f1()
> +{
> +    switch (0) {
> +        int x; /* { dg-warning "cannot be initialized with" } */
> +        int y; /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g1(x, y);
> +    }
> +}
> +
> +struct S
> +{
> +  char a;
> +  int b;
> +};
> +int g2(int);
> +int f2(int input)
> +{
> +    switch (0) {
> +        struct S x; /* { dg-warning "cannot be initialized with" } */
> +        struct S y; /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g2(input) + x.b + y.b;
> +    }
> +}
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr102276-4.c b/gcc/testsuite/gcc.dg/auto-init-pr102276-4.c
> new file mode 100644
> index 00000000000..662e0d1182e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr102276-4.c
> @@ -0,0 +1,40 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wtrivial-auto-var-init -ftrivial-auto-var-init=pattern" } */
> +
> +int g(int *, int *);
> +int f()
> +{
> +    switch (0) {
> +        int x;  /* { dg-warning "cannot be initialized with" } */
> +        int y;  /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g(&x, &y);
> +    }
> +}
> +
> +int g1(int, int);
> +int f1()
> +{
> +    switch (0) {
> +        int x; /* { dg-warning "cannot be initialized with" } */
> +        int y; /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g1(x, y);
> +    }
> +}
> +
> +struct S
> +{
> +  char a;
> +  int b;
> +};
> +int g2(int);
> +int f2(int input)
> +{
> +    switch (0) {
> +        struct S x; /* { dg-warning "cannot be initialized with" } */
> +        struct S y; /* { dg-warning "cannot be initialized with" } */
> +        default:
> +        return g2(input) + x.b + y.b;
> +    }
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)