From 2c8c83ac7737c58b5f34a277142b9a1efaf27534 Mon Sep 17 00:00:00 2001 From: Martin Sebor Date: Thu, 15 Dec 2016 04:35:31 +0000 Subject: [PATCH] PR middle-end/78519 - missing warning for sprintf %s with null pointer gcc/ChangeLog: PR middle-end/78519 * gimple-ssa-sprintf.c (format_string): Handle null pointers. (format_directive): Diagnose null pointer arguments. (pass_sprintf_length::handle_gimple_call): Diagnose null destination pointers. Correct location of null format string in diagnostics. gcc/testsuite/ChangeLog: PR middle-end/78519 * gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test. From-SVN: r243684 --- gcc/ChangeLog | 8 ++ gcc/gimple-ssa-sprintf.c | 55 ++++++++++- gcc/testsuite/ChangeLog | 5 + .../gcc.dg/tree-ssa/builtin-sprintf-warn-8.c | 93 +++++++++++++++++++ 4 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-8.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 159dda09e42..35123f0f77c 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2016-12-14 Martin Sebor + + PR middle-end/78519 + * gimple-ssa-sprintf.c (format_string): Handle null pointers. + (format_directive): Diagnose null pointer arguments. + (pass_sprintf_length::handle_gimple_call): Diagnose null destination + pointers. Correct location of null format string in diagnostics. + 2016-12-14 David Malcolm * Makefile.in (SELFTEST_FLAGS): Add path argument to -fself-test. diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 5909e054a61..a91dcb8fc0b 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -439,7 +439,7 @@ struct result_range struct fmtresult { fmtresult () - : argmin (), argmax (), knownrange (), bounded (), constant () + : argmin (), argmax (), knownrange (), bounded (), constant (), nullp () { range.min = range.max = HOST_WIDE_INT_MAX; } @@ -467,6 +467,9 @@ struct fmtresult are also constant (such as determined by constant propagation, though not value range propagation). */ bool constant; + + /* True when the argument is a null pointer. */ + bool nullp; }; /* Description of a conversion specification. */ @@ -1765,6 +1768,16 @@ format_string (const conversion_spec &spec, tree arg) res.range.min = 0; } } + else if (arg && integer_zerop (arg)) + { + /* Handle null pointer argument. */ + + fmtresult res; + res.range.min = 0; + res.range.max = HOST_WIDE_INT_MAX; + res.nullp = true; + return res; + } else { /* For a '%s' and '%ls' directive with a non-constant string, @@ -1910,13 +1923,27 @@ format_directive (const pass_sprintf_length::call_info &info, } } + if (fmtres.nullp) + { + fmtwarn (dirloc, pargrange, NULL, + OPT_Wformat_length_, + "%<%.*s%> directive argument is null", + (int)cvtlen, cvtbeg); + + /* Don't bother processing the rest of the format string. */ + res->warned = true; + res->number_chars = HOST_WIDE_INT_M1U; + res->number_chars_min = res->number_chars_max = res->number_chars; + return; + } + + bool warned = res->warned; + /* Compute the number of available bytes in the destination. There must always be at least one byte of space for the terminating NUL that's appended after the format string has been processed. */ unsigned HOST_WIDE_INT navail = min_bytes_remaining (info.objsize, *res); - bool warned = res->warned; - if (fmtres.range.min < fmtres.range.max) { /* The result is a range (i.e., it's inexact). */ @@ -2871,6 +2898,9 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi) return; } + /* The first argument is a pointer to the destination. */ + tree dstptr = gimple_call_arg (info.callstmt, 0); + info.format = gimple_call_arg (info.callstmt, idx_format); if (idx_dstsize == HOST_WIDE_INT_M1U) @@ -2878,7 +2908,7 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi) /* For non-bounded functions like sprintf, determine the size of the destination from the object or pointer passed to it as the first argument. */ - dstsize = get_destination_size (gimple_call_arg (info.callstmt, 0)); + dstsize = get_destination_size (dstptr); } else if (tree size = gimple_call_arg (info.callstmt, idx_dstsize)) { @@ -2948,6 +2978,20 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi) } else { + /* For calls to non-bounded functions or to those of bounded + functions with a non-zero size, warn if the destination + pointer is null. */ + if (integer_zerop (dstptr)) + { + /* This is diagnosed with -Wformat only when the null is a constant + pointer. The warning here diagnoses instances where the pointer + is not constant. */ + location_t loc = gimple_location (info.callstmt); + warning_at (EXPR_LOC_OR_LOC (dstptr, loc), + OPT_Wformat_length_, "null destination pointer"); + return; + } + /* Set the object size to the smaller of the two arguments of both have been specified and they're not equal. */ info.objsize = dstsize < objsize ? dstsize : objsize; @@ -2971,7 +3015,8 @@ pass_sprintf_length::handle_gimple_call (gimple_stmt_iterator *gsi) /* This is diagnosed with -Wformat only when the null is a constant pointer. The warning here diagnoses instances where the pointer is not constant. */ - warning_at (EXPR_LOC_OR_LOC (info.format, input_location), + location_t loc = gimple_location (info.callstmt); + warning_at (EXPR_LOC_OR_LOC (info.format, loc), OPT_Wformat_length_, "null format string"); return; } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 88665a2c6a8..f23ccd6ba24 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2016-12-14 Martin Sebor + + PR middle-end/78519 + * gcc.dg/tree-ssa/builtin-sprintf-warn-7.c: New test. + 2016-12-14 Martin Sebor PR c++/78774 diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-8.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-8.c new file mode 100644 index 00000000000..2550065549d --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-8.c @@ -0,0 +1,93 @@ +/* PR middle-end/78519 - missing warning for sprintf %s with null pointer + Also exercises null destination pointer and null format string. + { dg-do compile } + { dg-options "-O2 -Wformat -Wformat-length -Wno-nonnull -ftrack-macro-expansion=0" } */ + +typedef __builtin_va_list va_list; + +#define sprintf __builtin_sprintf +#define snprintf __builtin_snprintf +#define vsprintf __builtin_vsprintf +#define vsnprintf __builtin_vsnprintf + + +static char* null (void) +{ + return 0; +} + + +void sink (int); +#define T sink + + +/* Verify that calls with a null destination pointer are diagnosed. */ + +void test_null_dest (va_list va) +{ + char *p = null (); + T (sprintf (null (), "%i", 0)); /* { dg-warning "null destination pointer" } */ + T (sprintf (p, "%i", 0)); /* { dg-warning "null destination pointer" } */ + T (sprintf (p, "%i abc", 0)); /* { dg-warning "null destination pointer" } */ + + T (snprintf (null (), 1, "%i", 0)); /* { dg-warning "null destination pointer" } */ + T (snprintf (p, 2, "%i", 0)); /* { dg-warning "null destination pointer" } */ + T (snprintf (p, 3, "%i abc", 0)); /* { dg-warning "null destination pointer" } */ + + /* Snprintf with a null pointer and a zero size is a special request + to determine the size of output without writing any. Such calls + are valid must not be diagnosed. */ + T (snprintf (p, 0, "%i", 0)); + + T (vsprintf (null (), "%i", va)); /* { dg-warning "null destination pointer" } */ + T (vsprintf (p, "%i", va)); /* { dg-warning "null destination pointer" } */ + + T (vsnprintf (null (), 1, "%i", va)); /* { dg-warning "null destination pointer" } */ + T (vsnprintf (p, 2, "%i", va)); /* { dg-warning "null destination pointer" } */ + T (vsnprintf (p, 0, "%i", va)); +} + +void test_null_format (char *d, va_list va) +{ + char *fmt = null (); + + T (sprintf (d, null ())); /* { dg-warning "null format string" } */ + T (sprintf (d, fmt)); /* { dg-warning "null format string" } */ + + T (snprintf (d, 0, null ())); /* { dg-warning "null format string" } */ + T (snprintf (d, 1, fmt)); /* { dg-warning "null format string" } */ + + T (vsprintf (d, null (), va)); /* { dg-warning "null format string" } */ + T (vsprintf (d, fmt, va)); /* { dg-warning "null format string" } */ + + T (vsnprintf (d, 0, null (), va)); /* { dg-warning "null format string" } */ + T (vsnprintf (d, 1, fmt, va)); /* { dg-warning "null format string" } */ +} + +void test_null_arg (char *d, const char *s) +{ + char *p = null (); + + T (sprintf (d, "%-s", null ())); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%-s", p)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%s %s", p, s)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%s %s", s, p)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%s %i", p, 1)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%i %s", 1, p)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%.0s", p)); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%1.0s", p)); /* { dg-warning "directive argument is null" } */ + + T (snprintf (d, 0, "%-s", null ())); /* { dg-warning "directive argument is null" } */ + T (snprintf (d, 1, "%-s", p)); /* { dg-warning "directive argument is null" } */ + + T (sprintf (d, "%i %s", 1, null ())); /* { dg-warning "directive argument is null" } */ + T (sprintf (d, "%i %s", 2, p)); /* { dg-warning "directive argument is null" } */ + + T (snprintf (d, 0, "%i %s", 1, null ())); /* { dg-warning "directive argument is null" } */ + T (snprintf (d, 9, "%i %s", 2, p)); /* { dg-warning "directive argument is null" } */ + + /* A sanity check that the %p directive doesn't emit a warning + with a null pointer. */ + T (sprintf (d, "%p", null ())); + T (sprintf (d, "%p", p)); +} -- 2.30.2