diff mbox series

tree-optimization/104009: Conservative underflow estimate in object size

Message ID 20220114033109.286522-1-siddhesh@gotplt.org
State New
Headers show
Series tree-optimization/104009: Conservative underflow estimate in object size | expand

Commit Message

Siddhesh Poyarekar Jan. 14, 2022, 3:31 a.m. UTC 
Restrict negative offset computation only to dynamic object sizes, where
size expressions are accurate and not a maximum/minimum estimate and in
cases where negative offsets definitely mean an underflow, e.g. in
MEM_REF of the whole object with negative ofset in addr_object_size.

This ends up missing some cases where __builtin_object_size could have
come up with more precise results, so tests have been adjusted to
reflect that.

gcc/ChangeLog:

	PR tree-optimization/104009
	* tree-object-size.c (compute_builtin_object_size): Bail out on
	negative offset.
	(plus_stmt_object_size): Return maximum of wholesize and minimum
	of 0 for negative offset.

gcc/testsuite/ChangeLog:

	PR tree-optimization/104009
	* gcc.dg/builtin-object-size-1.c (test10): New test.
	* gcc.dg/builtin-object-size-3.c (test10): Likewise.
	(test9): Expect zero size for negative offsets.
	* gcc.dg/builtin-object-size-4.c (test8): Likewise.
	* gcc.dg/builtin-object-size-5.c (test7): Drop test for
	__builtin_object_size.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Testing:
- bootstrap build+test for x86_64
- build+test for i686
- bootstrap build --with-build-config=bootstrap-ubsan

 gcc/testsuite/gcc.dg/builtin-object-size-1.c | 27 ++++++++++++++++
 gcc/testsuite/gcc.dg/builtin-object-size-3.c | 34 +++++++++++++++++---
 gcc/testsuite/gcc.dg/builtin-object-size-4.c |  6 ++--
 gcc/testsuite/gcc.dg/builtin-object-size-5.c |  2 ++
 gcc/tree-object-size.c                       | 15 +++++++--
 5 files changed, 75 insertions(+), 9 deletions(-)

Comments

Jakub Jelinek Jan. 14, 2022, 4:48 a.m. UTC | #1 
On Fri, Jan 14, 2022 at 09:01:09AM +0530, Siddhesh Poyarekar wrote:
> Restrict negative offset computation only to dynamic object sizes, where
> size expressions are accurate and not a maximum/minimum estimate and in
> cases where negative offsets definitely mean an underflow, e.g. in
> MEM_REF of the whole object with negative ofset in addr_object_size.
> 
> This ends up missing some cases where __builtin_object_size could have
> come up with more precise results, so tests have been adjusted to
> reflect that.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/104009
> 	* tree-object-size.c (compute_builtin_object_size): Bail out on
> 	negative offset.
> 	(plus_stmt_object_size): Return maximum of wholesize and minimum
> 	of 0 for negative offset.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/104009
> 	* gcc.dg/builtin-object-size-1.c (test10): New test.
> 	* gcc.dg/builtin-object-size-3.c (test10): Likewise.
> 	(test9): Expect zero size for negative offsets.
> 	* gcc.dg/builtin-object-size-4.c (test8): Likewise.
> 	* gcc.dg/builtin-object-size-5.c (test7): Drop test for
> 	__builtin_object_size.

Ok.

	Jakub
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
index 161f426ec0b..b772e2da9b9 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
@@ -603,6 +603,32 @@  test9 (unsigned cond)
 #endif
 }
 
+void
+__attribute__ ((noinline))
+test10 (void)
+{
+  static char buf[255];
+  unsigned int i, len = sizeof (buf);
+  char *p = buf;
+
+  for (i = 0 ; i < sizeof (buf) ; i++)
+    {
+      if (len < 2)
+	{
+#ifdef __builtin_object_size
+	  if (__builtin_object_size (p - 3, 0) != sizeof (buf) - i + 3)
+	    abort ();
+#else
+	  if (__builtin_object_size (p - 3, 0) != sizeof (buf))
+	    abort ();
+#endif
+	  break;
+	}
+      p++;
+      len--;
+    }
+}
+
 int
 main (void)
 {
@@ -617,5 +643,6 @@  main (void)
   test7 ();
   test8 ();
   test9 (1);
+  test10 ();
   exit (0);
 }
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-3.c b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
index db31171a8bd..44a99189776 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-3.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-3.c
@@ -581,7 +581,7 @@  test9 (unsigned cond)
   if (__builtin_object_size (&p[-4], 2) != (cond ? 6 : 10))
     abort ();
 #else
-  if (__builtin_object_size (&p[-4], 2) != 6)
+  if (__builtin_object_size (&p[-4], 2) != 0)
     abort ();
 #endif
 
@@ -592,7 +592,7 @@  test9 (unsigned cond)
   if (__builtin_object_size (p, 2) != ((cond ? 2 : 6) + cond))
     abort ();
 #else
-  if (__builtin_object_size (p, 2) != 2)
+  if (__builtin_object_size (p, 2) != 0)
     abort ();
 #endif
 
@@ -605,12 +605,37 @@  test9 (unsigned cond)
       != sizeof (y) - __builtin_offsetof (struct A, c) - 8 + cond)
     abort ();
 #else
-  if (__builtin_object_size (p, 2)
-      != sizeof (y) - __builtin_offsetof (struct A, c) - 8)
+  if (__builtin_object_size (p, 2) != 0)
     abort ();
 #endif
 }
 
+void
+__attribute__ ((noinline))
+test10 (void)
+{
+  static char buf[255];
+  unsigned int i, len = sizeof (buf);
+  char *p = buf;
+
+  for (i = 0 ; i < sizeof (buf) ; i++)
+    {
+      if (len < 2)
+	{
+#ifdef __builtin_object_size
+	  if (__builtin_object_size (p - 3, 2) != sizeof (buf) - i + 3)
+	    abort ();
+#else
+	  if (__builtin_object_size (p - 3, 2) != 0)
+	    abort ();
+#endif
+	  break;
+	}
+      p++;
+      len--;
+    }
+}
+
 int
 main (void)
 {
@@ -625,5 +650,6 @@  main (void)
   test7 ();
   test8 ();
   test9 (1);
+  test10 ();
   exit (0);
 }
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-4.c b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
index f644890dd14..b9fddfed036 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-4.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-4.c
@@ -489,7 +489,7 @@  test8 (unsigned cond)
   if (__builtin_object_size (&p[-4], 3) != (cond ? 6 : 10))
     abort ();
 #else
-  if (__builtin_object_size (&p[-4], 3) != 6)
+  if (__builtin_object_size (&p[-4], 3) != 0)
     abort ();
 #endif
 
@@ -500,7 +500,7 @@  test8 (unsigned cond)
   if (__builtin_object_size (p, 3) != ((cond ? 2 : 6) + cond))
     abort ();
 #else
-  if (__builtin_object_size (p, 3) != 2)
+  if (__builtin_object_size (p, 3) != 0)
     abort ();
 #endif
 
@@ -512,7 +512,7 @@  test8 (unsigned cond)
   if (__builtin_object_size (p, 3) != sizeof (y.c) - 8 + cond)
     abort ();
 #else
-  if (__builtin_object_size (p, 3) != sizeof (y.c) - 8)
+  if (__builtin_object_size (p, 3) != 0)
     abort ();
 #endif
 }
diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-5.c b/gcc/testsuite/gcc.dg/builtin-object-size-5.c
index cb8f753b039..d1ad761cb69 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-5.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-5.c
@@ -89,6 +89,7 @@  test6 (void)
     abort ();
 }
 
+#ifdef __builtin_object_size
 void
 test7 (void)
 {
@@ -97,5 +98,6 @@  test7 (void)
   if (__builtin_object_size (p + 2, 0) != 0)
     abort ();
 }
+#endif
 
 /* { dg-final { scan-assembler-not "abort" } } */
diff --git a/gcc/tree-object-size.c b/gcc/tree-object-size.c
index 3a39e6f15bb..8be0df6ba40 100644
--- a/gcc/tree-object-size.c
+++ b/gcc/tree-object-size.c
@@ -1054,7 +1054,8 @@  compute_builtin_object_size (tree ptr, int object_size_type,
 	      ptr = gimple_assign_rhs1 (def);
 
 	      if (((object_size_type & OST_DYNAMIC)
-		   || tree_fits_shwi_p (offset))
+		   || (tree_fits_shwi_p (offset)
+		       && compare_tree_int (offset, offset_limit) <= 0))
 		  && compute_builtin_object_size (ptr, object_size_type,
 						  psize))
 		{
@@ -1346,8 +1347,18 @@  plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
       /* size_for_offset doesn't make sense for -1 size, but it does for size 0
 	 since the wholesize could be non-zero and a negative offset could give
 	 a non-zero size.  */
-      if (!size_unknown_p (bytes, 0))
+      if (size_unknown_p (bytes, 0))
+	;
+      else if ((object_size_type & OST_DYNAMIC)
+	       || compare_tree_int (op1, offset_limit) <= 0)
 	bytes = size_for_offset (bytes, op1, wholesize);
+      /* In the static case, with a negative offset, the best estimate for
+	 minimum size is size_unknown but for maximum size, the wholesize is a
+	 better estimate than size_unknown.  */
+      else if (object_size_type & OST_MINIMUM)
+	bytes = size_unknown (object_size_type);
+      else
+	bytes = wholesize;
     }
   else
     bytes = wholesize = size_unknown (object_size_type);