From e52ed3fee2d607473dc50d48e5f05c30b42f1966 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 8 Aug 2016 20:10:19 +0000 Subject: [PATCH] Use class substring_loc in c-format.c (PR c/52952) gcc/c-family/ChangeLog: PR c/52952 * c-format.c: Include "diagnostic.h". (location_column_from_byte_offset): Delete. (location_from_offset): Delete. (format_warning_va): New function. (format_warning_at_substring): New function. (format_warning_at_char): New function. (check_format_arg): Capture location of format_tree and pass to check_format_info_main. (argument_parser): Add fields "start_of_this_format" and "format_string_cst". (flag_chars_t::validate): Add param "format_string_cst". Convert warning_at call using location_from_offset to call to format_warning_at_char. (argument_parser::argument_parser): Add param "format_string_cst_" and use use it to initialize field "format_string_cst". Initialize new field "start_of_this_format". (argument_parser::read_format_flags): Convert warning_at call using location_from_offset to a call to format_warning_at_char. (argument_parser::read_any_format_left_precision): Likewise. (argument_parser::read_any_format_precision): Likewise. (argument_parser::read_any_other_modifier): Likewise. (argument_parser::find_format_char_info): Likewise, in three places. (argument_parser::parse_any_scan_set): Likewise, in one place. (argument_parser::handle_conversions): Likewise, in two places. (argument_parser::check_argument_type): Add param "fmt_param_loc" and use it to make a substring_loc. Pass the latter to check_format_types. (check_format_info_main): Add params "fmt_param_loc" and "format_string_cst". Convert warning_at calls using location_from_offset to calls to format_warning_at_char. Pass the new params to the arg_parser ctor. Pass "format_string_cst" to flag_chars.validate. Pass "fmt_param_loc" to arg_parser.check_argument_type. (check_format_types): Convert first param from a location_t to a const substring_loc & and rename to "fmt_loc". Attempt to extract the range of the relevant parameter and pass it to format_type_warning. (format_type_warning): Convert first param from a location_t to a const substring_loc & and rename to "fmt_loc". Add params "param_range" and "type". Replace calls to warning_at with calls to format_warning_at_substring. gcc/testsuite/ChangeLog: PR c/52952 * gcc.dg/cpp/pr66415-1.c: Likewise. * gcc.dg/format/asm_fprintf-1.c: Update column numbers. * gcc.dg/format/c90-printf-1.c: Likewise. * gcc.dg/format/diagnostic-ranges.c: New test case. From-SVN: r239253 --- gcc/c-family/ChangeLog | 45 ++ gcc/c-family/c-format.c | 487 +++++++++++------- gcc/testsuite/ChangeLog | 8 + gcc/testsuite/gcc.dg/cpp/pr66415-1.c | 8 +- gcc/testsuite/gcc.dg/format/asm_fprintf-1.c | 6 +- gcc/testsuite/gcc.dg/format/c90-printf-1.c | 14 +- .../gcc.dg/format/diagnostic-ranges.c | 222 ++++++++ 7 files changed, 605 insertions(+), 185 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/format/diagnostic-ranges.c diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 4374dbfb1b4..f03a078b438 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,48 @@ +2016-08-08 David Malcolm + + PR c/52952 + * c-format.c: Include "diagnostic.h". + (location_column_from_byte_offset): Delete. + (location_from_offset): Delete. + (format_warning_va): New function. + (format_warning_at_substring): New function. + (format_warning_at_char): New function. + (check_format_arg): Capture location of format_tree and pass to + check_format_info_main. + (argument_parser): Add fields "start_of_this_format" and + "format_string_cst". + (flag_chars_t::validate): Add param "format_string_cst". Convert + warning_at call using location_from_offset to call to + format_warning_at_char. + (argument_parser::argument_parser): Add param "format_string_cst_" + and use use it to initialize field "format_string_cst". + Initialize new field "start_of_this_format". + (argument_parser::read_format_flags): Convert warning_at call + using location_from_offset to a call to format_warning_at_char. + (argument_parser::read_any_format_left_precision): Likewise. + (argument_parser::read_any_format_precision): Likewise. + (argument_parser::read_any_other_modifier): Likewise. + (argument_parser::find_format_char_info): Likewise, in three places. + (argument_parser::parse_any_scan_set): Likewise, in one place. + (argument_parser::handle_conversions): Likewise, in two places. + (argument_parser::check_argument_type): Add param "fmt_param_loc" + and use it to make a substring_loc. Pass the latter to + check_format_types. + (check_format_info_main): Add params "fmt_param_loc" and + "format_string_cst". Convert warning_at calls using + location_from_offset to calls to format_warning_at_char. Pass the + new params to the arg_parser ctor. Pass "format_string_cst" to + flag_chars.validate. Pass "fmt_param_loc" to + arg_parser.check_argument_type. + (check_format_types): Convert first param from a location_t + to a const substring_loc & and rename to "fmt_loc". Attempt + to extract the range of the relevant parameter and pass it + to format_type_warning. + (format_type_warning): Convert first param from a location_t + to a const substring_loc & and rename to "fmt_loc". Add + params "param_range" and "type". Replace calls to warning_at + with calls to format_warning_at_substring. + 2016-08-08 David Malcolm * c-format.c (class flag_chars_t): New class. diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c index 92d2c1cf5be..eff2ab4321c 100644 --- a/gcc/c-family/c-format.c +++ b/gcc/c-family/c-format.c @@ -29,6 +29,7 @@ along with GCC; see the file COPYING3. If not see #include "intl.h" #include "langhooks.h" #include "c-format.h" +#include "diagnostic.h" /* Handle attributes associated with format checking. */ @@ -65,78 +66,169 @@ static int first_target_format_type; static const char *format_name (int format_num); static int format_flags (int format_num); -/* Given a string S of length LINE_WIDTH, find the visual column - corresponding to OFFSET bytes. */ +/* Emit a warning governed by option OPT, using GMSGID as the format + string and AP as its arguments. -static unsigned int -location_column_from_byte_offset (const char *s, int line_width, - unsigned int offset) -{ - const char * c = s; - if (*c != '"') - return 0; + Attempt to obtain precise location information within a string + literal from FMT_LOC. + + Case 1: if substring location is available, and is within the range of + the format string itself, the primary location of the + diagnostic is the substring range obtained from FMT_LOC, with the + caret at the *end* of the substring range. + + For example: + + test.c:90:10: warning: problem with '%i' here [-Wformat=] + printf ("hello %i", msg); + ~^ + + Case 2: if the substring location is available, but is not within + the range of the format string, the primary location is that of the + format string, and an note is emitted showing the substring location. + + For example: + test.c:90:10: warning: problem with '%i' here [-Wformat=] + printf("hello " INT_FMT " world", msg); + ^~~~~~~~~~~~~~~~~~~~~~~~~ + test.c:19: note: format string is defined here + #define INT_FMT "%i" + ~^ + + Case 3: if precise substring information is unavailable, the primary + location is that of the whole string passed to FMT_LOC's constructor. + For example: + + test.c:90:10: warning: problem with '%i' here [-Wformat=] + printf(fmt, msg); + ^~~ + + For each of cases 1-3, if param_range is non-NULL, then it is used + as a secondary range within the warning. For example, here it + is used with case 1: + + test.c:90:16: warning: '%s' here but arg 2 has 'long' type [-Wformat=] + printf ("foo %s bar", long_i + long_j); + ~^ ~~~~~~~~~~~~~~~ - c++, offset--; - while (offset > 0) + and here with case 2: + + test.c:90:16: warning: '%s' here but arg 2 has 'long' type [-Wformat=] + printf ("foo " STR_FMT " bar", long_i + long_j); + ^~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~ + test.c:89:16: note: format string is defined here + #define STR_FMT "%s" + ~^ + + and with case 3: + + test.c:90:10: warning: '%i' here, but arg 2 is "const char *' [-Wformat=] + printf(fmt, msg); + ^~~ ~~~ + + Return true if a warning was emitted, false otherwise. */ + +ATTRIBUTE_GCC_DIAG (4,0) +static bool +format_warning_va (const substring_loc &fmt_loc, source_range *param_range, + int opt, const char *gmsgid, va_list *ap) +{ + bool substring_within_range = false; + location_t primary_loc; + location_t substring_loc = UNKNOWN_LOCATION; + source_range fmt_loc_range + = get_range_from_loc (line_table, fmt_loc.get_fmt_string_loc ()); + source_range fmt_substring_range; + const char *err = fmt_loc.get_range (&fmt_substring_range); + if (err) + /* Case 3: unable to get substring location. */ + primary_loc = fmt_loc.get_fmt_string_loc (); + else { - if (c - s >= line_width) - return 0; + substring_loc = make_location (fmt_substring_range.m_finish, + fmt_substring_range.m_start, + fmt_substring_range.m_finish); - switch (*c) + if (fmt_substring_range.m_start >= fmt_loc_range.m_start + && fmt_substring_range.m_finish <= fmt_loc_range.m_finish) + /* Case 1. */ { - case '\\': - c++; - if (c - s >= line_width) - return 0; - switch (*c) - { - case '\\': case '\'': case '"': case '?': - case '(': case '{': case '[': case '%': - case 'a': case 'b': case 'f': case 'n': - case 'r': case 't': case 'v': - case 'e': case 'E': - c++, offset--; - break; + substring_within_range = true; + primary_loc = substring_loc; + } + else + /* Case 2. */ + { + substring_within_range = false; + primary_loc = fmt_loc.get_fmt_string_loc (); + } + } - default: - return 0; - } - break; + rich_location richloc (line_table, primary_loc); - case '"': - /* We found the end of the string too early. */ - return 0; - - default: - c++, offset--; - break; - } + if (param_range) + { + location_t param_loc = make_location (param_range->m_start, + param_range->m_start, + param_range->m_finish); + richloc.add_range (param_loc, false); } - return c - s; + + diagnostic_info diagnostic; + diagnostic_set_info (&diagnostic, gmsgid, ap, &richloc, DK_WARNING); + diagnostic.option_index = opt; + bool warned = report_diagnostic (&diagnostic); + + if (!err && substring_loc && !substring_within_range) + /* Case 2. */ + if (warned) + inform (substring_loc, "format string is defined here"); + + return warned; } -/* Return a location that encodes the same location as LOC but shifted - by OFFSET bytes. */ +/* Variadic call to format_warning_va. */ -static location_t -location_from_offset (location_t loc, int offset) +ATTRIBUTE_GCC_DIAG (4,0) +static bool +format_warning_at_substring (const substring_loc &fmt_loc, + source_range *param_range, + int opt, const char *gmsgid, ...) { - gcc_checking_assert (offset >= 0); - if (linemap_location_from_macro_expansion_p (line_table, loc) - || offset < 0) - return loc; + va_list ap; + va_start (ap, gmsgid); + bool warned = format_warning_va (fmt_loc, param_range, opt, gmsgid, &ap); + va_end (ap); - expanded_location s = expand_location_to_spelling_point (loc); - int line_width; - const char *line = location_get_source_line (s.file, s.line, &line_width); - if (line == NULL) - return loc; - line += s.column - 1 ; - line_width -= s.column - 1; - unsigned int column = - location_column_from_byte_offset (line, line_width, (unsigned) offset); + return warned; +} + +/* Emit a warning as per format_warning_va, but construct the substring_loc + for the character at offset (CHAR_IDX - 1) within a string constant + FORMAT_STRING_CST at FMT_STRING_LOC. */ - return linemap_position_for_loc_and_offset (line_table, loc, column); +ATTRIBUTE_GCC_DIAG (5,6) +static bool +format_warning_at_char (location_t fmt_string_loc, tree format_string_cst, + int char_idx, int opt, const char *gmsgid, ...) +{ + va_list ap; + va_start (ap, gmsgid); + tree string_type = TREE_TYPE (format_string_cst); + + /* The callers are of the form: + format_warning (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + where format_chars has already been incremented, so that + CHAR_IDX is one character beyond where the warning should + be emitted. Fix it. */ + char_idx -= 1; + + substring_loc fmt_loc (fmt_string_loc, string_type, char_idx, char_idx); + bool warned = format_warning_va (fmt_loc, NULL, opt, gmsgid, &ap); + va_end (ap); + + return warned; } /* Check that we have a pointer to a string suitable for use as a format. @@ -1018,8 +1110,9 @@ format_flags (int format_num) static void check_format_info (function_format_info *, tree); static void check_format_arg (void *, tree, unsigned HOST_WIDE_INT); static void check_format_info_main (format_check_results *, - function_format_info *, - const char *, int, tree, + function_format_info *, const char *, + location_t, tree, + int, tree, unsigned HOST_WIDE_INT, object_allocator &); @@ -1032,8 +1125,12 @@ static void finish_dollar_format_checking (format_check_results *, int); static const format_flag_spec *get_flag_spec (const format_flag_spec *, int, const char *); -static void check_format_types (location_t, format_wanted_type *); -static void format_type_warning (location_t, format_wanted_type *, tree, tree); +static void check_format_types (const substring_loc &fmt_loc, + format_wanted_type *); +static void format_type_warning (const substring_loc &fmt_loc, + source_range *param_range, + format_wanted_type *, tree, + tree); /* Decode a format type from a string, returning the type, or format_type_error if not valid, in which case the caller should print an @@ -1509,6 +1606,8 @@ check_format_arg (void *ctx, tree format_tree, tree array_size = 0; tree array_init; + location_t fmt_param_loc = EXPR_LOC_OR_LOC (format_tree, input_location); + if (VAR_P (format_tree)) { /* Pull out a constant value if the front end didn't. */ @@ -1684,8 +1783,8 @@ check_format_arg (void *ctx, tree format_tree, need not adjust it for every return. */ res->number_other++; object_allocator fwt_pool ("format_wanted_type pool"); - check_format_info_main (res, info, format_chars, format_length, - params, arg_num, fwt_pool); + check_format_info_main (res, info, format_chars, fmt_param_loc, format_tree, + format_length, params, arg_num, fwt_pool); } /* Support class for argument_parser and check_format_info_main. @@ -1702,6 +1801,7 @@ class flag_chars_t const format_char_info *fci, const format_flag_spec *flag_specs, const char * const format_chars, + tree format_string_cst, location_t format_string_loc, const char * const orig_format_chars, char format_char); @@ -1744,6 +1844,7 @@ class argument_parser { public: argument_parser (function_format_info *info, const char *&format_chars, + tree format_string_cst, const char * const orig_format_chars, location_t format_string_loc, flag_chars_t &flag_chars, int &has_operand_number, tree first_fillin_param, @@ -1799,13 +1900,16 @@ class argument_parser unsigned HOST_WIDE_INT &arg_num, tree ¶ms, const int alloc_flag, - const char * const format_start); + const char * const format_start, + location_t fmt_param_loc); private: const function_format_info *const info; const format_kind_info * const fki; const format_flag_spec * const flag_specs; + const char *start_of_this_format; const char *&format_chars; + const tree format_string_cst; const char * const orig_format_chars; const location_t format_string_loc; object_allocator &fwt_pool; @@ -1855,6 +1959,7 @@ flag_chars_t::validate (const format_kind_info *fki, const format_char_info *fci, const format_flag_spec *flag_specs, const char * const format_chars, + tree format_string_cst, location_t format_string_loc, const char * const orig_format_chars, char format_char) @@ -1870,11 +1975,11 @@ flag_chars_t::validate (const format_kind_info *fki, continue; if (strchr (fci->flag_chars, m_flag_chars[i]) == 0) { - warning_at (location_from_offset (format_string_loc, - format_chars - - orig_format_chars), - OPT_Wformat_, "%s used with %<%%%c%> %s format", - _(s->name), format_char, fki->name); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "%s used with %<%%%c%> %s format", + _(s->name), format_char, fki->name); d++; continue; } @@ -1935,6 +2040,7 @@ flag_chars_t::assignment_suppression_p (const format_kind_info *fki) argument_parser:: argument_parser (function_format_info *info_, const char *&format_chars_, + tree format_string_cst_, const char * const orig_format_chars_, location_t format_string_loc_, flag_chars_t &flag_chars_, @@ -1944,7 +2050,9 @@ argument_parser (function_format_info *info_, const char *&format_chars_, : info (info_), fki (&format_types[info->format_type]), flag_specs (fki->flag_specs), + start_of_this_format (format_chars_), format_chars (format_chars_), + format_string_cst (format_string_cst_), orig_format_chars (orig_format_chars_), format_string_loc (format_string_loc_), fwt_pool (fwt_pool_), @@ -2008,11 +2116,10 @@ argument_parser::read_format_flags () *format_chars, NULL); if (flag_chars.has_char_p (*format_chars)) { - warning_at (location_from_offset (format_string_loc, - format_chars + 1 - - orig_format_chars), - OPT_Wformat_, - "repeated %s in format", _(s->name)); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars + 1 - orig_format_chars, + OPT_Wformat_, + "repeated %s in format", _(s->name)); } else flag_chars.add_char (*format_chars); @@ -2145,10 +2252,10 @@ argument_parser::read_any_format_left_precision () ++format_chars; flag_chars.add_char (fki->left_precision_char); if (!ISDIGIT (*format_chars)) - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "empty left precision in %s format", fki->name); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "empty left precision in %s format", fki->name); while (ISDIGIT (*format_chars)) ++format_chars; } @@ -2236,10 +2343,10 @@ read_any_format_precision (tree ¶ms, { if (!(fki->flags & (int) FMT_FLAG_EMPTY_PREC_OK) && !ISDIGIT (*format_chars)) - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "empty precision in %s format", fki->name); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "empty precision in %s format", fki->name); while (ISDIGIT (*format_chars)) ++format_chars; } @@ -2340,11 +2447,10 @@ argument_parser::read_any_other_modifier () { const format_flag_spec *s = get_flag_spec (flag_specs, *format_chars, NULL); - warning_at (location_from_offset (format_string_loc, - format_chars - - orig_format_chars), - OPT_Wformat_, - "repeated %s in format", _(s->name)); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "repeated %s in format", _(s->name)); } else flag_chars.add_char (*format_chars); @@ -2372,28 +2478,30 @@ argument_parser::find_format_char_info (char format_char) if (fci->format_chars == 0) { if (ISGRAPH (format_char)) - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "unknown conversion type character %qc in format", - format_char); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "unknown conversion type character" + " %qc in format", + format_char); else - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "unknown conversion type character 0x%x in format", - format_char); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "unknown conversion type character" + " 0x%x in format", + format_char); return NULL; } if (pedantic) { if (ADJ_STD (fci->std) > C_STD_VER) - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "%s does not support the %<%%%c%> %s format", - C_STD_NAME (fci->std), format_char, fki->name); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "%s does not support the %<%%%c%> %s format", + C_STD_NAME (fci->std), format_char, fki->name); } return fci; @@ -2496,10 +2604,10 @@ argument_parser::parse_any_scan_set (const format_char_info *fci) ++format_chars; if (*format_chars != ']') /* The end of the format string was reached. */ - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "no closing %<]%> for %<%%[%> format"); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "no closing %<]%> for %<%%[%> format"); } /* Return true if this argument is to be continued to be parsed, @@ -2525,12 +2633,13 @@ argument_parser::handle_conversions (const format_char_info *fci, wanted_type_std = fci->types[len_modifier.val].std; if (wanted_type == 0) { - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "use of %qs length modifier with %qc type character" - " has either no effect or undefined behavior", - len_modifier.chars, format_char); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "use of %qs length modifier with %qc type" + " character has either no effect" + " or undefined behavior", + len_modifier.chars, format_char); /* Heuristic: skip one argument when an invalid length/type combination is encountered. */ arg_num++; @@ -2546,12 +2655,13 @@ argument_parser::handle_conversions (const format_char_info *fci, && ADJ_STD (wanted_type_std) > ADJ_STD (fci->std)) { if (ADJ_STD (wanted_type_std) > C_STD_VER) - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "%s does not support the %<%%%s%c%> %s format", - C_STD_NAME (wanted_type_std), len_modifier.chars, - format_char, fki->name); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "%s does not support the %<%%%s%c%> %s format", + C_STD_NAME (wanted_type_std), + len_modifier.chars, + format_char, fki->name); } return true; @@ -2571,7 +2681,8 @@ check_argument_type (const format_char_info *fci, unsigned HOST_WIDE_INT &arg_num, tree ¶ms, const int alloc_flag, - const char * const format_start) + const char * const format_start, + location_t fmt_param_loc) { if (info->first_arg_num == 0) return true; @@ -2670,7 +2781,13 @@ check_argument_type (const format_char_info *fci, } if (first_wanted_type != 0) - check_format_types (format_string_loc, first_wanted_type); + { + ptrdiff_t offset_to_format_start = (start_of_this_format - 1) - orig_format_chars; + ptrdiff_t offset_to_format_end = (format_chars - 1) - orig_format_chars; + substring_loc fmt_loc (fmt_param_loc, TREE_TYPE (format_string_cst), + offset_to_format_start, offset_to_format_end); + check_format_types (fmt_loc, first_wanted_type); + } return true; } @@ -2685,6 +2802,7 @@ check_argument_type (const format_char_info *fci, static void check_format_info_main (format_check_results *res, function_format_info *info, const char *format_chars, + location_t fmt_param_loc, tree format_string_cst, int format_length, tree params, unsigned HOST_WIDE_INT arg_num, object_allocator &fwt_pool) @@ -2708,10 +2826,10 @@ check_format_info_main (format_check_results *res, continue; if (*format_chars == 0) { - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "spurious trailing %<%%%> in format"); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "spurious trailing %<%%%> in format"); continue; } if (*format_chars == '%') @@ -2721,8 +2839,8 @@ check_format_info_main (format_check_results *res, } flag_chars_t flag_chars; - argument_parser arg_parser (info, format_chars, orig_format_chars, - format_string_loc, + argument_parser arg_parser (info, format_chars, format_string_cst, + orig_format_chars, format_string_loc, flag_chars, has_operand_number, first_fillin_param, fwt_pool); @@ -2759,10 +2877,10 @@ check_format_info_main (format_check_results *res, || (!(fki->flags & (int) FMT_FLAG_FANCY_PERCENT_OK) && format_char == '%')) { - warning_at (location_from_offset (format_string_loc, - format_chars - orig_format_chars), - OPT_Wformat_, - "conversion lacks type at end of format"); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars - orig_format_chars, + OPT_Wformat_, + "conversion lacks type at end of format"); continue; } format_chars++; @@ -2773,6 +2891,7 @@ check_format_info_main (format_check_results *res, continue; flag_chars.validate (fki, fci, flag_specs, format_chars, + format_string_cst, format_string_loc, orig_format_chars, format_char); const int alloc_flag = flag_chars.get_alloc_flag (fki); @@ -2803,15 +2922,15 @@ check_format_info_main (format_check_results *res, suppressed, arg_num, params, alloc_flag, - format_start)) + format_start, fmt_param_loc)) return; } if (format_chars - orig_format_chars != format_length) - warning_at (location_from_offset (format_string_loc, - format_chars + 1 - orig_format_chars), - OPT_Wformat_contains_nul, - "embedded %<\\0%> in format"); + format_warning_at_char (format_string_loc, format_string_cst, + format_chars + 1 - orig_format_chars, + OPT_Wformat_contains_nul, + "embedded %<\\0%> in format"); if (info->first_arg_num != 0 && params != 0 && has_operand_number <= 0) { @@ -2822,12 +2941,12 @@ check_format_info_main (format_check_results *res, finish_dollar_format_checking (res, fki->flags & (int) FMT_FLAG_DOLLAR_GAP_POINTER_OK); } - /* Check the argument types from a single format conversion (possibly - including width and precision arguments). LOC is the location of - the format string. */ + including width and precision arguments). FMT_LOC is the + location of the format conversion. */ static void -check_format_types (location_t loc, format_wanted_type *types) +check_format_types (const substring_loc &fmt_loc, + format_wanted_type *types) { for (; types != 0; types = types->next) { @@ -2854,7 +2973,7 @@ check_format_types (location_t loc, format_wanted_type *types) cur_param = types->param; if (!cur_param) { - format_type_warning (loc, types, wanted_type, NULL); + format_type_warning (fmt_loc, NULL, types, wanted_type, NULL); continue; } @@ -2864,6 +2983,16 @@ check_format_types (location_t loc, format_wanted_type *types) orig_cur_type = cur_type; char_type_flag = 0; + source_range param_range; + source_range *param_range_ptr; + if (CAN_HAVE_LOCATION_P (cur_param)) + { + param_range = EXPR_LOCATION_RANGE (cur_param); + param_range_ptr = ¶m_range; + } + else + param_range_ptr = NULL; + STRIP_NOPS (cur_param); /* Check the types of any additional pointer arguments @@ -2928,7 +3057,8 @@ check_format_types (location_t loc, format_wanted_type *types) } else { - format_type_warning (loc, types, wanted_type, orig_cur_type); + format_type_warning (fmt_loc, param_range_ptr, + types, wanted_type, orig_cur_type); break; } } @@ -2996,20 +3126,24 @@ check_format_types (location_t loc, format_wanted_type *types) && TYPE_PRECISION (cur_type) == TYPE_PRECISION (wanted_type)) continue; /* Now we have a type mismatch. */ - format_type_warning (loc, types, wanted_type, orig_cur_type); + format_type_warning (fmt_loc, param_range_ptr, types, + wanted_type, orig_cur_type); } } -/* Give a warning at LOC about a format argument of different type from that - expected. WANTED_TYPE is the type the argument should have, possibly - stripped of pointer dereferences. The description (such as "field +/* Give a warning at FMT_LOC about a format argument of different type + from that expected. If non-NULL, PARAM_RANGE is the source range of the + relevant argument. WANTED_TYPE is the type the argument should have, + possibly stripped of pointer dereferences. The description (such as "field precision"), the placement in the format string, a possibly more friendly name of WANTED_TYPE, and the number of pointer dereferences are taken from TYPE. ARG_TYPE is the type of the actual argument, or NULL if it is missing. */ static void -format_type_warning (location_t loc, format_wanted_type *type, +format_type_warning (const substring_loc &fmt_loc, + source_range *param_range, + format_wanted_type *type, tree wanted_type, tree arg_type) { int kind = type->kind; @@ -3018,7 +3152,6 @@ format_type_warning (location_t loc, format_wanted_type *type, int format_length = type->format_length; int pointer_count = type->pointer_count; int arg_num = type->arg_num; - unsigned int offset_loc = type->offset_loc; char *p; /* If ARG_TYPE is a typedef with a misleading name (for example, @@ -3052,41 +3185,47 @@ format_type_warning (location_t loc, format_wanted_type *type, p[pointer_count + 1] = 0; } - loc = location_from_offset (loc, offset_loc); - if (wanted_type_name) { if (arg_type) - warning_at (loc, OPT_Wformat_, - "%s %<%s%.*s%> expects argument of type %<%s%s%>, " - "but argument %d has type %qT", - gettext (kind_descriptions[kind]), - (kind == CF_KIND_FORMAT ? "%" : ""), - format_length, format_start, - wanted_type_name, p, arg_num, arg_type); + format_warning_at_substring + (fmt_loc, param_range, + OPT_Wformat_, + "%s %<%s%.*s%> expects argument of type %<%s%s%>, " + "but argument %d has type %qT", + gettext (kind_descriptions[kind]), + (kind == CF_KIND_FORMAT ? "%" : ""), + format_length, format_start, + wanted_type_name, p, arg_num, arg_type); else - warning_at (loc, OPT_Wformat_, - "%s %<%s%.*s%> expects a matching %<%s%s%> argument", - gettext (kind_descriptions[kind]), - (kind == CF_KIND_FORMAT ? "%" : ""), - format_length, format_start, wanted_type_name, p); + format_warning_at_substring + (fmt_loc, param_range, + OPT_Wformat_, + "%s %<%s%.*s%> expects a matching %<%s%s%> argument", + gettext (kind_descriptions[kind]), + (kind == CF_KIND_FORMAT ? "%" : ""), + format_length, format_start, wanted_type_name, p); } else { if (arg_type) - warning_at (loc, OPT_Wformat_, - "%s %<%s%.*s%> expects argument of type %<%T%s%>, " - "but argument %d has type %qT", - gettext (kind_descriptions[kind]), - (kind == CF_KIND_FORMAT ? "%" : ""), - format_length, format_start, - wanted_type, p, arg_num, arg_type); + format_warning_at_substring + (fmt_loc, param_range, + OPT_Wformat_, + "%s %<%s%.*s%> expects argument of type %<%T%s%>, " + "but argument %d has type %qT", + gettext (kind_descriptions[kind]), + (kind == CF_KIND_FORMAT ? "%" : ""), + format_length, format_start, + wanted_type, p, arg_num, arg_type); else - warning_at (loc, OPT_Wformat_, - "%s %<%s%.*s%> expects a matching %<%T%s%> argument", - gettext (kind_descriptions[kind]), - (kind == CF_KIND_FORMAT ? "%" : ""), - format_length, format_start, wanted_type, p); + format_warning_at_substring + (fmt_loc, param_range, + OPT_Wformat_, + "%s %<%s%.*s%> expects a matching %<%T%s%> argument", + gettext (kind_descriptions[kind]), + (kind == CF_KIND_FORMAT ? "%" : ""), + format_length, format_start, wanted_type, p); } } diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 5e34b8595da..6e406f6de8c 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,11 @@ +2016-08-08 David Malcolm + + PR c/52952 + * gcc.dg/cpp/pr66415-1.c: Likewise. + * gcc.dg/format/asm_fprintf-1.c: Update column numbers. + * gcc.dg/format/c90-printf-1.c: Likewise. + * gcc.dg/format/diagnostic-ranges.c: New test case. + 2016-08-08 Jakub Jelinek PR fortran/72716 diff --git a/gcc/testsuite/gcc.dg/cpp/pr66415-1.c b/gcc/testsuite/gcc.dg/cpp/pr66415-1.c index 349ec4883fa..1f67cb4f84c 100644 --- a/gcc/testsuite/gcc.dg/cpp/pr66415-1.c +++ b/gcc/testsuite/gcc.dg/cpp/pr66415-1.c @@ -1,9 +1,15 @@ /* PR c/66415 */ /* { dg-do compile } */ -/* { dg-options "-Wformat" } */ +/* { dg-options "-Wformat -fdiagnostics-show-caret" } */ void fn1 (void) { __builtin_printf ("xxxxxxxxxxxxxxxxx%dxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); /* { dg-warning "71:format" } */ + +/* { dg-begin-multiline-output "" } + __builtin_printf ("xxxxxxxxxxxxxxxxx%dxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"); + ~^ + { dg-end-multiline-output "" } */ + } diff --git a/gcc/testsuite/gcc.dg/format/asm_fprintf-1.c b/gcc/testsuite/gcc.dg/format/asm_fprintf-1.c index 2eabbf9190e..50ca57206b1 100644 --- a/gcc/testsuite/gcc.dg/format/asm_fprintf-1.c +++ b/gcc/testsuite/gcc.dg/format/asm_fprintf-1.c @@ -66,9 +66,9 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p, asm_fprintf ("%d", i, i); /* { dg-warning "16:arguments" "wrong number of args" } */ /* Miscellaneous bogus constructions. */ asm_fprintf (""); /* { dg-warning "16:zero-length" "warning for empty format" } */ - asm_fprintf ("\0"); /* { dg-warning "17:embedded" "warning for embedded NUL" } */ - asm_fprintf ("%d\0", i); /* { dg-warning "19:embedded" "warning for embedded NUL" } */ - asm_fprintf ("%d\0%d", i, i); /* { dg-warning "19:embedded|too many" "warning for embedded NUL" } */ + asm_fprintf ("\0"); /* { dg-warning "18:embedded" "warning for embedded NUL" } */ + asm_fprintf ("%d\0", i); /* { dg-warning "20:embedded" "warning for embedded NUL" } */ + asm_fprintf ("%d\0%d", i, i); /* { dg-warning "20:embedded|too many" "warning for embedded NUL" } */ asm_fprintf (NULL); /* { dg-warning "null" "null format string warning" } */ asm_fprintf ("%"); /* { dg-warning "17:trailing" "trailing % warning" } */ asm_fprintf ("%++d", i); /* { dg-warning "19:repeated" "repeated flag warning" } */ diff --git a/gcc/testsuite/gcc.dg/format/c90-printf-1.c b/gcc/testsuite/gcc.dg/format/c90-printf-1.c index 5329dad83a4..338b971da09 100644 --- a/gcc/testsuite/gcc.dg/format/c90-printf-1.c +++ b/gcc/testsuite/gcc.dg/format/c90-printf-1.c @@ -58,11 +58,11 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p, printf ("%-%"); /* { dg-warning "13:type" "missing type" } */ /* { dg-warning "14:trailing" "bogus %%" { target *-*-* } 58 } */ printf ("%-%\n"); /* { dg-warning "13:format" "bogus %%" } */ - /* { dg-warning "15:format" "bogus %%" { target *-*-* } 60 } */ + /* { dg-warning "16:format" "bogus %%" { target *-*-* } 60 } */ printf ("%5%\n"); /* { dg-warning "13:format" "bogus %%" } */ - /* { dg-warning "15:format" "bogus %%" { target *-*-* } 62 } */ + /* { dg-warning "16:format" "bogus %%" { target *-*-* } 62 } */ printf ("%h%\n"); /* { dg-warning "13:format" "bogus %%" } */ - /* { dg-warning "15:format" "bogus %%" { target *-*-* } 64 } */ + /* { dg-warning "16:format" "bogus %%" { target *-*-* } 64 } */ /* Valid and invalid %h, %l, %L constructions. */ printf ("%hd", i); printf ("%hi", i); @@ -184,8 +184,8 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p, printf ("%-08G", d); /* { dg-warning "11:flags|ignored" "0 flag ignored with - flag" } */ /* Various tests of bad argument types. */ printf ("%d", l); /* { dg-warning "13:format" "bad argument types" } */ - printf ("%*.*d", l, i2, i); /* { dg-warning "13:field" "bad * argument types" } */ - printf ("%*.*d", i1, l, i); /* { dg-warning "15:field" "bad * argument types" } */ + printf ("%*.*d", l, i2, i); /* { dg-warning "16:field" "bad * argument types" } */ + printf ("%*.*d", i1, l, i); /* { dg-warning "16:field" "bad * argument types" } */ printf ("%ld", i); /* { dg-warning "14:format" "bad argument types" } */ printf ("%s", n); /* { dg-warning "13:format" "bad argument types" } */ printf ("%p", i); /* { dg-warning "13:format" "bad argument types" } */ @@ -231,8 +231,8 @@ foo (int i, int i1, int i2, unsigned int u, double d, char *s, void *p, printf ("%d", i, i); /* { dg-warning "11:arguments" "wrong number of args" } */ /* Miscellaneous bogus constructions. */ printf (""); /* { dg-warning "11:zero-length" "warning for empty format" } */ - printf ("\0"); /* { dg-warning "12:embedded" "warning for embedded NUL" } */ - printf ("%d\0", i); /* { dg-warning "14:embedded" "warning for embedded NUL" } */ + printf ("\0"); /* { dg-warning "13:embedded" "warning for embedded NUL" } */ + printf ("%d\0", i); /* { dg-warning "15:embedded" "warning for embedded NUL" } */ printf ("%d\0%d", i, i); /* { dg-warning "embedded|too many" "warning for embedded NUL" } */ printf (NULL); /* { dg-warning "3:null" "null format string warning" } */ printf ("%"); /* { dg-warning "12:trailing" "trailing % warning" } */ diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c new file mode 100644 index 00000000000..9e86b5210f1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c @@ -0,0 +1,222 @@ +/* { dg-options "-Wformat -fdiagnostics-show-caret" } */ + +/* See PR 52952. */ + +#include "format.h" + +void test_mismatching_types (const char *msg) +{ + printf("hello %i", msg); /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */ + +/* TODO: ideally would also underline "msg". */ +/* { dg-begin-multiline-output "" } + printf("hello %i", msg); + ~^ + { dg-end-multiline-output "" } */ +} + +void test_multiple_arguments (void) +{ + printf ("arg0: %i arg1: %s arg 2: %i", /* { dg-warning "29: format '%s'" } */ + 100, 101, 102); +/* TODO: ideally would also underline "101". */ +/* { dg-begin-multiline-output "" } + printf ("arg0: %i arg1: %s arg 2: %i", + ~^ + { dg-end-multiline-output "" } */ +} + +void test_multiple_arguments_2 (int i, int j) +{ + printf ("arg0: %i arg1: %s arg 2: %i", /* { dg-warning "29: format '%s'" } */ + 100, i + j, 102); +/* { dg-begin-multiline-output "" } + printf ("arg0: %i arg1: %s arg 2: %i", + ~^ + 100, i + j, 102); + ~~~~~ + { dg-end-multiline-output "" } */ +} + +void multiline_format_string (void) { + printf ("before the fmt specifier" /* { dg-warning "11: format '%d' expects a matching 'int' argument" } */ +/* { dg-begin-multiline-output "" } + printf ("before the fmt specifier" + ^~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + "%" + "d" /* { dg-message "12: format string is defined here" } */ + "after the fmt specifier"); + +/* { dg-begin-multiline-output "" } + "%" + ~~ + "d" + ~^ + { dg-end-multiline-output "" } */ +} + +void test_hex (const char *msg) +{ + /* "%" is \x25 + "i" is \x69 */ + printf("hello \x25\x69", msg); /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */ + +/* TODO: ideally would also underline "msg". */ +/* { dg-begin-multiline-output "" } + printf("hello \x25\x69", msg); + ~~~~~~~^ + { dg-end-multiline-output "" } */ +} + +void test_oct (const char *msg) +{ + /* "%" is octal 045 + "i" is octal 151. */ + printf("hello \045\151", msg); /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */ + +/* TODO: ideally would also underline "msg". */ +/* { dg-begin-multiline-output "" } + printf("hello \045\151", msg); + ~~~~~~~^ + { dg-end-multiline-output "" } */ +} + +void test_multiple (const char *msg) +{ + /* "%" is \x25 in hex + "i" is \151 in octal. */ + printf("prefix" "\x25" "\151" "suffix", /* { dg-warning "format '%i'" } */ + msg); +/* { dg-begin-multiline-output "" } + printf("prefix" "\x25" "\151" "suffix", + ^~~~~~~~ + { dg-end-multiline-output "" } */ + +/* TODO: ideally would also underline "msg". */ +/* { dg-begin-multiline-output "" } + printf("prefix" "\x25" "\151" "suffix", + ~~~~~~~~~~~^ + { dg-end-multiline-output "" } */ +} + +void test_u8 (const char *msg) +{ + printf(u8"hello %i", msg);/* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */ +/* TODO: ideally would also underline "msg". */ +/* { dg-begin-multiline-output "" } + printf(u8"hello %i", msg); + ~^ + { dg-end-multiline-output "" } */ +} + +void test_param (long long_i, long long_j) +{ + printf ("foo %s bar", long_i + long_j); /* { dg-warning "17: format '%s' expects argument of type 'char \\*', but argument 2 has type 'long int'" } */ +/* { dg-begin-multiline-output "" } + printf ("foo %s bar", long_i + long_j); + ~^ ~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ +} + +void test_field_width_specifier (long l, int i1, int i2) +{ + printf (" %*.*d ", l, i1, i2); /* { dg-warning "17: field width specifier '\\*' expects argument of type 'int', but argument 2 has type 'long int'" } */ +/* { dg-begin-multiline-output "" } + printf (" %*.*d ", l, i1, i2); + ~~~~^ + { dg-end-multiline-output "" } */ +} + +void test_spurious_percent (void) +{ + printf("hello world %"); /* { dg-warning "23: spurious trailing" } */ + +/* { dg-begin-multiline-output "" } + printf("hello world %"); + ^ + { dg-end-multiline-output "" } */ +} + +void test_empty_precision (char *s, size_t m, double d) +{ + strfmon (s, m, "%#.5n", d); /* { dg-warning "20: empty left precision in gnu_strfmon format" } */ +/* { dg-begin-multiline-output "" } + strfmon (s, m, "%#.5n", d); + ^ + { dg-end-multiline-output "" } */ + + strfmon (s, m, "%#5.n", d); /* { dg-warning "22: empty precision in gnu_strfmon format" } */ +/* { dg-begin-multiline-output "" } + strfmon (s, m, "%#5.n", d); + ^ + { dg-end-multiline-output "" } */ +} + +void test_repeated (int i) +{ + printf ("%++d", i); /* { dg-warning "14: repeated '\\+' flag in format" } */ +/* { dg-begin-multiline-output "" } + printf ("%++d", i); + ^ + { dg-end-multiline-output "" } */ +} + +void test_conversion_lacks_type (void) +{ + printf (" %h"); /* { dg-warning "14:conversion lacks type at end of format" } */ +/* { dg-begin-multiline-output "" } + printf (" %h"); + ^ + { dg-end-multiline-output "" } */ +} + +void test_embedded_nul (void) +{ + printf (" \0 "); /* { dg-warning "14:embedded" "warning for embedded NUL" } */ +/* { dg-begin-multiline-output "" } + printf (" \0 "); + ~^ + { dg-end-multiline-output "" } */ +} + +void test_macro (const char *msg) +{ +#define INT_FMT "%i" /* { dg-message "19: format string is defined here" } */ + printf("hello " INT_FMT " world", msg); /* { dg-warning "10: format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */ +/* { dg-begin-multiline-output "" } + printf("hello " INT_FMT " world", msg); + ^~~~~~~~ + { dg-end-multiline-output "" } */ +/* { dg-begin-multiline-output "" } + #define INT_FMT "%i" + ~^ + { dg-end-multiline-output "" } */ +} + +void test_non_contiguous_strings (void) +{ + __builtin_printf(" %" "d ", 0.5); /* { dg-warning "20: format .%d. expects argument of type .int., but argument 2 has type .double." } */ + /* { dg-message "26: format string is defined here" "" { target *-*-* } 200 } */ + /* { dg-begin-multiline-output "" } + __builtin_printf(" %" "d ", 0.5); + ^~~~ + { dg-end-multiline-output "" } */ + /* { dg-begin-multiline-output "" } + __builtin_printf(" %" "d ", 0.5); + ~~~~^ + { dg-end-multiline-output "" } */ +} + +void test_const_arrays (void) +{ + /* TODO: ideally we'd highlight both the format string *and* the use of + it here. For now, just verify that we gracefully handle this case. */ + const char a[] = " %d "; + __builtin_printf(a, 0.5); /* { dg-warning "20: format .%d. expects argument of type .int., but argument 2 has type .double." } */ + /* { dg-begin-multiline-output "" } + __builtin_printf(a, 0.5); + ^ + { dg-end-multiline-output "" } */ +} -- 2.30.2