This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] handle non-constant offsets in -Wstringop-overflow (PR 77608)


The attached patch enhances -Wstringop-overflow to detect more
instances of buffer overflow at compile time by handling non-
constant offsets into the destination object that are known to
be in some range.  The solution could be improved by handling
even more cases (e.g., anti-ranges or offsets relative to
pointers beyond the beginning of an object) but it's a start.

In addition to bootsrapping/regtesting GCC, also tested with
Binutils/GDB, Glibc, and the Linux kernel on x86_64 with no
regressions.

Martin

The top of GDB fails to compile at the moment so the validation
there was incomplete.
PR middle-end/77608 - missing protection on trivially detectable runtime buffer overflow

gcc/ChangeLog:

	PR middle-end/77608
	* builtins.c (compute_objsize): Handle non-constant offsets.

gcc/testsuite/ChangeLog:

	PR middle-end/77608
	* gcc.dg/Wstringop-overflow.c: New test.

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 254879)
+++ gcc/builtins.c	(working copy)
@@ -3276,11 +3276,6 @@ compute_objsize (tree dest, int ostype)
   if (compute_builtin_object_size (dest, ostype, &size))
     return build_int_cst (sizetype, size);
 
-  /* Unless computing the largest size (for memcpy and other raw memory
-     functions), try to determine the size of the object from its type.  */
-  if (!ostype)
-    return NULL_TREE;
-
   if (TREE_CODE (dest) == SSA_NAME)
     {
       gimple *stmt = SSA_NAME_DEF_STMT (dest);
@@ -3287,13 +3282,47 @@ compute_objsize (tree dest, int ostype)
       if (!is_gimple_assign (stmt))
 	return NULL_TREE;
 
+      dest = gimple_assign_rhs1 (stmt);
+
       tree_code code = gimple_assign_rhs_code (stmt);
-      if (code != ADDR_EXPR && code != POINTER_PLUS_EXPR)
+      if (code == POINTER_PLUS_EXPR)
+	{
+	  /* compute_builtin_object_size fails for addresses with
+	     non-constant offsets.  Try to determine the range of
+	     such an offset here and use it to adjus the constant
+	     size.  */
+	  tree off = gimple_assign_rhs2 (stmt);
+	  if (TREE_CODE (off) == SSA_NAME
+	      && INTEGRAL_TYPE_P (TREE_TYPE (off)))
+	    {
+	      wide_int min, max;
+	      enum value_range_type rng = get_range_info (off, &min, &max);
+
+	      if (rng == VR_RANGE)
+		{
+		  if (tree size = compute_objsize (dest, ostype))
+		    {
+		      wide_int wisiz = wi::to_wide (size);
+		      if (wi::sign_mask (min))
+			/* ignore negative offsets for now */;
+		      else if (wi::ltu_p (min, wisiz))
+			return wide_int_to_tree (TREE_TYPE (size),
+						 wi::sub (wisiz, min));
+		      else
+			return size_zero_node;
+		    }
+		}
+	    }
+	}
+      else if (code != ADDR_EXPR)
 	return NULL_TREE;
-
-      dest = gimple_assign_rhs1 (stmt);
     }
 
+  /* Unless computing the largest size (for memcpy and other raw memory
+     functions), try to determine the size of the object from its type.  */
+  if (!ostype)
+    return NULL_TREE;
+
   if (TREE_CODE (dest) != ADDR_EXPR)
     return NULL_TREE;
 
Index: gcc/testsuite/gcc.dg/Wstringop-overflow.c
===================================================================
--- gcc/testsuite/gcc.dg/Wstringop-overflow.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wstringop-overflow.c	(working copy)
@@ -0,0 +1,132 @@
+/* PR middle-end/77608 - missing protection on trivially detectable runtime
+   buffer overflow
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+#define SIZE_MAX   __SIZE_MAX__
+#define DIFF_MAX   __PTRDIFF_MAX__
+#define DIFF_MIN   (-DIFF_MAX - 1)
+
+typedef __SIZE_TYPE__ size_t;
+
+extern void* memcpy (void*, const void*, size_t);
+extern char* strcpy (char*, const char*);
+extern char* strncpy (char*, const char*, size_t);
+
+void sink (void*);
+
+extern size_t unsigned_value (void)
+{
+  extern volatile size_t unsigned_value_source;
+  return unsigned_value_source;
+}
+
+size_t unsigned_range (size_t min, size_t max)
+{
+  size_t val = unsigned_value ();
+  return val < min || max < val ? min : val;
+}
+
+#define UR(min, max) unsigned_range (min, max)
+
+
+char a7[7];
+
+struct MemArray { char a9[9]; char a1[1]; };
+
+void test_memcpy_array (const void *s)
+{
+#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d))
+
+  T (a7 + UR (0, 1), s, 7);
+  T (a7 + UR (0, 7), s, 7);
+  T (a7 + UR (0, 8), s, 7);
+  T (a7 + UR (0, DIFF_MAX), s, 7);
+  T (a7 + UR (0, SIZE_MAX), s, 7);
+
+  T (a7 + UR (1, 2), s, 7);   /* { dg-warning "writing 7 bytes into a region of size 6" } */
+  T (a7 + UR (2, 3), s, 7);   /* { dg-warning "writing 7 bytes into a region of size 5" } */
+  T (a7 + UR (6, 9), s, 7);   /* { dg-warning "writing 7 bytes into a region of size 1" } */
+  T (a7 + UR (7, 9), s, 7);   /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (8, 9), s, 7);   /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+  T (a7 + UR (9, 10), s, 7);  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 7);  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (DIFF_MAX, SIZE_MAX), s, 7);  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+  /* This is valid.  */
+  char *d = a7 + 7;
+  T (d + UR (-8, -7), s, 7);
+}
+
+/* Verify the absence of warnings for memcpy writing beyond object
+   boundaries. */
+
+void test_memcpy_memarray (struct MemArray *p, const void *s)
+{
+#undef T
+#define T(d, s, n) (memcpy ((d), (s), (n)), sink (d))
+
+  /* The following are valid.  */
+  T (p->a9 + UR (0, 1), s, 9);
+  T (p->a9 + UR (0, 7), s, 9);
+  T (p->a9 + UR (0, 8), s, 9);
+  T (p->a9 + UR (0, DIFF_MAX), s, 9);
+  T (p->a9 + UR (0, SIZE_MAX), s, 9);
+
+  /* The following are invalid.  Unfortunately, there is apparently enough
+     code out there that abuses memcpy to write past the end of one member
+     and into the members that follow so the following are not diagnosed
+     by design.  It sure would be nice not to have to cater to hacks like
+     these...  */
+  T (p->a9 + UR (1, 2), s, 9);
+  T (p->a9 + UR (1, 2), s, 123);
+}
+
+
+void test_strcpy_array (void)
+{
+#undef T
+#define T(d, s) (strcpy ((d), (s)), sink (d))
+
+  T (a7 + UR (0, 1), "012345");
+  T (a7 + UR (0, 7), "012345");
+  T (a7 + UR (0, 8), "012345");
+  T (a7 + UR (0, DIFF_MAX), "012345");
+  T (a7 + UR (0, SIZE_MAX), "012345");
+
+  T (a7 + UR (1, 2), "012345");   /* { dg-warning "writing 7 bytes into a region of size 6" } */
+  T (a7 + UR (2, 3), "012345");   /* { dg-warning "writing 7 bytes into a region of size 5" } */
+  T (a7 + UR (6, 9), "012345");   /* { dg-warning "writing 7 bytes into a region of size 1" } */
+  T (a7 + UR (7, 9), "012345");   /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (8, 9), "012345");   /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+  T (a7 + UR (9, 10), "012345");  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), "012345");  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+  T (a7 + UR (DIFF_MAX, SIZE_MAX), "012345");  /* { dg-warning "writing 7 bytes into a region of size 0" } */
+
+  char *d = a7 + 7;
+
+  T (d + UR (-8, -7), "012345");
+}
+
+void test_strncpy_memarray (struct MemArray *p, const void *s)
+{
+#undef T
+#define T(d, s, n) (strncpy ((d), (s), (n)), sink (d))
+
+  T (p->a9 + UR (0, 1), s, 9);
+  T (p->a9 + UR (0, 7), s, 9);
+  T (p->a9 + UR (0, 8), s, 9);
+  T (p->a9 + UR (0, DIFF_MAX), s, 9);
+  T (p->a9 + UR (0, SIZE_MAX), s, 9);
+
+  T (p->a9 + UR (1, 2), s, 9);    /* { dg-warning "writing 9 bytes into a region of size 8" } */
+  T (p->a9 + UR (2, 3), s, 9);    /* { dg-warning "writing 9 bytes into a region of size 7" } */
+  T (p->a9 + UR (6, 9), s, 9);    /* { dg-warning "writing 9 bytes into a region of size 3" } */
+  T (p->a9 + UR (9, 10), s, 9);   /* { dg-warning "writing 9 bytes into a region of size 0" } */
+  T (p->a9 + UR (10, 11), s, 9);  /* { dg-warning "writing 9 bytes into a region of size 0" } */
+
+  T (p->a9 + UR (DIFF_MAX, DIFF_MAX + (size_t)1), s, 1);  /* { dg-warning "writing 1 byte into a region of size 0" } */
+  T (p->a9 + UR (DIFF_MAX, SIZE_MAX), s, 3);  /* { dg-warning "writing 3 bytes into a region of size 0" } */
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]