diff mbox series

clear base0 flag for pointers (PR 100307)

Message ID c4021a5a-5917-58c9-877a-46d05353b1c0@gmail.com
State New
Headers show
Series clear base0 flag for pointers (PR 100307) | expand

Commit Message

Martin Sebor April 29, 2021, 12:51 a.m. UTC 
When the compute_objsize_r() function sees a pointer whose target
it can't determine it sets the size of the pointed to object to
the maximum but it doesn't clear the base0 flag to indicate that
the offset need not be zero-based.  This is done when the source
is in SSA form but not before.  Since the function is now also
called from the C++ front end to detect out-of-bounds placement
new (-Wplacement-new) this causes false positives there, as
reported in the PR.  The same problem also affects the "early"
-Wformat-overflow (when it runs without optimization).

Clearing the base0 flag as done in the attached patch avoids
the false positives in both warnings.

Besides avoiding false positives the change also makes some valid
(though incidental) warnings disappear.  Running -Wplacement-new
in the front end, and -Wformat-overflow before small IPA passes,
is too early; the warnings need to run before placement new is
inlined but after the program has been converted to SSA.  Since
it will introduce additional warnings I will submit separate
patches for that just for GCC 12.

I plan to commit the attached patch into GCC 12 and 11.1.

Martin

Comments

Jeff Law April 30, 2021, 3:11 p.m. UTC | #1 
On 4/28/2021 6:51 PM, Martin Sebor via Gcc-patches wrote:
> When the compute_objsize_r() function sees a pointer whose target
> it can't determine it sets the size of the pointed to object to
> the maximum but it doesn't clear the base0 flag to indicate that
> the offset need not be zero-based.  This is done when the source
> is in SSA form but not before.  Since the function is now also
> called from the C++ front end to detect out-of-bounds placement
> new (-Wplacement-new) this causes false positives there, as
> reported in the PR.  The same problem also affects the "early"
> -Wformat-overflow (when it runs without optimization).
>
> Clearing the base0 flag as done in the attached patch avoids
> the false positives in both warnings.
>
> Besides avoiding false positives the change also makes some valid
> (though incidental) warnings disappear.  Running -Wplacement-new
> in the front end, and -Wformat-overflow before small IPA passes,
> is too early; the warnings need to run before placement new is
> inlined but after the program has been converted to SSA.  Since
> it will introduce additional warnings I will submit separate
> patches for that just for GCC 12.
>
> I plan to commit the attached patch into GCC 12 and 11.1.
>
> Martin
>
> gcc-100307.diff
>
> PR middle-end/100307 - spurious -Wplacement-new with negative pointer offset
>
> gcc/ChangeLog:
>
> 	PR middle-end/100307
> 	* builtins.c (compute_objsize_r): Clear base0 for pointers.
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/100307
> 	* g++.dg/warn/Wplacement-new-size-9.C: New test.
> 	* gcc.dg/tree-ssa/builtin-sprintf-warn-26.c: New test.

OK

jeff
diff mbox series

Patch

PR middle-end/100307 - spurious -Wplacement-new with negative pointer offset

gcc/ChangeLog:

	PR middle-end/100307
	* builtins.c (compute_objsize_r): Clear base0 for pointers.

gcc/testsuite/ChangeLog:

	PR middle-end/100307
	* g++.dg/warn/Wplacement-new-size-9.C: New test.
	* gcc.dg/tree-ssa/builtin-sprintf-warn-26.c: New test.


diff --git a/gcc/builtins.c b/gcc/builtins.c
index 8c5324bf7de..e9486a46460 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5449,8 +5449,10 @@  compute_objsize_r (tree ptr, int ostype, access_ref *pref,
       if (!addr && POINTER_TYPE_P (TREE_TYPE (ptr)))
 	{
 	  /* Set the maximum size if the reference is to the pointer
-	     itself (as opposed to what it points to).  */
+	     itself (as opposed to what it points to), and clear
+	     BASE0 since the offset isn't necessarily zero-based.  */
 	  pref->set_max_size_range ();
+	  pref->base0 = false;
 	  return true;
 	}
 
diff --git a/gcc/testsuite/g++.dg/warn/Wplacement-new-size-9.C b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-9.C
new file mode 100644
index 00000000000..9478e858f87
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wplacement-new-size-9.C
@@ -0,0 +1,39 @@ 
+/* PR middle-end/100307 - spurious -Wplacement-new with negative pointer
+   offset
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+void* operator new (__SIZE_TYPE__, void *p) { return p; }
+void* operator new[] (__SIZE_TYPE__, void *p) { return p; }
+
+static char a[2];
+
+void* nowarn_scalar ()
+{
+  char* p = a + 1;
+  char *q = new (p - 1) char ();    // { dg-bogus "-Wplacement-new" }
+  return q;
+}
+
+
+void* nowarn_array ()
+{
+  char* p = a + 1;
+  char *q = new (p - 1) char[2];    // { dg-bogus "-Wplacement-new" }
+  return q;
+}
+
+void* warn_scalar ()
+{
+  char* p = a + 1;
+  char *q = new (p - 2) char ();    // { dg-warning "-Wplacement-new" "pr?????" { xfail *-*-* } }
+  return q;
+}
+
+
+void* warn_array ()
+{
+  char* p = a + 1;
+  char *q = new (p - 1) char[2];    // { dg-warning "-Wplacement-new" "pr?????" { xfail *-*-* } }
+  return q;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-26.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-26.c
new file mode 100644
index 00000000000..16a551d9c8d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-26.c
@@ -0,0 +1,38 @@ 
+/* PR middle-end/100307 - spurious -Wplacement-new with negative pointer
+   offset
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+extern int sprintf (char*, const char*, ...);
+
+char a[4];
+
+void nowarn_1m1 ()
+{
+  char *p = a + 1;
+  sprintf (p - 1, "%i", 123);   // { dg-bogus "-Wformat-overflow" }
+}
+
+void nowarn_4m3 ()
+{
+  char *p = a + 4;
+  sprintf (p - 3, "%i", 12);    // { dg-bogus "-Wformat-overflow" }
+}
+
+void warn_2m1 ()
+{
+  char *p = a + 2;
+  sprintf (p - 1, "%i", 123);   // { dg-warning "-Wformat-overflow" "pr100325" { xfail *-*-* } }
+}
+
+void warn_3m1 ()
+{
+  char *p = a + 3;
+  sprintf (p - 1, "%i", 12);    // { dg-warning "-Wformat-overflow" "pr100325" { xfail *-*-* } }
+}
+
+void warn_4m1 ()
+{
+  char *p = a + 4;
+  sprintf (p - 1, "%i", 1);     // { dg-warning "-Wformat-overflow" "pr100325" { xfail *-*-* } }
+}