Hi, Richard,
> On Feb 23, 2022, at 1:38 AM, Richard Biener <rguent...@suse.de> wrote:
>
> On Tue, 22 Feb 2022, Qing Zhao wrote:
>
>> __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.
>>
>> The patch has been bootstrapped and regress tested on both x86 and aarch64.
>>
>> Okay for trunk?
>>
>> Thanks.
>>
>> Qing
>>
>> ======================================
>> From cf6620005f55d4a1f782332809445c270d22cf86 Mon Sep 17 00:00:00 2001
>> From: qing zhao <qing.z...@oracle.com>
>> Date: Mon, 21 Feb 2022 16:38:31 +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.
>> (clear_padding_emit_loop): Likewise.
>> (clear_padding_type): Likewise.
>> (gimple_fold_builtin_clear_padding): Likewise.
>>
>> 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 | 31 +++++++++++++++------
>> 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 ++++++++
>> 4 files changed, 55 insertions(+), 8 deletions(-)
>> 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 16f02c2d098..1e18ba3465a 100644
>> --- a/gcc/gimple-fold.cc
>> +++ b/gcc/gimple-fold.cc
>> @@ -4296,6 +4296,7 @@ clear_padding_flush (clear_padding_struct *buf, bool
>> full)
>> build_int_cst (buf->alias_type,
>> buf->off + padding_end
>> - padding_bytes));
>> + suppress_warning (dst, OPT_Wuninitialized);
>> gimple *g = gimple_build_assign (dst, src);
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4341,6 +4342,7 @@ clear_padding_flush (clear_padding_struct *buf, bool
>> full)
>> tree dst = build2_loc (buf->loc, MEM_REF, atype,
>> buf->base,
>> build_int_cst (buf->alias_type, off));
>> + suppress_warning (dst, OPT_Wuninitialized);
>> gimple *g = gimple_build_assign (dst, src);
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4370,6 +4372,7 @@ clear_padding_flush (clear_padding_struct *buf, bool
>> full)
>> atype = build_aligned_type (type, buf->align);
>> tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
>> build_int_cst (buf->alias_type, off));
>> + suppress_warning (dst, OPT_Wuninitialized);
>> tree src;
>> gimple *g;
>> if (all_ones
>> @@ -4420,6 +4423,7 @@ clear_padding_flush (clear_padding_struct *buf, bool
>> full)
>> build_int_cst (buf->alias_type,
>> buf->off + end
>> - padding_bytes));
>> + suppress_warning (dst, OPT_Wuninitialized);
>> gimple *g = gimple_build_assign (dst, src);
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4620,14 +4624,18 @@ clear_padding_emit_loop (clear_padding_struct *buf,
>> tree type,
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> clear_padding_type (buf, type, buf->sz, for_auto_init);
>> clear_padding_flush (buf, true);
>> - g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR, buf->base,
>> - size_int (buf->sz));
>> + tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
>> + buf->base, size_int (buf->sz));
>> + suppress_warning (rhs, OPT_Wuninitialized);
>> + g = gimple_build_assign (buf->base, rhs);
>
> why do we need to suppress warnings on a POINTER_PLUS_EXPR?
This is the place I was not sure either. Need some discussion…
From my understanding, __builtin_clear_padding (&object), does not _use_ any
variable,
therefore, no uninitialized usage warning should be emitted for it.
Therefore, during the folding of __builtin_clear_padding, all the artificial
usages of variables
that were introduced by the folding should be suppressed from issue
uninitialized warning.
That’s the basic idea.
As a result, I added suppress_warning for all the possible usages of a variable
introduced during
The folding.
So, my question here is: does the new expression “POINTER_PLUS_EXPR
(buf->base, buf->size)”
Introduce a usage of the “buf->base”? If so, then we should suppress warning
for this expression,
Right?
> The
> use of fold_build2 here is a step backwards btw, I'm not sure
> whether suppress_warning is properly preserved here.
Don’t quite understand here, what’s you mean by “a step backwards”?
> If needed,
> doesn't suppress_warning (g, OPT_Wuninitialized) work as well,
> thus suppress the warning on the stmt?
I am not sure here.
>
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> g = gimple_build_label (l2);
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> - g = gimple_build_cond (NE_EXPR, buf->base, end, l1, l3);
>> + tree cond_expr = fold_build2 (NE_EXPR, boolean_type_node, buf->base, end);
>> + suppress_warning (cond_expr, OPT_Wuninitialized);
>> + g = gimple_build_cond_from_tree (cond_expr, l1, l3);
>
> Likewise.
>
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> g = gimple_build_label (l3);
>> @@ -4774,12 +4782,16 @@ clear_padding_type (clear_padding_struct *buf, tree
>> type,
>> tree elttype = TREE_TYPE (type);
>> buf->base = create_tmp_var (build_pointer_type (elttype));
>> tree end = make_ssa_name (TREE_TYPE (buf->base));
>> - gimple *g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR,
>> - base, size_int (off));
>> + tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (base),
>> + base, size_int (off));
>> + suppress_warning (rhs, OPT_Wuninitialized);
>> + gimple *g = gimple_build_assign (buf->base, rhs);
>
> Likewise and below - I'd have expected we only need to suppress
> -Wuninitialized on memory references. Can you clarify?
Based on the current testing case of PR104550, suppress_warning for memory
references is enough to
resolve the bug, however, if the folding of __builtin_clear_padding might
introduce new loops, it will
emit POINTER_PLUS_EXPR, I am not sure whether we need to suppress warning for
them.
Thanks.
Qing
>
> Thanks,
> Richard.
>
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> - g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf->base,
>> - size_int (sz));
>> + rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
>> + buf->base, size_int (sz));
>> + suppress_warning (rhs, OPT_Wuninitialized);
>> + g = gimple_build_assign (end, rhs);
>> gimple_set_location (g, buf->loc);
>> gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> buf->sz = fldsz;
>> @@ -4933,7 +4945,10 @@ gimple_fold_builtin_clear_padding
>> (gimple_stmt_iterator *gsi)
>> gimple *g = gimple_build_assign (buf.base, ptr);
>> gimple_set_location (g, loc);
>> gsi_insert_before (gsi, g, GSI_SAME_STMT);
>> - g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf.base, sz);
>> + tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf.base),
>> + buf.base, sz);
>> + suppress_warning (rhs, OPT_Wuninitialized);
>> + g = gimple_build_assign (end, rhs);
>> gimple_set_location (g, loc);
>> gsi_insert_before (gsi, g, GSI_SAME_STMT);
>> buf.sz = eltsz;
>> 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 00000000000..a08110c3a17
>> --- /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 00000000000..2c395b32d32
>> --- /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 00000000000..9893e37f12d
>> --- /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 <rguent...@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)
