[PATCH] avoid a couple of missing -Wuninitialized (PR 98583, 93100)

[PATCH] avoid a couple of missing -Wuninitialized (PR 98583, 93100)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

The attached change teaches the uninitialized pass about
__builtin_stack_restore and __builtin___asan_mark to avoid two
classes of -Wuninitialized false negatives.

Richard, you already approved the __builtin_stack_restore change
in the bug but I figured I'd submit a patch with both changes for
approval since they affect the same piece of code.

Martin

[-- Attachment #2: gcc-93100.diff --]
[-- Type: text/x-patch, Size: 4561 bytes --]

Avoid -Wuninitialized false negatives with sanitization and VLAs.

Resolves:
PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block

gcc/ChangeLog:

	PR tree-optimization/93100
	PR middle-end/98583
	* tree-ssa-uninit.c (check_defs):

gcc/testsuite/ChangeLog:

	PR tree-optimization/93100
	PR middle-end/98583
	* g++.dg/warn/uninit-pr93100.C: New test.
	* gcc.dg/uninit-pr93100.c: New test.
	* gcc.dg/uninit-pr98583.c: New test.

diff --git a/gcc/testsuite/g++.dg/warn/uninit-pr93100.C b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
new file mode 100644
index 00000000000..c9cd3ef0174
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/uninit-pr93100.C
@@ -0,0 +1,59 @@
+/* PR tree-optimization/98508 - Sanitizer disable -Wall and -Wextra
+   { dg-do compile }
+   { dg-options "-O0 -Wall -fsanitize=address" } */
+
+struct S
+{
+  int a;
+};
+
+void warn_init_self_O0 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+  (void)&s;
+}
+
+
+void warn_init_self_use_O0 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+
+  void sink (void*);
+  sink (&s);
+}
+
+
+#pragma GCC optimize ("1")
+
+void warn_init_self_O1 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+  (void)&s;
+}
+
+
+void warn_init_self_use_O1 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+
+  void sink (void*);
+  sink (&s);
+}
+
+
+#pragma GCC optimize ("2")
+
+void warn_init_self_O2 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+  (void)&s;
+}
+
+
+void warn_init_self_use_O2 ()
+{
+  S s = S (s);      // { dg-warning "\\\[-Wuninitialized" }
+
+  void sink (void*);
+  sink (&s);
+}
diff --git a/gcc/testsuite/gcc.dg/uninit-pr93100.c b/gcc/testsuite/gcc.dg/uninit-pr93100.c
new file mode 100644
index 00000000000..61b7e434038
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr93100.c
@@ -0,0 +1,74 @@
+/* PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
+   { dg-do compile }
+   { dg-options "-Wall -fsanitize=address" } */
+
+struct A
+{
+  _Bool b;
+  int i;
+};
+
+void warn_A_b_O0 (void)
+{
+  struct A a;
+
+  if (a.b)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
+
+void warn_A_i_O0 (void)
+{
+  struct A a;
+
+  if (a.i)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
+
+#pragma GCC optimize ("1")
+
+void warn_A_b_O1 (void)
+{
+  struct A a;
+
+  if (a.b)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
+
+void warn_A_i_O1 (void)
+{
+  struct A a;
+
+  if (a.i)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
+
+
+#pragma GCC optimize ("2")
+
+void warn_A_b_O2 (void)
+{
+  struct A a;
+
+  if (a.b)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
+
+void warn_A_i_O2 (void)
+{
+  struct A a;
+
+  if (a.i)          // { dg-warning "\\\[-Wuninitialized" }
+    {
+      (void)&a;
+    }
+}
diff --git a/gcc/testsuite/gcc.dg/uninit-pr98583.c b/gcc/testsuite/gcc.dg/uninit-pr98583.c
new file mode 100644
index 00000000000..638b0295809
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr98583.c
@@ -0,0 +1,31 @@
+/* PR middle-end/98583 - missing -Wuninitialized reading from a second VLA
+   in its own block
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+void f (int*);
+void g (int);
+
+void h1 (int n)
+{
+  int a[n];
+  f (a);
+
+  int b[n];
+  g (b[1]);         // { dg-warning "\\\[-Wuninitialized" }
+}
+
+void h2 (int n, int i, int j)
+{
+  if (i)
+    {
+      int a[n];
+      f (a);
+    }
+
+  if (j)
+    {
+      int b[n];
+      g (b[1]);     // { dg-warning "\\\[-Wmaybe-uninitialized" }
+    }
+}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index 0800f596ab1..f55ce1939ac 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -209,6 +209,16 @@ check_defs (ao_ref *ref, tree vdef, void *data_)
 {
   check_defs_data *data = (check_defs_data *)data_;
   gimple *def_stmt = SSA_NAME_DEF_STMT (vdef);
+
+  /* The ASAN_MARK intrinsic doesn't modify the variable.  */
+  if (is_gimple_call (def_stmt)
+      && gimple_call_internal_p (def_stmt, IFN_ASAN_MARK))
+    return false;
+
+  /* End of VLA scope is not a kill.  */
+  if (gimple_call_builtin_p (def_stmt, BUILT_IN_STACK_RESTORE))
+    return false;
+
   /* If this is a clobber then if it is not a kill walk past it.  */
   if (gimple_clobber_p (def_stmt))
     {

Re: [PATCH] avoid a couple of missing -Wuninitialized (PR 98583, 93100)

[Jeff Law]

On 5/11/2021 1:49 PM, Martin Sebor via Gcc-patches wrote:
> The attached change teaches the uninitialized pass about
> __builtin_stack_restore and __builtin___asan_mark to avoid two
> classes of -Wuninitialized false negatives.
>
> Richard, you already approved the __builtin_stack_restore change
> in the bug but I figured I'd submit a patch with both changes for
> approval since they affect the same piece of code.
>
> Martin
>
> gcc-93100.diff
>
> Avoid -Wuninitialized false negatives with sanitization and VLAs.
>
> Resolves:
> PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
> PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block
>
> gcc/ChangeLog:
>
> 	PR tree-optimization/93100
> 	PR middle-end/98583
> 	* tree-ssa-uninit.c (check_defs):
>
> gcc/testsuite/ChangeLog:
>
> 	PR tree-optimization/93100
> 	PR middle-end/98583
> 	* g++.dg/warn/uninit-pr93100.C: New test.
> 	* gcc.dg/uninit-pr93100.c: New test.
> 	* gcc.dg/uninit-pr98583.c: New test.

OK.  I wonder if it would make sense to describe this property when we 
construct the builtin and check that property rather than each builtin 
we find over time.  Your call on whether or not to explore that.


Jeff

Re: [PATCH] avoid a couple of missing -Wuninitialized (PR 98583, 93100)

[Martin Sebor]

On 5/13/21 1:03 PM, Jeff Law wrote:
> 
> On 5/11/2021 1:49 PM, Martin Sebor via Gcc-patches wrote:
>> The attached change teaches the uninitialized pass about
>> __builtin_stack_restore and __builtin___asan_mark to avoid two
>> classes of -Wuninitialized false negatives.
>>
>> Richard, you already approved the __builtin_stack_restore change
>> in the bug but I figured I'd submit a patch with both changes for
>> approval since they affect the same piece of code.
>>
>> Martin
>>
>> gcc-93100.diff
>>
>> Avoid -Wuninitialized false negatives with sanitization and VLAs.
>>
>> Resolves:
>> PR tree-optimization/93100 - gcc -fsanitize=address inhibits -Wuninitialized
>> PR middle-end/98583 - missing -Wuninitialized reading from a second VLA in its own block
>>
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/93100
>> 	PR middle-end/98583
>> 	* tree-ssa-uninit.c (check_defs):
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR tree-optimization/93100
>> 	PR middle-end/98583
>> 	* g++.dg/warn/uninit-pr93100.C: New test.
>> 	* gcc.dg/uninit-pr93100.c: New test.
>> 	* gcc.dg/uninit-pr98583.c: New test.
> 
> OK.  I wonder if it would make sense to describe this property when we 
> construct the builtin and check that property rather than each builtin 
> we find over time.  Your call on whether or not to explore that.

I like the idea.  Adding atrribute access to the built-ins would
be one way.  Attribute fn spec might be able to express the same
thing although there I'm not sure if it would apply to
the sanitizer functions.  Either way it seems worth looking into.

Thanks
Martin

> 
> 
> Jeff
>