diff mbox series

handle a null data.decl consistently in strnlen (PR 87490)

Message ID cf8c914b-42d2-82fc-8a1e-c45f6bf22667@gmail.com
State New
Headers show
Series handle a null data.decl consistently in strnlen (PR 87490) | expand

Commit Message

Martin Sebor Oct. 3, 2018, 5:58 p.m. UTC
The recent strnlen changes to detect reading past unterminated
arrays introduced a couple of bugs:

1) ICE due to assuming that the strnlen argument necessarily
    refers to a known declaration under some conditions.
2) Failing to diagnose uses of unterminated arrays in calls
    with a non-constant bound known to be in excess of the size
    of the array.

The attached patch tested on x86_64-linux fixes both of these
problems.

Martin

Comments

Jeff Law Oct. 4, 2018, 6:09 p.m. UTC | #1
On 10/3/18 11:58 AM, Martin Sebor wrote:
> The recent strnlen changes to detect reading past unterminated
> arrays introduced a couple of bugs:
> 
> 1) ICE due to assuming that the strnlen argument necessarily
>    refers to a known declaration under some conditions.
> 2) Failing to diagnose uses of unterminated arrays in calls
>    with a non-constant bound known to be in excess of the size
>    of the array.
> 
> The attached patch tested on x86_64-linux fixes both of these
> problems.
> 
> Martin
> 
> gcc-87490.diff
> 
> PR tree-optimization/87490 - ICE in expand_builtin_strnlen with a constant argument and non-constant bound
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/87490
> 	* builtins.c (expand_builtin_strnlen): Handle a null data.decl
> 	consistently.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/87490
> 	* gcc.dg/pr87490.c: New test.
> 	* gcc.dg/warn-strnlen-no-nul-2.c: Same.
OK.
jeff
diff mbox series

Patch

PR tree-optimization/87490 - ICE in expand_builtin_strnlen with a constant argument and non-constant bound

gcc/ChangeLog:

	PR tree-optimization/87490
	* builtins.c (expand_builtin_strnlen): Handle a null data.decl
	consistently.

gcc/testsuite/ChangeLog:

	PR tree-optimization/87490
	* gcc.dg/pr87490.c: New test.
	* gcc.dg/warn-strnlen-no-nul-2.c: Same.

Index: gcc/builtins.c
===================================================================
--- gcc/builtins.c	(revision 264800)
+++ gcc/builtins.c	(working copy)
@@ -3151,21 +3151,39 @@  expand_builtin_strnlen (tree exp, rtx target, mach
 		     exp, func, min.to_uhwi (), max.to_uhwi (), maxobjsize))
       TREE_NO_WARNING (exp) = true;
 
+  bool exact = true;
   if (!len || TREE_CODE (len) != INTEGER_CST)
-    return NULL_RTX;
+    {
+      data.decl = unterminated_array (src, &len, &exact);
+      if (!data.decl)
+	return NULL_RTX;
+    }
 
-  if (!TREE_NO_WARNING (exp)
-      && wi::ltu_p (wi::to_wide (len), min)
-      && warning_at (loc, OPT_Wstringop_overflow_,
-		     "%K%qD specified bound [%wu, %wu] "
-		     "exceeds the size %E of unterminated array",
-		     exp, func, min.to_uhwi (), max.to_uhwi (), len))
+  if (data.decl
+      && !TREE_NO_WARNING (exp)
+      && (wi::ltu_p (wi::to_wide (len), min)
+	  || !exact))
     {
-      inform (DECL_SOURCE_LOCATION (data.decl),
-	      "referenced argument declared here");
-      TREE_NO_WARNING (exp) = true;
+      location_t warnloc
+	= expansion_point_location_if_in_system_header (loc);
+
+      if (warning_at (warnloc, OPT_Wstringop_overflow_,
+		      exact
+		      ? G_("%K%qD specified bound [%wu, %wu] exceeds "
+			   "the size %E of unterminated array")
+		      : G_("%K%qD specified bound [%wu, %wu] may exceed "
+			   "the size of at most %E of unterminated array"),
+		      exp, func, min.to_uhwi (), max.to_uhwi (), len))
+	{
+	  inform (DECL_SOURCE_LOCATION (data.decl),
+		  "referenced argument declared here");
+	  TREE_NO_WARNING (exp) = true;
+	}
     }
 
+  if (data.decl)
+    return NULL_RTX;
+
   if (wi::gtu_p (min, wi::to_wide (len)))
     return expand_expr (len, target, target_mode, EXPAND_NORMAL);
 
Index: gcc/testsuite/gcc.dg/pr87490.c
===================================================================
--- gcc/testsuite/gcc.dg/pr87490.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr87490.c	(working copy)
@@ -0,0 +1,13 @@ 
+/* PR tree-optimization/87490 - ICE in expand_builtin_strnlen with a constant
+   argument and non-constant bound
+   { dg-do compile }
+   { dg-options "-O1 -Wall -fno-optimize-strlen" }  */
+
+void test_O1 (int i)
+{
+  int n = (i & 3) | 1;
+
+  /* The ICE here triggers at -O1, with tree-ssa-strlen disabled.  */
+  if (__builtin_strnlen ("", n) != 0)
+    __builtin_abort ();
+}
Index: gcc/testsuite/gcc.dg/warn-strnlen-no-nul-2.c
===================================================================
--- gcc/testsuite/gcc.dg/warn-strnlen-no-nul-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/warn-strnlen-no-nul-2.c	(working copy)
@@ -0,0 +1,66 @@ 
+/* Verify that calls to strnlen with an unterminated array and
+   an excessive non-constant bound are diagnosed.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+#include "range.h"
+
+extern size_t strnlen (const char*, size_t);
+
+const char a[5] = "12345";   /* { dg-message "declared here" } */
+enum { asz = sizeof a };
+
+int v0 = 0;
+int v1 = 1;
+
+void sink (int, ...);
+
+#define CONCAT(a, b)   a ## b
+#define CAT(a, b)      CONCAT(a, b)
+
+#define T(str, n)					\
+  __attribute__ ((noipa))				\
+  void CAT (test_, __LINE__) (void) {			\
+    int i0 = 0, i1 = i0 + 1, i2 = i1 + 1, i3 = i2 + 1;	\
+    sink (strnlen (str, n), i0, i1, i2, i3);		\
+  } typedef void dummy_type
+
+
+T (a, UR (asz, -1));
+T (a, UR (asz - 1, -1));
+T (a, UR (asz - 2, -1));
+T (a, UR (asz - 5, -1));
+T (&a[0], UR (asz, -1));
+T (&a[0] + 1, UR (asz, asz + 1)); /* { dg-warning "specified bound \\\[5, 6] exceeds the size 4 of unterminated array" } */
+T (&a[1], UR (asz, 6));           /* { dg-warning "specified bound \\\[5, 6] exceeds the size 4 of unterminated array" } */
+T (&a[1], UR (asz - 1, 7));
+T (&a[v0], UR (asz, 8));          /* { dg-warning "specified bound \\\[5, 8] may exceed the size of at most 5 of unterminated array" } */
+T (&a[v0] + 1, UR (asz, 9));      /* { dg-warning "specified bound \\\[5, 9] may exceed the size of at most 5 of unterminated array" } */
+
+T (a, UR (asz + 1, asz + 2));     /* { dg-warning "specified bound \\\[6, 7] exceeds the size 5 " } */
+T (&a[0], UR (asz + 1, 10));      /* { dg-warning "unterminated" } */
+T (&a[0] + 1, UR (asz - 1, 11));
+T (&a[0] + 1, UR (asz + 1, 12));  /* { dg-warning "unterminated" } */
+T (&a[1], UR (asz + 1, 13));      /* { dg-warning "unterminated" } */
+T (&a[v0], UR (asz + 1, 14));     /* { dg-warning "unterminated" } */
+T (&a[v0] + 1, UR (asz + 1, 15)); /* { dg-warning "unterminated" } */
+
+T (&a[v0] + 1, UR (DIFF_MAX, SIZE_MAX)); /* { dg-warning "unterminated" } */
+
+T (&a[v0] + 1, UR (DIFF_MAX + (size_t)1, SIZE_MAX)); /* { dg-warning "specified bound \\\[\[0-9\]+, \[0-9\]+] exceeds maximum object size " } */
+
+
+const char c[4] = "1234";
+
+void test (int n0)
+{
+  char a[] = "123";
+
+  if (n0 < 4)
+    n0 = 4;
+  int n1 = __builtin_strlen (a);
+
+  int n = n0 < n1 ? n1 : n0;
+
+  sink (strnlen (c + n, n + 1));    /* { dg-warning "specified bound \\\[5, \[0-9\]+] may exceed the size of at most 4 of unterminated array" } */
+}