diff mbox series

[V3,middle-end/104550] Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding

Message ID 2801D462-0547-4E07-BF31-CA60182CA160@oracle.com
State New
Headers
Series [V3,middle-end/104550] Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding |

Commit Message

Qing Zhao Feb. 25, 2022, 1:23 a.m. UTC 
  Hi, Jakub and Richard:

This is the 3rd version of the patch, the major change compared to the previous version are:

1. Add warning_enabled_at guard before “suppress_warning”
2. Add location to the call to __builtin_clear_padding for auto init.

The patch has been bootstrapped and regress tested on both x86 and aarch64.
Okay for trunk?

Thanks.

Qing

==================================
From 8314ded4ca0f59c5a3ec431c9c3768fcaf2a0949 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Thu, 24 Feb 2022 22:38:38 +0000
Subject: [PATCH] Suppress uninitialized warnings for new created uses from
 __builtin_clear_padding folding [PR104550]

__builtin_clear_padding(&object) will clear all the padding bits of the object.
actually, it doesn't involve any use of an user variable. Therefore, users do
not expect any uninitialized warning from it. It's reasonable to suppress
uninitialized warnings for all new created uses from __builtin_clear_padding
folding.

	PR middle-end/104550

gcc/ChangeLog:

	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
	created uses.
	* gimplify.cc (gimple_add_padding_init_for_auto_var): Set
	location for new created call to __builtin_clear_padding.

gcc/testsuite/ChangeLog:

	* gcc.dg/auto-init-pr104550-1.c: New test.
	* gcc.dg/auto-init-pr104550-2.c: New test.
	* gcc.dg/auto-init-pr104550-3.c: New test.
---
 gcc/gimple-fold.cc                          | 11 ++++++++++-
 gcc/gimplify.cc                             |  1 +
 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 ++++++++++
 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 +++++++++++
 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 +++++++++++
 5 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c

Comments

Richard Biener Feb. 25, 2022, 8:38 a.m. UTC | #1 
On Fri, 25 Feb 2022, Qing Zhao wrote:

> Hi, Jakub and Richard:
> 
> This is the 3rd version of the patch, the major change compared to the previous version are:
> 
> 1. Add warning_enabled_at guard before “suppress_warning”
> 2. Add location to the call to __builtin_clear_padding for auto init.
> 
> The patch has been bootstrapped and regress tested on both x86 and aarch64.
> Okay for trunk?
> 
> Thanks.
> 
> Qing
> 
> ==================================
> From 8314ded4ca0f59c5a3ec431c9c3768fcaf2a0949 Mon Sep 17 00:00:00 2001
> From: Qing Zhao <qing.zhao@oracle.com>
> Date: Thu, 24 Feb 2022 22:38:38 +0000
> Subject: [PATCH] Suppress uninitialized warnings for new created uses from
>  __builtin_clear_padding folding [PR104550]
> 
> __builtin_clear_padding(&object) will clear all the padding bits of the object.
> actually, it doesn't involve any use of an user variable. Therefore, users do
> not expect any uninitialized warning from it. It's reasonable to suppress
> uninitialized warnings for all new created uses from __builtin_clear_padding
> folding.
> 
> 	PR middle-end/104550
> 
> gcc/ChangeLog:
> 
> 	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
> 	created uses.
> 	* gimplify.cc (gimple_add_padding_init_for_auto_var): Set
> 	location for new created call to __builtin_clear_padding.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/auto-init-pr104550-1.c: New test.
> 	* gcc.dg/auto-init-pr104550-2.c: New test.
> 	* gcc.dg/auto-init-pr104550-3.c: New test.
> ---
>  gcc/gimple-fold.cc                          | 11 ++++++++++-
>  gcc/gimplify.cc                             |  1 +
>  gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 ++++++++++
>  gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 +++++++++++
>  gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 +++++++++++
>  5 files changed, 43 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> 
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 16f02c2d098d..67b4963ffd96 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "attribs.h"
>  #include "asan.h"
>  #include "diagnostic-core.h"
> +#include "diagnostic.h"
>  #include "intl.h"
>  #include "calls.h"
>  #include "tree-vector-builder.h"
> @@ -4379,7 +4380,15 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>  	      else
>  		{
>  		  src = make_ssa_name (type);
> -		  g = gimple_build_assign (src, unshare_expr (dst));
> +		  tree tmp_dst = unshare_expr (dst);
> +		  /* The folding introduces a read from the tmp_dst, we should
> +		     prevent uninitialized warning analysis from issuing warning
> +		     for such fake read.  */
> +		  if (warning_enabled_at (buf->loc, OPT_Wuninitialized)
> +		      || warning_enabled_at (buf->loc,
> +					     OPT_Wmaybe_uninitialized))
> +		    suppress_warning (tmp_dst, OPT_Wuninitialized);
> +		  g = gimple_build_assign (src, tmp_dst);

So what about just gimple_set_no_warning (g, true); ?  (sorry for
the ping-pong between us three...)

>  		  gimple_set_location (g, buf->loc);
>  		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>  		  tree mask = native_interpret_expr (type,
> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
> index f570daa015a5..977cf458f858 100644
> --- a/gcc/gimplify.cc
> +++ b/gcc/gimplify.cc
> @@ -1823,6 +1823,7 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
>  
>    gimple *call = gimple_build_call (fn, 2, addr_of_decl,
>  				    build_one_cst (TREE_TYPE (addr_of_decl)));
> +  gimple_set_location (call, EXPR_LOCATION (decl));

I believe EXPR_LOCATION (decl) is bogus, 'decl's have DECL_LOCATION,
EXPR_LOCATION here will just return UNKNOWN_LOCATION.

>    gimplify_seq_add_stmt (seq_p, call);
>  }
>  
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
> new file mode 100644
> index 000000000000..a08110c3a170
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
> @@ -0,0 +1,10 @@
> +/* PR 104550*/
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
> +}
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
> new file mode 100644
> index 000000000000..2c395b32d322
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
> @@ -0,0 +1,11 @@
> +/* PR 104550 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info; 
> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
> +}
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> new file mode 100644
> index 000000000000..9893e37f12d8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> @@ -0,0 +1,11 @@
> +/* PR 104550 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
> +}
>
Qing Zhao Feb. 25, 2022, 3:03 p.m. UTC | #2 
> On Feb 25, 2022, at 2:38 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Fri, 25 Feb 2022, Qing Zhao wrote:
> 
>> Hi, Jakub and Richard:
>> 
>> This is the 3rd version of the patch, the major change compared to the previous version are:
>> 
>> 1. Add warning_enabled_at guard before “suppress_warning”
>> 2. Add location to the call to __builtin_clear_padding for auto init.
>> 
>> The patch has been bootstrapped and regress tested on both x86 and aarch64.
>> Okay for trunk?
>> 
>> Thanks.
>> 
>> Qing
>> 
>> ==================================
>> From 8314ded4ca0f59c5a3ec431c9c3768fcaf2a0949 Mon Sep 17 00:00:00 2001
>> From: Qing Zhao <qing.zhao@oracle.com>
>> Date: Thu, 24 Feb 2022 22:38:38 +0000
>> Subject: [PATCH] Suppress uninitialized warnings for new created uses from
>> __builtin_clear_padding folding [PR104550]
>> 
>> __builtin_clear_padding(&object) will clear all the padding bits of the object.
>> actually, it doesn't involve any use of an user variable. Therefore, users do
>> not expect any uninitialized warning from it. It's reasonable to suppress
>> uninitialized warnings for all new created uses from __builtin_clear_padding
>> folding.
>> 
>> 	PR middle-end/104550
>> 
>> gcc/ChangeLog:
>> 
>> 	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
>> 	created uses.
>> 	* gimplify.cc (gimple_add_padding_init_for_auto_var): Set
>> 	location for new created call to __builtin_clear_padding.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.dg/auto-init-pr104550-1.c: New test.
>> 	* gcc.dg/auto-init-pr104550-2.c: New test.
>> 	* gcc.dg/auto-init-pr104550-3.c: New test.
>> ---
>> gcc/gimple-fold.cc                          | 11 ++++++++++-
>> gcc/gimplify.cc                             |  1 +
>> gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 ++++++++++
>> gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 +++++++++++
>> gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 +++++++++++
>> 5 files changed, 43 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> 
>> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
>> index 16f02c2d098d..67b4963ffd96 100644
>> --- a/gcc/gimple-fold.cc
>> +++ b/gcc/gimple-fold.cc
>> @@ -62,6 +62,7 @@ along with GCC; see the file COPYING3.  If not see
>> #include "attribs.h"
>> #include "asan.h"
>> #include "diagnostic-core.h"
>> +#include "diagnostic.h"
>> #include "intl.h"
>> #include "calls.h"
>> #include "tree-vector-builder.h"
>> @@ -4379,7 +4380,15 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>> 	      else
>> 		{
>> 		  src = make_ssa_name (type);
>> -		  g = gimple_build_assign (src, unshare_expr (dst));
>> +		  tree tmp_dst = unshare_expr (dst);
>> +		  /* The folding introduces a read from the tmp_dst, we should
>> +		     prevent uninitialized warning analysis from issuing warning
>> +		     for such fake read.  */
>> +		  if (warning_enabled_at (buf->loc, OPT_Wuninitialized)
>> +		      || warning_enabled_at (buf->loc,
>> +					     OPT_Wmaybe_uninitialized))
>> +		    suppress_warning (tmp_dst, OPT_Wuninitialized);
>> +		  g = gimple_build_assign (src, tmp_dst);
> 
> So what about just gimple_set_no_warning (g, true); ?  (sorry for
> the ping-pong between us three...)

This didn’t work.  The small testing case still failed. This is due to in tree-ssa-uninit.cc,
 it checks get_no_uninit_warning (RHS), not for the whole stmt.

We can update tree-sea-uninit.cc to check the whole stmt, but I am not sure whether doing this might introduce other issue.

Qing

> 
>> 		  gimple_set_location (g, buf->loc);
>> 		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 		  tree mask = native_interpret_expr (type,
>> diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
>> index f570daa015a5..977cf458f858 100644
>> --- a/gcc/gimplify.cc
>> +++ b/gcc/gimplify.cc
>> @@ -1823,6 +1823,7 @@ gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
>> 
>>   gimple *call = gimple_build_call (fn, 2, addr_of_decl,
>> 				    build_one_cst (TREE_TYPE (addr_of_decl)));
>> +  gimple_set_location (call, EXPR_LOCATION (decl));
> 
> I believe EXPR_LOCATION (decl) is bogus, 'decl's have DECL_LOCATION,
> EXPR_LOCATION here will just return UNKNOWN_LOCATION.
> 
>>   gimplify_seq_add_stmt (seq_p, call);
>> }
>> 
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> new file mode 100644
>> index 000000000000..a08110c3a170
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> @@ -0,0 +1,10 @@
>> +/* PR 104550*/
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> new file mode 100644
>> index 000000000000..2c395b32d322
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> @@ -0,0 +1,11 @@
>> +/* PR 104550 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info; 
>> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> new file mode 100644
>> index 000000000000..9893e37f12d8
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> @@ -0,0 +1,11 @@
>> +/* PR 104550 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
>> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
>> +}
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
diff mbox series

Patch

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 16f02c2d098d..67b4963ffd96 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -62,6 +62,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "attribs.h"
 #include "asan.h"
 #include "diagnostic-core.h"
+#include "diagnostic.h"
 #include "intl.h"
 #include "calls.h"
 #include "tree-vector-builder.h"
@@ -4379,7 +4380,15 @@  clear_padding_flush (clear_padding_struct *buf, bool full)
 	      else
 		{
 		  src = make_ssa_name (type);
-		  g = gimple_build_assign (src, unshare_expr (dst));
+		  tree tmp_dst = unshare_expr (dst);
+		  /* The folding introduces a read from the tmp_dst, we should
+		     prevent uninitialized warning analysis from issuing warning
+		     for such fake read.  */
+		  if (warning_enabled_at (buf->loc, OPT_Wuninitialized)
+		      || warning_enabled_at (buf->loc,
+					     OPT_Wmaybe_uninitialized))
+		    suppress_warning (tmp_dst, OPT_Wuninitialized);
+		  g = gimple_build_assign (src, tmp_dst);
 		  gimple_set_location (g, buf->loc);
 		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
 		  tree mask = native_interpret_expr (type,
diff --git a/gcc/gimplify.cc b/gcc/gimplify.cc
index f570daa015a5..977cf458f858 100644
--- a/gcc/gimplify.cc
+++ b/gcc/gimplify.cc
@@ -1823,6 +1823,7 @@  gimple_add_padding_init_for_auto_var (tree decl, bool is_vla,
 
   gimple *call = gimple_build_call (fn, 2, addr_of_decl,
 				    build_one_cst (TREE_TYPE (addr_of_decl)));
+  gimple_set_location (call, EXPR_LOCATION (decl));
   gimplify_seq_add_stmt (seq_p, call);
 }
 
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
new file mode 100644
index 000000000000..a08110c3a170
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
@@ -0,0 +1,10 @@ 
+/* PR 104550*/
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
+}
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
new file mode 100644
index 000000000000..2c395b32d322
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
@@ -0,0 +1,11 @@ 
+/* PR 104550 */
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info; 
+ __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
+}
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
new file mode 100644
index 000000000000..9893e37f12d8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
@@ -0,0 +1,11 @@ 
+/* PR 104550 */
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
+ __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
+}