The attached changes implement the detection of past-the-end reads by strcpy due to unterminated arguments.
PR tree-optimization/86552 - missing warning for reading past the end of non-string arrays
gcc/ChangeLog: * builtins.c (unterminated_array): New. (expand_builtin_strcpy): Adjust. (expand_builtin_strcpy_args): Detect unterminated arrays. * gimple-fold.c (get_maxval_strlen): Add argument. Detect unterminated arrays. * gimple-fold.h (get_maxval_strlen): Add argument. (gimple_fold_builtin_strcpy): Detec unterminated arrays. gcc/testsuite/ChangeLog: * gcc.dg/warn-strcpy-no-nul.c: New test. diff --git a/gcc/builtins.c b/gcc/builtins.c index 78ced93..a77f25c 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -132,7 +132,7 @@ static rtx expand_builtin_mempcpy (tree, rtx); static rtx expand_builtin_mempcpy_args (tree, tree, tree, rtx, tree, int); static rtx expand_builtin_strcat (tree, rtx); static rtx expand_builtin_strcpy (tree, rtx); -static rtx expand_builtin_strcpy_args (tree, tree, rtx); +static rtx expand_builtin_strcpy_args (tree, tree, tree, rtx); static rtx expand_builtin_stpcpy (tree, rtx, machine_mode); static rtx expand_builtin_stpncpy (tree, rtx); static rtx expand_builtin_strncat (tree, rtx); @@ -580,6 +580,34 @@ warn_string_no_nul (location_t loc, tree exp, tree fndecl, tree nonstr) inform (DECL_SOURCE_LOCATION (nonstr), "referenced argument declared here"); } +/* If EXP refers to an unterminated constant character array return + the declaration of the object of which the array is a member or + element. Otherwise return null. */ + +static tree +unterminated_array (tree exp) +{ + if (TREE_CODE (exp) == SSA_NAME) + { + gimple *stmt = SSA_NAME_DEF_STMT (exp); + if (!is_gimple_assign (stmt)) + return NULL_TREE; + + tree rhs1 = gimple_assign_rhs1 (stmt); + tree_code code = gimple_assign_rhs_code (stmt); + if (code != POINTER_PLUS_EXPR) + return NULL_TREE; + + exp = rhs1; + } + + tree nonstr; + if (c_strlen (exp, 1, &nonstr) && nonstr) + return nonstr; + + return NULL_TREE; +} + /* Compute the length of a null-terminated character string or wide character string handling character sizes of 1, 2, and 4 bytes. TREE_STRING_LENGTH is not the right way because it evaluates to @@ -3902,7 +3930,7 @@ expand_builtin_strcpy (tree exp, rtx target) src, destsize); } - if (rtx ret = expand_builtin_strcpy_args (dest, src, target)) + if (rtx ret = expand_builtin_strcpy_args (exp, dest, src, target)) { /* Check to see if the argument was declared attribute nonstring and if so, issue a warning since at this point it's not known @@ -3922,8 +3950,17 @@ expand_builtin_strcpy (tree exp, rtx target) expand_builtin_strcpy. */ static rtx -expand_builtin_strcpy_args (tree dest, tree src, rtx target) +expand_builtin_strcpy_args (tree exp, tree dest, tree src, rtx target) { + /* Detect strcpy calls with unterminated arrays.. */ + if (tree nonstr = unterminated_array (src)) + { + /* NONSTR refers to the non-nul terminated constant array. */ + if (!TREE_NO_WARNING (exp)) + warn_string_no_nul (EXPR_LOCATION (exp), exp, NULL_TREE, nonstr); + return NULL_RTX; + } + return expand_movstr (dest, src, target, /*endp=*/0); } @@ -3983,7 +4020,7 @@ expand_builtin_stpcpy_1 (tree exp, rtx target, machine_mode mode) if (CONST_INT_P (len_rtx)) { - ret = expand_builtin_strcpy_args (dst, src, target); + ret = expand_builtin_strcpy_args (exp, dst, src, target); if (ret) { diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index 5c88e33..3fb8d85 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -1591,21 +1591,30 @@ get_range_strlen (tree arg, tree minmaxlen[2], bool strict /* = false */, } tree -get_maxval_strlen (tree arg, int type) +get_maxval_strlen (tree arg, int type, tree *nonstr /* = NULL */) { bitmap visited = NULL; tree len[2] = { NULL_TREE, NULL_TREE }; bool dummy; /* Set to non-null if ARG refers to an unterminated array. */ - tree nonstr = NULL_TREE; - if (!get_range_strlen (arg, len, &visited, type, 0, &dummy, &nonstr)) + tree mynonstr = NULL_TREE; + if (!get_range_strlen (arg, len, &visited, type, 0, &dummy, &mynonstr)) len[1] = NULL_TREE; if (visited) BITMAP_FREE (visited); + if (nonstr) + { + /* For callers prepared to handle unterminated arrays set + *NONSTR to point to the declaration of the array and return + the maximum length/size. */ + *nonstr = mynonstr; + return len[1]; + } + /* Fail if the constant array isn't nul-terminated. */ - return nonstr ? NULL_TREE : len[1]; + return mynonstr ? NULL_TREE : len[1]; } @@ -1648,10 +1657,21 @@ gimple_fold_builtin_strcpy (gimple_stmt_iterator *gsi, if (!fn) return false; - tree len = get_maxval_strlen (src, 0); + /* Set to non-null if ARG refers to an unterminated array. */ + tree nonstr; + tree len = get_maxval_strlen (src, 0, &nonstr); if (!len) return false; + if (nonstr) + { + /* Avoid folding calls with unterminated arrays. */ + if (!gimple_no_warning_p (stmt)) + warn_string_no_nul (loc, NULL_TREE, gimple_call_fndecl (stmt), nonstr); + gimple_set_no_warning (stmt, true); + return false; + } + len = fold_convert_loc (loc, size_type_node, len); len = size_binop_loc (loc, PLUS_EXPR, len, build_int_cst (size_type_node, 1)); len = force_gimple_operand_gsi (gsi, len, true, diff --git a/gcc/gimple-fold.h b/gcc/gimple-fold.h index 9bfc468..9ffe75bf 100644 --- a/gcc/gimple-fold.h +++ b/gcc/gimple-fold.h @@ -26,7 +26,7 @@ extern tree create_tmp_reg_or_ssa_name (tree, gimple *stmt = NULL); extern tree canonicalize_constructor_val (tree, tree); extern tree get_symbol_constant_value (tree); extern bool get_range_strlen (tree, tree[2], bool = false, tree * = NULL); -extern tree get_maxval_strlen (tree, int); +extern tree get_maxval_strlen (tree, int, tree * = NULL); extern void gimplify_and_update_call_from_tree (gimple_stmt_iterator *, tree); extern bool fold_stmt (gimple_stmt_iterator *); extern bool fold_stmt (gimple_stmt_iterator *, tree (*) (tree)); diff --git a/gcc/testsuite/gcc.dg/warn-strcpy-no-nul.c b/gcc/testsuite/gcc.dg/warn-strcpy-no-nul.c new file mode 100644 index 0000000..b06ec52 --- /dev/null +++ b/gcc/testsuite/gcc.dg/warn-strcpy-no-nul.c @@ -0,0 +1,324 @@ +/* PR tree-optimization/86552 - missing warning for reading past the end + of non-string arrays + { dg-do compile } + { dg-options "-O2 -Wall -Wno-array-bounds -ftrack-macro-expansion=0" } */ + +extern char* strcpy (char*, const char*); + +const char a[5] = "12345"; /* { dg-message "declared here" } */ + +int v0 = 0; +int v1 = 1; +int v2 = 1; +int v3 = 1; + +void sink (char*, ...); + +#define T(str) sink (strcpy (d, str)) + +void test_one_dim_array (char *d) +{ + T (a); /* { dg-warning "argument missing terminating nul" } */ + T (&a[0]); /* { dg-warning "nul" } */ + T (&a[0] + 1); /* { dg-warning "nul" } */ + T (&a[1]); /* { dg-warning "nul" } */ + + int i0 = 0; + int i1 = i0 + 1; + + T (&a[i0]); /* { dg-warning "nul" } */ + T (&a[i0] + 1); /* { dg-warning "nul" } */ + T (&a[i1]); /* { dg-warning "nul" } */ + + T (&a[v0]); /* { dg-warning "nul" } */ + T (&a[v0] + 1); /* { dg-warning "nul" } */ + T (&a[v0] + v1); /* { dg-warning "nul" } */ +} + +const char b[][5] = { /* { dg-message "declared here" } */ + "12", "123", "1234", "54321" +}; + +void test_two_dim_array (char *d) +{ + int i0 = 0; + int i1 = i0 + 1; + int i2 = i1 + 1; + int i3 = i2 + 1; + + T (b[0]); + T (b[1]); + T (b[2]); + T (b[3]); /* { dg-warning "nul" } */ + T (b[i0]); + T (b[i1]); + T (b[i2]); + T (b[i3]); /* { dg-warning "nul" } */ + T (b[v0]); + T (b[v3]); + + T (&b[2][1]); + T (&b[2][1] + 1); + T (&b[2][v0]); + T (&b[2][1] + v0); + + T (&b[i2][i1]); + T (&b[i2][i1] + i1); + T (&b[i2][v0]); + T (&b[i2][i1] + v0); + + T (&b[3][1]); /* { dg-warning "nul" } */ + T (&b[3][1] + 1); /* { dg-warning "nul" } */ + T (&b[3][v0]); /* { dg-warning "nul" } */ + T (&b[3][1] + v0); /* { dg-warning "nul" } */ + T (&b[3][v0] + v1); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (&b[i3][i1]); /* { dg-warning "nul" } */ + T (&b[i3][i1] + i1); /* { dg-warning "nul" } */ + T (&b[i3][v0]); /* { dg-warning "nul" } */ + T (&b[i3][i1] + v0); /* { dg-warning "nul" } */ + T (&b[i3][v0] + v1); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? "" : b[0]); + T (v0 ? "" : b[1]); + T (v0 ? "" : b[2]); + T (v0 ? "" : b[3]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? b[0] : ""); + T (v0 ? b[1] : ""); + T (v0 ? b[2] : ""); + T (v0 ? b[3] : ""); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? "1234" : b[3]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? b[3] : "1234"); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? a : b[3]); /* { dg-warning "nul" } */ + T (v0 ? b[0] : b[2]); + T (v0 ? b[2] : b[3]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? b[3] : b[2]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? b[0] : &b[3][0] + 1); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? b[1] : &b[3][1] + v0); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + /* It's possible to detect the missing nul in the following two + expressions but GCC doesn't do it yet. */ + T (v0 ? &b[3][1] + v0 : b[2]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? &b[3][v0] : &b[3][v1]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ +} + +struct A { char a[5], b[5]; }; + +const struct A s = { "1234", "12345" }; + +void test_struct_member (char *d) +{ + int i0 = 0; + int i1 = i0 + 1; + + T (s.a); + T (&s.a[0]); + T (&s.a[0] + 1); + T (&s.a[0] + i0); + T (&s.a[1]); + T (&s.a[1] + 1); + T (&s.a[1] + i0); + + T (&s.a[i0]); + T (&s.a[i0] + 1); + T (&s.a[i0] + v0); + T (&s.a[i1]); + T (&s.a[i1] + 1); + T (&s.a[i1] + v0); + + T (s.a); + T (&s.a[0]); + T (&s.a[0] + 1); + T (&s.a[0] + v0); + T (&s.a[1]); + T (&s.a[1] + 1); + T (&s.a[1] + v0); + + T (&s.a[i0]); + T (&s.a[i0] + 1); + T (&s.a[i0] + v0); + T (&s.a[i1]); + T (&s.a[i1] + 1); + T (&s.a[i1] + v0); + + T (&s.a[v0]); + T (&s.a[v0] + 1); + T (&s.a[v0] + v0); + T (&s.a[v1]); + T (&s.a[v1] + 1); + T (&s.a[v1] + v0); + + T (s.b); /* { dg-warning "nul" } */ + T (&s.b[0]); /* { dg-warning "nul" } */ + T (&s.b[0] + 1); /* { dg-warning "nul" } */ + T (&s.b[0] + i0); /* { dg-warning "nul" } */ + T (&s.b[1]); /* { dg-warning "nul" } */ + T (&s.b[1] + 1); /* { dg-warning "nul" } */ + T (&s.b[1] + i0); /* { dg-warning "nul" } */ + + T (s.b); /* { dg-warning "nul" } */ + T (&s.b[0]); /* { dg-warning "nul" } */ + T (&s.b[0] + 1); /* { dg-warning "nul" } */ + T (&s.b[0] + v0); /* { dg-warning "nul" } */ + T (&s.b[1]); /* { dg-warning "nul" } */ + T (&s.b[1] + 1); /* { dg-warning "nul" } */ + T (&s.b[1] + v0); /* { dg-warning "nul" } */ + + T (s.b); /* { dg-warning "nul" } */ + T (&s.b[v0]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (&s.b[v0] + 1); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (&s.b[v0] + v0); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (&s.b[v1]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (&s.b[v1] + 1); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (&s.b[v1] + v0); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ +} + +struct B { struct A a[2]; }; + +const struct B ba[] = { + { { { "123", "12345" }, { "12345", "123" } } }, + { { { "12345", "123" }, { "123", "12345" } } }, + { { { "1", "12" }, { "123", "1234" } } }, + { { { "123", "1234" }, { "12345", "12" } } } +}; + +void test_array_of_structs (char *d) +{ + T (ba[0].a[0].a); + T (&ba[0].a[0].a[0]); + T (&ba[0].a[0].a[0] + 1); + T (&ba[0].a[0].a[0] + v0); + T (&ba[0].a[0].a[1]); + T (&ba[0].a[0].a[1] + 1); + T (&ba[0].a[0].a[1] + v0); + + T (ba[0].a[0].b); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[0]); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[0] + 1); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[0] + v0); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[1]); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[1] + 1); /* { dg-warning "nul" } */ + T (&ba[0].a[0].b[1] + v0); /* { dg-warning "nul" } */ + + T (ba[0].a[1].a); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[0]); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[0] + 1); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[0] + v0); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[1]); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[1] + 1); /* { dg-warning "nul" } */ + T (&ba[0].a[1].a[1] + v0); /* { dg-warning "nul" } */ + + T (ba[0].a[1].b); + T (&ba[0].a[1].b[0]); + T (&ba[0].a[1].b[0] + 1); + T (&ba[0].a[1].b[0] + v0); + T (&ba[0].a[1].b[1]); + T (&ba[0].a[1].b[1] + 1); + T (&ba[0].a[1].b[1] + v0); + + + T (ba[1].a[0].a); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[0]); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[0] + 1); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[0] + v0); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[1]); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[1] + 1); /* { dg-warning "nul" } */ + T (&ba[1].a[0].a[1] + v0); /* { dg-warning "nul" } */ + + T (ba[1].a[0].b); + T (&ba[1].a[0].b[0]); + T (&ba[1].a[0].b[0] + 1); + T (&ba[1].a[0].b[0] + v0); + T (&ba[1].a[0].b[1]); + T (&ba[1].a[0].b[1] + 1); + T (&ba[1].a[0].b[1] + v0); + + T (ba[1].a[1].a); + T (&ba[1].a[1].a[0]); + T (&ba[1].a[1].a[0] + 1); + T (&ba[1].a[1].a[0] + v0); + T (&ba[1].a[1].a[1]); + T (&ba[1].a[1].a[1] + 1); + T (&ba[1].a[1].a[1] + v0); + + T (ba[1].a[1].b); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[0]); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[0] + 1); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[0] + v0); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[1]); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[1] + 1); /* { dg-warning "nul" } */ + T (&ba[1].a[1].b[1] + v0); /* { dg-warning "nul" } */ + + + T (ba[2].a[0].a); + T (&ba[2].a[0].a[0]); + T (&ba[2].a[0].a[0] + 1); + T (&ba[2].a[0].a[0] + v0); + T (&ba[2].a[0].a[1]); + T (&ba[2].a[0].a[1] + 1); + T (&ba[2].a[0].a[1] + v0); + + T (ba[2].a[0].b); + T (&ba[2].a[0].b[0]); + T (&ba[2].a[0].b[0] + 1); + T (&ba[2].a[0].b[0] + v0); + T (&ba[2].a[0].b[1]); + T (&ba[2].a[0].b[1] + 1); + T (&ba[2].a[0].b[1] + v0); + + T (ba[2].a[1].a); + T (&ba[2].a[1].a[0]); + T (&ba[2].a[1].a[0] + 1); + T (&ba[2].a[1].a[0] + v0); + T (&ba[2].a[1].a[1]); + T (&ba[2].a[1].a[1] + 1); + T (&ba[2].a[1].a[1] + v0); + + + T (ba[3].a[0].a); + T (&ba[3].a[0].a[0]); + T (&ba[3].a[0].a[0] + 1); + T (&ba[3].a[0].a[0] + v0); + T (&ba[3].a[0].a[1]); + T (&ba[3].a[0].a[1] + 1); + T (&ba[3].a[0].a[1] + v0); + + T (ba[3].a[0].b); + T (&ba[3].a[0].b[0]); + T (&ba[3].a[0].b[0] + 1); + T (&ba[3].a[0].b[0] + v0); + T (&ba[3].a[0].b[1]); + T (&ba[3].a[0].b[1] + 1); + T (&ba[3].a[0].b[1] + v0); + + T (ba[3].a[1].a); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[0]); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[0] + 1); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[0] + v0); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[1]); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[1] + 1); /* { dg-warning "nul" } */ + T (&ba[3].a[1].a[1] + v0); /* { dg-warning "nul" } */ + + T (ba[3].a[1].b); + T (&ba[3].a[1].b[0]); + T (&ba[3].a[1].b[0] + 1); + T (&ba[3].a[1].b[0] + v0); + T (&ba[3].a[1].b[1]); + T (&ba[3].a[1].b[1] + 1); + T (&ba[3].a[1].b[1] + v0); + + + T (v0 ? ba[0].a[0].a : ba[0].a[0].b); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? ba[0].a[0].a : ba[0].a[0].b); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? &ba[0].a[0].a[0] : &ba[3].a[1].a[0]); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + T (v0 ? &ba[3].a[1].a[1] : ba[0].a[0].a); /* { dg-warning "nul" "bug ???" { xfail *-*-* } } */ + + T (v0 ? ba[0].a[0].a : ba[0].a[1].b); + T (v0 ? ba[0].a[1].b : ba[0].a[0].a); +} + +/* { dg-prune-output " reading \[1-9\]\[0-9\]? bytes from a region " } */