gracefully deal with invalid strlen declarations (PR 86114)

Message ID 0e941f41-e1b1-1255-8fe7-8a72427c19d9@gmail.com
State New
Headers show
Series
  • gracefully deal with invalid strlen declarations (PR 86114)
Related show

Commit Message

Martin Sebor June 12, 2018, 9:29 p.m.
Declaring strlen() to return a pointer instead of size_t
and then calling the function can result in an ICE due to
both gimple-fold and tree-ssa-strlen assuming the function
necessarily returns an integer.

As luck would have it, the incompatible declaration isn't
detected by -Wbuiltin-declaration-mismatch (bug 86125), nor
apparently even by gimple_builtin_call_types_compatible_p(),
and so the invalid declaration makes its way where it isn't
expected.

The attached patch avoids the ICE by removing the unsafe
assumption from both the folder and the strlen pass.

Martin

Comments

Jeff Law June 12, 2018, 9:37 p.m. | #1
On 06/12/2018 03:29 PM, Martin Sebor wrote:
> Declaring strlen() to return a pointer instead of size_t
> and then calling the function can result in an ICE due to
> both gimple-fold and tree-ssa-strlen assuming the function
> necessarily returns an integer.
> 
> As luck would have it, the incompatible declaration isn't
> detected by -Wbuiltin-declaration-mismatch (bug 86125), nor
> apparently even by gimple_builtin_call_types_compatible_p(),
> and so the invalid declaration makes its way where it isn't
> expected.
> 
> The attached patch avoids the ICE by removing the unsafe
> assumption from both the folder and the strlen pass.
> 
> Martin
> 
> gcc-86114.diff
> 
> 
> PR tree-optimization/86114 - ICE in gimple_fold_builtin_strlen with an invalid call to strnlen
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/86114
> 	* gcc.dg/pr86114.c: New test.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/86114
> 	* gimple-fold.c (gimple_fold_builtin_strlen): Only handle LHS
> 	of integer types.
> 	* tree-ssa-strlen.c (maybe_set_strlen_range): Same.
OK.  However ISTM that catching this earlier would be advisable.

jeff
Martin Sebor June 18, 2018, 10:27 p.m. | #2
On 06/12/2018 03:37 PM, Jeff Law wrote:
> On 06/12/2018 03:29 PM, Martin Sebor wrote:
>> Declaring strlen() to return a pointer instead of size_t
>> and then calling the function can result in an ICE due to
>> both gimple-fold and tree-ssa-strlen assuming the function
>> necessarily returns an integer.
>>
>> As luck would have it, the incompatible declaration isn't
>> detected by -Wbuiltin-declaration-mismatch (bug 86125), nor
>> apparently even by gimple_builtin_call_types_compatible_p(),
>> and so the invalid declaration makes its way where it isn't
>> expected.
>>
>> The attached patch avoids the ICE by removing the unsafe
>> assumption from both the folder and the strlen pass.
>>
>> Martin
>>
>> gcc-86114.diff
>>
>>
>> PR tree-optimization/86114 - ICE in gimple_fold_builtin_strlen with an invalid call to strnlen
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR tree-optimization/86114
>> 	* gcc.dg/pr86114.c: New test.
>>
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/86114
>> 	* gimple-fold.c (gimple_fold_builtin_strlen): Only handle LHS
>> 	of integer types.
>> 	* tree-ssa-strlen.c (maybe_set_strlen_range): Same.
> OK.  However ISTM that catching this earlier would be advisable.

Agreed.  Discarding invalid built-in declarations (in addition
to issuing a warning) would have prevented this bug as well as
bug 86202.  I'm working on a patch to at least warn on these
bad declarations if not drop them altogether (as GCC already
does some others).  This is bug 86125.

Martin

Patch

PR tree-optimization/86114 - ICE in gimple_fold_builtin_strlen with an invalid call to strnlen

gcc/testsuite/ChangeLog:

	PR tree-optimization/86114
	* gcc.dg/pr86114.c: New test.

gcc/ChangeLog:

	PR tree-optimization/86114
	* gimple-fold.c (gimple_fold_builtin_strlen): Only handle LHS
	of integer types.
	* tree-ssa-strlen.c (maybe_set_strlen_range): Same.

Index: gcc/gimple-fold.c
===================================================================
--- gcc/gimple-fold.c	(revision 261516)
+++ gcc/gimple-fold.c	(working copy)
@@ -3538,9 +3538,10 @@  gimple_fold_builtin_strlen (gimple_stmt_iterator *
       return true;
     }
 
-  tree lhs = gimple_call_lhs (stmt);
-  if (lhs && TREE_CODE (lhs) == SSA_NAME)
-    set_range_info (lhs, VR_RANGE, minlen, maxlen);
+  if (tree lhs = gimple_call_lhs (stmt))
+    if (TREE_CODE (lhs) == SSA_NAME
+	&& INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
+      set_range_info (lhs, VR_RANGE, minlen, maxlen);
 
   return false;
 }
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 261518)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -1124,14 +1124,15 @@  adjust_last_stmt (strinfo *si, gimple *stmt, bool
   update_stmt (last.stmt);
 }
 
-/* For an LHS that is an SSA_NAME and for strlen() argument SRC, set
-   LHS range info to [0, N] if SRC refers to a character array A[N]
-   with unknown length bounded by N.  */
+/* For an LHS that is an SSA_NAME with integer type and for strlen()
+   argument SRC, set LHS range info to [0, N] if SRC refers to
+   a character array A[N] with unknown length bounded by N.  */
 
 static void
 maybe_set_strlen_range (tree lhs, tree src)
 {
-  if (TREE_CODE (lhs) != SSA_NAME)
+  if (TREE_CODE (lhs) != SSA_NAME
+      || !INTEGRAL_TYPE_P (TREE_TYPE (lhs)))
     return;
 
   if (TREE_CODE (src) == SSA_NAME)
Index: gcc/testsuite/gcc.dg/pr86114.c
===================================================================
--- gcc/testsuite/gcc.dg/pr86114.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/pr86114.c	(working copy)
@@ -0,0 +1,42 @@ 
+/* PR tree-optimization/86114 - ICE in gimple_fold_builtin_strlen with
+   an invalid call to strnlen
+   { dg-do compile }
+   { dg-options "-O2" }  */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern char* strcmp (const char*, const char*);
+extern char* strncmp (const char*, const char*, size_t);
+extern char* strlen (const char*);
+extern char* strnlen (const char*, size_t);
+extern char* strcspn (const char*, const char*);
+extern char* strspn (const char*, const char*);
+extern char* strxfrm (const char*, const char*, size_t);
+
+char** q;
+
+void test_array (const char *s)
+{
+  extern char a[8];
+
+  q[0] = strcmp (a, s);
+  q[1] = strncmp (a, s, 7);
+  q[2] = strlen (a);
+  q[3] = strnlen (a, 7);
+  q[4] = strcspn (a, s);
+  q[5] = strspn (a, s);
+  q[6] = strxfrm (a, s, 7);
+}
+
+void test_pointer (const char *s, const char *t)
+{
+  q[0] = strcmp (s, t);
+  q[1] = strncmp (s, t, 7);
+  q[2] = strlen (s);
+  q[3] = strnlen (s, 7);
+  q[4] = strcspn (s, t);
+  q[5] = strspn (s, t);
+  q[6] = strxfrm (s, s, 7);
+}
+
+/* { dg-prune-output "-Wbuiltin-declaration-mismatch" } */