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

[Qing Zhao <qing.zhao@oracle.com>]

> 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)