diff mbox series

correct uninitialized object offset and size computation [PR101494]

Message ID 596aa986-8619-ae8e-8fe8-7c3bfc6e08ec@gmail.com
State New
Headers show
Series correct uninitialized object offset and size computation [PR101494] | expand

Commit Message

Martin Sebor July 22, 2021, 9:58 p.m. UTC 
The code that computes the size of an access to an object in
-Wuninitialized is limited to declared objects and so doesn't
apply to allocated objects, and doesn't correctly account for
an offset into the object and the access size.  This causes
false positives.

The attached fix tested on x86_64-linux corrects this.

Martin

Comments

Jeff Law July 23, 2021, 4:39 p.m. UTC | #1 
On 7/22/2021 3:58 PM, Martin Sebor via Gcc-patches wrote:
> The code that computes the size of an access to an object in
> -Wuninitialized is limited to declared objects and so doesn't
> apply to allocated objects, and doesn't correctly account for
> an offset into the object and the access size.  This causes
> false positives.
>
> The attached fix tested on x86_64-linux corrects this.
>
> Martin
>
> gcc-101494.diff
>
> Correct uninitialized object offset and size computation [PR101494].
>
> Resolves:
> PR middle-end/101494 - -uninitialized false alarm with memrchr of size 0
>
> gcc/ChangeLog:
>
> 	PR middle-end/101494
> 	* tree-ssa-uninit.c (builtin_call_nomodifying_p):
> 	(check_defs):
> 	(maybe_warn_operand):
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/101494
> 	* gcc.dg/uninit-38.c:
> 	* gcc.dg/uninit-41.c: New test.
> 	* gcc.dg/uninit-pr101494.c: New test.
OK once you complete the ChangeLog entry for the tree-ssa-uninit.c 
change.  Note this change only modifies maybe_warn_operand.

jeff
Martin Sebor July 28, 2021, 10:28 p.m. UTC | #2 
On 7/23/21 10:39 AM, Jeff Law wrote:
> 
> 
> On 7/22/2021 3:58 PM, Martin Sebor via Gcc-patches wrote:
>> The code that computes the size of an access to an object in
>> -Wuninitialized is limited to declared objects and so doesn't
>> apply to allocated objects, and doesn't correctly account for
>> an offset into the object and the access size.  This causes
>> false positives.
>>
>> The attached fix tested on x86_64-linux corrects this.
>>
>> Martin
>>
>> gcc-101494.diff
>>
>> Correct uninitialized object offset and size computation [PR101494].
>>
>> Resolves:
>> PR middle-end/101494 - -uninitialized false alarm with memrchr of size 0
>>
>> gcc/ChangeLog:
>>
>> 	PR middle-end/101494
>> 	* tree-ssa-uninit.c (builtin_call_nomodifying_p):
>> 	(check_defs):
>> 	(maybe_warn_operand):
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR middle-end/101494
>> 	* gcc.dg/uninit-38.c:
>> 	* gcc.dg/uninit-41.c: New test.
>> 	* gcc.dg/uninit-pr101494.c: New test.
> OK once you complete the ChangeLog entry for the tree-ssa-uninit.c 
> change.  Note this change only modifies maybe_warn_operand.

Whoops.  Fixed and pushed in r12-2583.

Martin
diff mbox series

Patch

Correct uninitialized object offset and size computation [PR101494].

Resolves:
PR middle-end/101494 - -uninitialized false alarm with memrchr of size 0

gcc/ChangeLog:

	PR middle-end/101494
	* tree-ssa-uninit.c (builtin_call_nomodifying_p):
	(check_defs):
	(maybe_warn_operand):

gcc/testsuite/ChangeLog:

	PR middle-end/101494
	* gcc.dg/uninit-38.c:
	* gcc.dg/uninit-41.c: New test.
	* gcc.dg/uninit-pr101494.c: New test.

@@ -304,16 +344,20 @@  maybe_warn_operand (ao_ref &ref, gimple *stmt, tree lhs, tree rhs,
       || get_no_uninit_warning (base))
     return NULL_TREE;
 
-  /* Do not warn if the access is fully outside of the variable.  */
+  /* Do not warn if the access is zero size or if it's fully outside
+     the object.  */
   poly_int64 decl_size;
+  if (known_size_p (ref.size)
+      && known_eq (ref.max_size, ref.size)
+      && (known_eq (ref.size, 0)
+	  || known_le (ref.offset + ref.size, 0)))
+    return NULL_TREE;
+
   if (DECL_P (base)
-      && ((known_size_p (ref.size)
-	   && known_eq (ref.max_size, ref.size)
-	   && known_le (ref.offset + ref.size, 0))
-	  || (known_ge (ref.offset, 0)
-	      && DECL_SIZE (base)
-	      && poly_int_tree_p (DECL_SIZE (base), &decl_size)
-	      && known_le (decl_size, ref.offset))))
+      && known_ge (ref.offset, 0)
+      && DECL_SIZE (base)
+      && poly_int_tree_p (DECL_SIZE (base), &decl_size)
+      && known_le (decl_size, ref.offset))
     return NULL_TREE;
 
   /* Do not warn if the result of the access is then used for
diff --git a/gcc/testsuite/gcc.dg/uninit-pr101494.c b/gcc/testsuite/gcc.dg/uninit-pr101494.c
new file mode 100644
index 00000000000..4fcb5f2dc79
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr101494.c
@@ -0,0 +1,60 @@ 
+/* PR middle-end/101494 - bogus -Wmaybe-uninitialized on memrchr of size 0
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void* alloca (size_t);
+
+__attribute__ ((malloc, alloc_size (1))) void* alloc (size_t);
+
+__attribute__ ((access (read_only, 1, 2))) void* sink (void*, size_t);
+
+void test_alloca_zero (size_t i)
+{
+  char *p = alloca (0);
+  sink (p, 0);      // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_zero_p1 (size_t i)
+{
+  char *p = alloca (0);
+  sink (p + i, 0);
+}
+
+void test_alloca_cst (void)
+{
+  char *p = alloca (7);
+  sink (p, 0);      // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_cst_p1 (void)
+{
+  char *p = alloca (7);
+  sink (p, 0);      // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_cst_p7 (void)
+{
+  char *p = alloca (7);
+  sink (p + 7, 0);  // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_var (size_t n)
+{
+  char *p = alloca (n);
+  sink (p, 0);      // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_var_p1 (size_t n)
+{
+  char *p = alloca (n);
+  sink (p + 1, 0);  // { dg-bogus "\\\[-Wuninitialized" }
+}
+
+void test_alloca_var_pn (size_t n)
+{
+  char *p = alloca (n);
+  sink (p + n, 0);  // { dg-bogus "\\\[-Wuninitialized" }
+}
+