| Message ID | 0e941f41-e1b1-1255-8fe7-8a72427c19d9@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gracefully deal with invalid strlen declarations (PR 86114) | expand |
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
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
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" } */