The machinery recently added to support -Warray-parameter and -Wvla-parameter also results in enhanced detection of null pointer arguments to VLA function parameters. This enhancement wasn't tested as comprehensively as it should have been and so has some bugs. The attached patch fixes one that leads to an ICE. It also restructures the function and improves the warning issues in this case.
The fix is slightly bit bigger than what I would normally commit without a review but since it's all in code I just wrote and in my view low risk I will go ahead and push it in a few days unless I hear requests for changes by then. Martin
PR middle-end/97188 - ICE passing a null VLA to a function expecting at least one element gcc/ChangeLog: PR middle-end/97188 * calls.c (maybe_warn_rdwr_sizes): Simplify warning messages. Correct handling of VLA argumments. gcc/testsuite/ChangeLog: PR middle-end/97188 * gcc.dg/Wstringop-overflow-23.c: Adjust text of expected warnings. * gcc.dg/Wnonnull-4.c: New test. diff --git a/gcc/calls.c b/gcc/calls.c index 0e5c696c463..ed4363811c8 100644 --- a/gcc/calls.c +++ b/gcc/calls.c @@ -17,6 +17,7 @@ You should have received a copy of the GNU General Public License along with GCC; see the file COPYING3. If not see <http://www.gnu.org/licenses/>. */ +#define INCLUDE_STRING #include "config.h" #include "system.h" #include "coretypes.h" @@ -1924,7 +1925,10 @@ static void maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) { auto_diagnostic_group adg; - bool warned = false; + + /* Set if a warning has been issued for any argument (used to decide + whether to emit an informational note at the end). */ + bool any_warned = false; /* A string describing the attributes that the warnings issued by this function apply to. Used to print one informational note per function @@ -1974,27 +1978,60 @@ maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) else access_size = rwm->get (sizidx)->size; - bool warned = false; + /* Format the value or range to avoid an explosion of messages. */ + char sizstr[80]; + tree sizrng[2] = { size_zero_node, build_all_ones_cst (sizetype) }; + if (get_size_range (access_size, sizrng, true)) + { + const char *s0 = print_generic_expr_to_str (sizrng[0]); + if (tree_int_cst_equal (sizrng[0], sizrng[1])) + { + gcc_checking_assert (strlen (s0) < sizeof sizstr); + strcpy (sizstr, s0); + } + else + { + const char *s1 = print_generic_expr_to_str (sizrng[1]); + gcc_checking_assert (strlen (s0) + strlen (s1) + < sizeof sizstr - 4); + sprintf (sizstr, "[%s, %s]", s0, s1); + } + } + else + *sizstr = '\0'; + + /* Set if a warning has been issued for the current argument. */ + bool arg_warned = false; location_t loc = EXPR_LOCATION (exp); tree ptr = access.second.ptr; - tree sizrng[2] = { size_zero_node, build_all_ones_cst (sizetype) }; - if (get_size_range (access_size, sizrng, true) + if (*sizstr && tree_int_cst_sgn (sizrng[0]) < 0 && tree_int_cst_sgn (sizrng[1]) < 0) { /* Warn about negative sizes. */ - if (tree_int_cst_equal (sizrng[0], sizrng[1])) - warned = warning_at (loc, OPT_Wstringop_overflow_, - "%Kargument %i value %E is negative", - exp, sizidx + 1, access_size); + if (access.second.internal_p) + { + const std::string argtypestr + = access.second.array_as_string (ptrtype); + + arg_warned = warning_at (loc, OPT_Wstringop_overflow_, + "%Kbound argument %i value %s is " + "negative for a variable length array " + "argument %i of type %s", + exp, sizidx + 1, sizstr, + ptridx + 1, argtypestr.c_str ()); + } else - warned = warning_at (loc, OPT_Wstringop_overflow_, - "%Kargument %i range [%E, %E] is negative", - exp, sizidx + 1, sizrng[0], sizrng[1]); - if (warned) + arg_warned = warning_at (loc, OPT_Wstringop_overflow_, + "%Kargument %i value %s is negative", + exp, sizidx + 1, sizstr); + + if (arg_warned) { append_attrname (access, attrstr, sizeof attrstr); - /* Avoid warning again for the same attribute. */ + /* Remember a warning has been issued and avoid warning + again below for the same attribute. */ + any_warned = true; continue; } } @@ -2006,7 +2043,6 @@ maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) /* Multiply ACCESS_SIZE by the size of the type the pointer argument points to. If it's incomplete the size is used as is. */ - access_size = NULL_TREE; if (tree argsize = TYPE_SIZE_UNIT (argtype)) if (TREE_CODE (argsize) == INTEGER_CST) { @@ -2028,35 +2064,44 @@ maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) different from also declaring the pointer argument with attribute nonnull when the function accepts null pointers only when the corresponding size is zero. */ - if (tree_int_cst_equal (sizrng[0], sizrng[1])) - warned = warning_at (loc, OPT_Wnonnull, - "%Kargument %i is null but " - "the corresponding size argument %i " - "value is %E", - exp, ptridx + 1, sizidx + 1, access_size); + if (access.second.internal_p) + { + const std::string argtypestr + = access.second.array_as_string (ptrtype); + + arg_warned = warning_at (loc, OPT_Wnonnull, + "%Kargument %i of variable length " + "array %s is null but " + "the corresponding bound argument " + "%i value is %s", + exp, sizidx + 1, argtypestr.c_str (), + ptridx + 1, sizstr); + } else - warned = warning_at (loc, OPT_Wnonnull, - "%Kargument %i is null but " - "the corresponding size argument %i " - "range is [%E, %E]", - exp, ptridx + 1, sizidx + 1, - sizrng[0], sizrng[1]); + arg_warned = warning_at (loc, OPT_Wnonnull, + "%Kargument %i is null but " + "the corresponding size argument " + "%i value is %s", + exp, ptridx + 1, sizidx + 1, + sizstr); } else if (access_size && access.second.static_p) { /* Warn about null pointers for [static N] array arguments but do not warn for ordinary (i.e., nonstatic) arrays. */ - warned = warning_at (loc, OPT_Wnonnull, - "%Kargument %i to %<%T[static %E]%> null " - "where non-null expected", - exp, ptridx + 1, argtype, - sizrng[0]); + arg_warned = warning_at (loc, OPT_Wnonnull, + "%Kargument %i to %<%T[static %E]%> " + "is null where non-null expected", + exp, ptridx + 1, argtype, + access_size); } - if (warned) + if (arg_warned) { append_attrname (access, attrstr, sizeof attrstr); - /* Avoid warning again for the same attribute. */ + /* Remember a warning has been issued and avoid warning + again below for the same attribute. */ + any_warned = true; continue; } } @@ -2101,7 +2146,7 @@ maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) if (TREE_NO_WARNING (exp)) { - warned = true; + any_warned = true; if (access.second.internal_p) inform (loc, "referencing argument %u of type %qT", @@ -2124,7 +2169,7 @@ maybe_warn_rdwr_sizes (rdwr_map *rwm, tree fndecl, tree fntype, tree exp) "in a call with type %qT and attribute %qs", fntype, attrstr); } - else if (warned) + else if (any_warned) { if (fndecl) inform (DECL_SOURCE_LOCATION (fndecl), diff --git a/gcc/testsuite/gcc.dg/Wnonnull-4.c b/gcc/testsuite/gcc.dg/Wnonnull-4.c new file mode 100644 index 00000000000..180a40d4606 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wnonnull-4.c @@ -0,0 +1,173 @@ +/* PR middle-end/97188 - ICE passing a null VLA to a function expecting + at least one element + { dg-do compile } + { dg-options "-O -Wall -ftrack-macro-expansion=0" } */ + +#define INT_MAX __INT_MAX__ +#define INT_MIN (-INT_MAX - 1) + +/* Exercise passing nul to a one-dimensional VLA argument. */ + +void test_fca_n (int r_m1) +{ + extern void fca_n (int n, char[n]); // { dg-message "in a call to function 'fca_n'" "note" } + +#define T(n) fca_n (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'char\\\[n]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'char\\\[n]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'char\\\[n]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'char\\\[n]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'char\\\[n]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'char\\\[n]' is null but the corresponding bound argument 2 value is \\d+" } +} + + +/* Exercise passing nul to an array with unspecified bound of VLAs. */ + +void test_fsa_x_n (int r_m1) +{ + extern void fsa_x_n (int n, short[][n]); // { dg-message "in a call to function 'fsa_x_n'" "note" } + +#undef T +#define T(n) fsa_x_n (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'short int\\\[]\\\[n]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'short int\\\[]\\\[n]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'short int\\\[]\\\[n]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'short int\\\[]\\\[n]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'short int\\\[]\\\[n]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'short int\\\[]\\\[n]' is null but the corresponding bound argument 2 value is \\d+" } +} + + +/* Exercise passing nul to an array of a single VLA. */ + +void test_fia_1_n (int r_m1) +{ + extern void fia_1_n (int n, int[1][n]); // { dg-message "in a call to function 'fia_1_n'" "note" } + +#undef T +#define T(n) fia_1_n (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'int\\\[1]\\\[n]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'int\\\[1]\\\[n]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'int\\\[1]\\\[n]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'int\\\[1]\\\[n]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'int\\\[1]\\\[n]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'int\\\[1]\\\[n]' is null but the corresponding bound argument 2 value is \\d+" } +} + + +/* Exercise passing nul to an array of three VLAs. */ + +void test_fla_3_n (int r_m1) +{ + extern void fla_3_n (int n, long[3][n]); // { dg-message "in a call to function 'fla_3_n'" "note" } + +#undef T +#define T(n) fla_3_n (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'long int\\\[3]\\\[n]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'long int\\\[3]\\\[n]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'long int\\\[3]\\\[n]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'long int\\\[3]\\\[n]' is null but the corresponding bound argument 2 value is \\d+" } +} + + +/* Exercise passing nul to a VLA of five-element arrays. */ + +void test_fda_n_5 (int r_m1) +{ + extern void fda_n_5 (int n, double[n][5]);// { dg-message "in a call to function 'fda_n_5'" "note" } + +#undef T +#define T(n) fda_n_5 (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'double\\\[n]\\\[5]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'double\\\[n]\\\[5]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'double\\\[n]\\\[5]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'double\\\[n]\\\[5]' is null but the corresponding bound argument 2 value is \\d+" } +} + + +/* Exercise passing nul to a two-dimensional VLA. */ + +void test_fca_n_n (int r_m1) +{ + extern void fca_n_n (int n, char[n][n]); // { dg-message "in a call to function 'fca_n_n'" "note" } + +#undef T +#define T(n) fca_n_n (n, 0) + + int min = INT_MIN; + int max = INT_MAX; + if (r_m1 >= 0) + r_m1 = -1; + + // Verify negative bounds. + T (min); // { dg-warning "bound argument 1 value -\\d+ is negative for a variable length array argument 2 of type 'char\\\[n]\\\[n]'" } + T (r_m1); // { dg-warning "bound argument 1 value \\\[-\\d+, -1] is negative for a variable length array argument 2 of type 'char\\\[n]\\\[n]" } + T ( -1); // { dg-warning "bound argument 1 value -1 is negative for a variable length array argument 2 of type 'char\\\[n]\\\[n]" } + + T ( 0); + + // Verify positive bounds. + T ( 1); // { dg-warning "argument 1 of variable length array 'char\\\[n]\\\[n]' is null but the corresponding bound argument 2 value is 1" } + T ( 9); // { dg-warning "argument 1 of variable length array 'char\\\[n]\\\[n]' is null but the corresponding bound argument 2 value is 9" } + T (max); // { dg-warning "argument 1 of variable length array 'char\\\[n]\\\[n]' is null but the corresponding bound argument 2 value is \\d+" } +} diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-23.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-23.c index bbc12102d14..0da916ad993 100644 --- a/gcc/testsuite/gcc.dg/Wstringop-overflow-23.c +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-23.c @@ -40,7 +40,11 @@ void test_rd2_1 (void) { void *null = 0; - rd2_1 (SR (1, 2), null); // { dg-warning "argument 2 is null but the corresponding size argument 1 range is \\\[1, 2]" } + /* Ideally the message would say "range" for a range and "value" + for a singular value but using the same reduces the complexity + of the code and keeps down the number of messages that need to + be translated, withot sacrificing (too much) clarity. */ + rd2_1 (SR (1, 2), null); // { dg-warning "argument 2 is null but the corresponding size argument 1 range|value is \\\[1, 2]" } } } @@ -59,7 +63,7 @@ void test_wr3_1 (void) void *null = 0; - wr3_1 (SR (1, 2), 1, null); // { dg-warning "argument 3 is null but the corresponding size argument 1 range is \\\[1, 2]" } + wr3_1 (SR (1, 2), 1, null); // { dg-warning "argument 3 is null but the corresponding size argument 1 range|value is \\\[1, 2]" } } @@ -71,7 +75,7 @@ void test_wrd2_1 (int n) wr2_1 (0, 0); wr2_1 (SR (-1, 1), 0); wr2_1 (SR (0, 1), 0); - wr2_1 (SR (1, 2), 0); // { dg-warning "argument 2 is null but the corresponding size argument 1 range is \\\[1, 2]" } + wr2_1 (SR (1, 2), 0); // { dg-warning "argument 2 is null but the corresponding size argument 1 range|value is \\\[1, 2]" } /* This should probably be diagnosed but to avoid false positives caused by jump threading and such it would have to be done @@ -127,7 +131,7 @@ void test_rd1_3_wr2_4 (const void *s, void *d, int n1, int n2) rd1_3_wr2_4 (s, d, -1, 2); // { dg-warning "argument 3 value -1 is negative" } const int ir_min_m1 = SR (INT_MIN, -1); - rd1_3_wr2_4 (s, d, ir_min_m1, 2); // { dg-warning "argument 3 range \\\[-\[0-9\]+, -1] is negative" } + rd1_3_wr2_4 (s, d, ir_min_m1, 2); // { dg-warning "argument 3 range|value \\\[-\[0-9\]+, -1] is negative" } rd1_3_wr2_4 (s, d, SR (-1, 0), 2); rd1_3_wr2_4 (s, d, SR (INT_MIN, INT_MAX), 2);
