From 89b6abbb7e4ba154dc5dd2458cd3ea93ddabd800 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Tue, 17 Oct 2017 19:41:01 +0000 Subject: [PATCH] Simplify format_warning_at_substring API The format_warning_at_substring API has a rather clunk way of indicating the location of the pertinent param (if any): a source_range * is passed in, which can be NULL. Doing so requires extracting a range from the location_t and passing around a pointer to it, or NULL, as needed. This patch simplifies things by eliminating the source_range * in favor of a location_t, with UNKNOWN_LOCATION used to signify that no param location is available. gcc/c-family/ChangeLog: * c-format.c (format_warning_at_char): Pass UNKNOWN_LOCATION rather than NULL to format_warning_va. (check_format_types): Likewise when calling format_type_warning. Remove code to extract source_ranges and source_range * in favor of just a location_t. (format_type_warning): Convert source_range * param to a location_t. gcc/ChangeLog: * gimple-ssa-sprintf.c (fmtwarn): Update for changed signature of format_warning_at_substring. (maybe_warn): Convert source_range * param to a location_t. Pass UNKNOWN_LOCATION rather than NULL to fmtwarn. (format_directive): Remove code to extract source_ranges and source_range * in favor of just a location_t. (parse_directive): Pass UNKNOWN_LOCATION rather than NULL to fmtwarn. * substring-locations.c (format_warning_va): Convert source_range * param to a location_t. (format_warning_at_substring): Likewise. * substring-locations.h (format_warning_va): Likewise. (format_warning_at_substring): Likewise. From-SVN: r253827 --- gcc/ChangeLog | 16 +++++++++++ gcc/c-family/ChangeLog | 10 +++++++ gcc/c-family/c-format.c | 45 +++++++++++++---------------- gcc/gimple-ssa-sprintf.c | 60 ++++++++++++++++++--------------------- gcc/substring-locations.c | 17 ++++------- gcc/substring-locations.h | 4 +-- 6 files changed, 81 insertions(+), 71 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index de2aa1dc389..fed7c36845a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,19 @@ +2017-10-17 David Malcolm + + * gimple-ssa-sprintf.c (fmtwarn): Update for changed signature of + format_warning_at_substring. + (maybe_warn): Convert source_range * param to a location_t. Pass + UNKNOWN_LOCATION rather than NULL to fmtwarn. + (format_directive): Remove code to extract source_ranges and + source_range * in favor of just a location_t. + (parse_directive): Pass UNKNOWN_LOCATION rather than NULL to + fmtwarn. + * substring-locations.c (format_warning_va): Convert + source_range * param to a location_t. + (format_warning_at_substring): Likewise. + * substring-locations.h (format_warning_va): Likewise. + (format_warning_at_substring): Likewise. + 2017-10-17 Jan Hubicka * target.h (enum vect_cost_for_stmt): Add vec_gather_load and diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index f880f29fd55..ef1211523a4 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,13 @@ +2017-10-17 David Malcolm + + * c-format.c (format_warning_at_char): Pass UNKNOWN_LOCATION + rather than NULL to format_warning_va. + (check_format_types): Likewise when calling format_type_warning. + Remove code to extract source_ranges and source_range * in favor + of just a location_t. + (format_type_warning): Convert source_range * param to a + location_t. + 2017-10-13 Jakub Jelinek * c-gimplify.c (c_gimplify_expr): Handle [LR]ROTATE_EXPR like diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c index 0dba9793311..164d0353967 100644 --- a/gcc/c-family/c-format.c +++ b/gcc/c-family/c-format.c @@ -97,7 +97,8 @@ format_warning_at_char (location_t fmt_string_loc, tree format_string_cst, substring_loc fmt_loc (fmt_string_loc, string_type, char_idx, char_idx, char_idx); - bool warned = format_warning_va (fmt_loc, NULL, NULL, opt, gmsgid, &ap); + bool warned = format_warning_va (fmt_loc, UNKNOWN_LOCATION, NULL, opt, + gmsgid, &ap); va_end (ap); return warned; @@ -1039,7 +1040,7 @@ static void check_format_types (const substring_loc &fmt_loc, char conversion_char, vec *arglocs); static void format_type_warning (const substring_loc &fmt_loc, - source_range *param_range, + location_t param_loc, format_wanted_type *, tree, tree, const format_kind_info *fki, @@ -3073,8 +3074,9 @@ check_format_types (const substring_loc &fmt_loc, cur_param = types->param; if (!cur_param) { - format_type_warning (fmt_loc, NULL, types, wanted_type, NULL, fki, - offset_to_type_start, conversion_char); + format_type_warning (fmt_loc, UNKNOWN_LOCATION, types, wanted_type, + NULL, fki, offset_to_type_start, + conversion_char); continue; } @@ -3084,23 +3086,15 @@ check_format_types (const substring_loc &fmt_loc, orig_cur_type = cur_type; char_type_flag = 0; - source_range param_range; - source_range *param_range_ptr; + location_t param_loc = UNKNOWN_LOCATION; if (EXPR_HAS_LOCATION (cur_param)) - { - param_range = EXPR_LOCATION_RANGE (cur_param); - param_range_ptr = ¶m_range; - } + param_loc = EXPR_LOCATION (cur_param); else if (arglocs) { /* arg_num is 1-based. */ gcc_assert (types->arg_num > 0); - location_t param_loc = (*arglocs)[types->arg_num - 1]; - param_range = get_range_from_loc (line_table, param_loc); - param_range_ptr = ¶m_range; + param_loc = (*arglocs)[types->arg_num - 1]; } - else - param_range_ptr = NULL; STRIP_NOPS (cur_param); @@ -3166,7 +3160,7 @@ check_format_types (const substring_loc &fmt_loc, } else { - format_type_warning (fmt_loc, param_range_ptr, + format_type_warning (fmt_loc, param_loc, types, wanted_type, orig_cur_type, fki, offset_to_type_start, conversion_char); break; @@ -3236,7 +3230,7 @@ check_format_types (const substring_loc &fmt_loc, && TYPE_PRECISION (cur_type) == TYPE_PRECISION (wanted_type)) continue; /* Now we have a type mismatch. */ - format_type_warning (fmt_loc, param_range_ptr, types, + format_type_warning (fmt_loc, param_loc, types, wanted_type, orig_cur_type, fki, offset_to_type_start, conversion_char); } @@ -3544,8 +3538,9 @@ get_corrected_substring (const substring_loc &fmt_loc, /* Give a warning about a format argument of different type from that expected. The range of the diagnostic is taken from WHOLE_FMT_LOC; the caret location is based on the location of the char at TYPE->offset_loc. - If non-NULL, PARAM_RANGE is the source range of the - relevant argument. WANTED_TYPE is the type the argument should have, + PARAM_LOC is the location of the relevant argument, or UNKNOWN_LOCATION + if this is unavailable. + 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 @@ -3566,7 +3561,7 @@ get_corrected_substring (const substring_loc &fmt_loc, V~~~~~~~~ : range of WHOLE_FMT_LOC, from cols 23-31 sprintf (d, "before %-+*.*lld after", int_expr, int_expr, long_expr); ^ ^ ^~~~~~~~~ - | ` CONVERSION_CHAR: 'd' *PARAM_RANGE + | ` CONVERSION_CHAR: 'd' PARAM_LOC type starts here OFFSET_TO_TYPE_START is 13, the offset to the "lld" within the @@ -3574,7 +3569,7 @@ get_corrected_substring (const substring_loc &fmt_loc, static void format_type_warning (const substring_loc &whole_fmt_loc, - source_range *param_range, + location_t param_loc, format_wanted_type *type, tree wanted_type, tree arg_type, const format_kind_info *fki, @@ -3636,7 +3631,7 @@ format_type_warning (const substring_loc &whole_fmt_loc, { if (arg_type) format_warning_at_substring - (fmt_loc, param_range, + (fmt_loc, param_loc, corrected_substring, OPT_Wformat_, "%s %<%s%.*s%> expects argument of type %<%s%s%>, " "but argument %d has type %qT", @@ -3646,7 +3641,7 @@ format_type_warning (const substring_loc &whole_fmt_loc, wanted_type_name, p, arg_num, arg_type); else format_warning_at_substring - (fmt_loc, param_range, + (fmt_loc, param_loc, corrected_substring, OPT_Wformat_, "%s %<%s%.*s%> expects a matching %<%s%s%> argument", gettext (kind_descriptions[kind]), @@ -3657,7 +3652,7 @@ format_type_warning (const substring_loc &whole_fmt_loc, { if (arg_type) format_warning_at_substring - (fmt_loc, param_range, + (fmt_loc, param_loc, corrected_substring, OPT_Wformat_, "%s %<%s%.*s%> expects argument of type %<%T%s%>, " "but argument %d has type %qT", @@ -3667,7 +3662,7 @@ format_type_warning (const substring_loc &whole_fmt_loc, wanted_type, p, arg_num, arg_type); else format_warning_at_substring - (fmt_loc, param_range, + (fmt_loc, param_loc, corrected_substring, OPT_Wformat_, "%s %<%s%.*s%> expects a matching %<%T%s%> argument", gettext (kind_descriptions[kind]), diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c index 7899e09195f..9770df72898 100644 --- a/gcc/gimple-ssa-sprintf.c +++ b/gcc/gimple-ssa-sprintf.c @@ -583,7 +583,7 @@ get_format_string (tree format, location_t *ploc) /* For convenience and brevity. */ static bool - (* const fmtwarn) (const substring_loc &, const source_range *, + (* const fmtwarn) (const substring_loc &, location_t, const char *, int, const char *, ...) = format_warning_at_substring; @@ -2418,7 +2418,7 @@ should_warn_p (const pass_sprintf_length::call_info &info, Return true if a warning has been issued. */ static bool -maybe_warn (substring_loc &dirloc, source_range *pargrange, +maybe_warn (substring_loc &dirloc, location_t argloc, const pass_sprintf_length::call_info &info, const result_range &avail_range, const result_range &res, const directive &dir) @@ -2476,8 +2476,8 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, : G_("%qE writing a terminating nul past the end " "of the destination"))); - return fmtwarn (dirloc, NULL, NULL, info.warnopt (), fmtstr, - info.func); + return fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (), + fmtstr, info.func); } if (res.min == res.max) @@ -2500,7 +2500,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "%wu bytes into a region of size %wu")) : G_("%<%.*s%> directive writing %wu bytes " "into a region of size %wu"))); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, navail); @@ -2517,7 +2517,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "up to %wu bytes into a region of size %wu")) : G_("%<%.*s%> directive writing up to %wu bytes " "into a region of size %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.max, navail); @@ -2537,7 +2537,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "likely %wu or more bytes into a region of size %wu")) : G_("%<%.*s%> directive writing likely %wu or more bytes " "into a region of size %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.likely, navail); @@ -2554,7 +2554,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "between %wu and %wu bytes into a region of size %wu")) : G_("%<%.*s%> directive writing between %wu and " "%wu bytes into a region of size %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, res.max, navail); @@ -2569,7 +2569,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "%wu or more bytes into a region of size %wu")) : G_("%<%.*s%> directive writing %wu or more bytes " "into a region of size %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, navail); @@ -2603,7 +2603,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, : G_("%qE writing a terminating nul past the end " "of the destination"))); - return fmtwarn (dirloc, NULL, NULL, info.warnopt (), fmtstr, + return fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (), fmtstr, info.func); } @@ -2628,7 +2628,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, : G_("%<%.*s%> directive writing %wu bytes " "into a region of size between %wu and %wu"))); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, avail_range.min, avail_range.max); @@ -2647,7 +2647,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "%wu and %wu")) : G_("%<%.*s%> directive writing up to %wu bytes " "into a region of size between %wu and %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.max, avail_range.min, avail_range.max); @@ -2669,7 +2669,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "%wu and %wu")) : G_("%<%.*s%> directive writing likely %wu or more bytes " "into a region of size between %wu and %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.likely, avail_range.min, avail_range.max); @@ -2688,7 +2688,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "between %wu and %wu")) : G_("%<%.*s%> directive writing between %wu and " "%wu bytes into a region of size between %wu and %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, res.max, avail_range.min, avail_range.max); @@ -2705,7 +2705,7 @@ maybe_warn (substring_loc &dirloc, source_range *pargrange, "%wu and %wu")) : G_("%<%.*s%> directive writing %wu or more bytes " "into a region of size between %wu and %wu")); - return fmtwarn (dirloc, pargrange, NULL, + return fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg), res.min, avail_range.min, avail_range.max); @@ -2730,17 +2730,11 @@ format_directive (const pass_sprintf_length::call_info &info, substring_loc dirloc (info.fmtloc, TREE_TYPE (info.format), offset, start, length); - /* Also create a location range for the argument if possible. + /* Also get the location of the argument if possible. This doesn't work for integer literals or function calls. */ - source_range argrange; - source_range *pargrange; - if (dir.arg && CAN_HAVE_LOCATION_P (dir.arg)) - { - argrange = EXPR_LOCATION_RANGE (dir.arg); - pargrange = &argrange; - } - else - pargrange = NULL; + location_t argloc = UNKNOWN_LOCATION; + if (dir.arg) + argloc = EXPR_LOCATION (dir.arg); /* Bail when there is no function to compute the output length, or when minimum length checking has been disabled. */ @@ -2797,7 +2791,7 @@ format_directive (const pass_sprintf_length::call_info &info, if (fmtres.nullp) { - fmtwarn (dirloc, pargrange, NULL, info.warnopt (), + fmtwarn (dirloc, argloc, NULL, info.warnopt (), "%<%.*s%> directive argument is null", dirlen, target_to_host (hostdir, sizeof hostdir, dir.beg)); @@ -2816,7 +2810,7 @@ format_directive (const pass_sprintf_length::call_info &info, bool warned = res->warned; if (!warned) - warned = maybe_warn (dirloc, pargrange, info, avail_range, + warned = maybe_warn (dirloc, argloc, info, avail_range, fmtres.range, dir); /* Bump up the total maximum if it isn't too big. */ @@ -2862,7 +2856,7 @@ format_directive (const pass_sprintf_length::call_info &info, (like Glibc does under some conditions). */ if (fmtres.range.min == fmtres.range.max) - warned = fmtwarn (dirloc, pargrange, NULL, + warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (), "%<%.*s%> directive output of %wu bytes exceeds " "minimum required size of 4095", @@ -2878,7 +2872,7 @@ format_directive (const pass_sprintf_length::call_info &info, : G_("%<%.*s%> directive output between %wu and %wu " "bytes exceeds minimum required size of 4095")); - warned = fmtwarn (dirloc, pargrange, NULL, + warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dirlen, target_to_host (hostdir, sizeof hostdir, dir.beg), fmtres.range.min, fmtres.range.max); @@ -2906,7 +2900,7 @@ format_directive (const pass_sprintf_length::call_info &info, to exceed INT_MAX bytes. */ if (fmtres.range.min == fmtres.range.max) - warned = fmtwarn (dirloc, pargrange, NULL, info.warnopt (), + warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (), "%<%.*s%> directive output of %wu bytes causes " "result to exceed %", dirlen, @@ -2920,7 +2914,7 @@ format_directive (const pass_sprintf_length::call_info &info, "bytes causes result to exceed %") : G_ ("%<%.*s%> directive output between %wu and %wu " "bytes may cause result to exceed %")); - warned = fmtwarn (dirloc, pargrange, NULL, + warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (), fmtstr, dirlen, target_to_host (hostdir, sizeof hostdir, dir.beg), fmtres.range.min, fmtres.range.max); @@ -3351,7 +3345,7 @@ parse_directive (pass_sprintf_length::call_info &info, substring_loc dirloc (info.fmtloc, TREE_TYPE (info.format), caret, begin, end); - fmtwarn (dirloc, NULL, NULL, + fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (), "%<%.*s%> directive width out of range", dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg)); } @@ -3385,7 +3379,7 @@ parse_directive (pass_sprintf_length::call_info &info, substring_loc dirloc (info.fmtloc, TREE_TYPE (info.format), caret, begin, end); - fmtwarn (dirloc, NULL, NULL, + fmtwarn (dirloc, UNKNOWN_LOCATION, NULL, info.warnopt (), "%<%.*s%> directive precision out of range", dir.len, target_to_host (hostdir, sizeof hostdir, dir.beg)); } diff --git a/gcc/substring-locations.c b/gcc/substring-locations.c index 433023d9845..095e5d073a7 100644 --- a/gcc/substring-locations.c +++ b/gcc/substring-locations.c @@ -63,7 +63,7 @@ along with GCC; see the file COPYING3. If not see printf(fmt, msg); ^~~ - For each of cases 1-3, if param_range is non-NULL, then it is used + For each of cases 1-3, if param_loc is not UNKNOWN_LOCATION, then it is used as a secondary range within the warning. For example, here it is used with case 1: @@ -100,7 +100,7 @@ along with GCC; see the file COPYING3. If not see ATTRIBUTE_GCC_DIAG (5,0) bool format_warning_va (const substring_loc &fmt_loc, - const source_range *param_range, + location_t param_loc, const char *corrected_substring, int opt, const char *gmsgid, va_list *ap) { @@ -136,13 +136,8 @@ format_warning_va (const substring_loc &fmt_loc, rich_location richloc (line_table, primary_loc); - 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); - } + if (param_loc != UNKNOWN_LOCATION) + richloc.add_range (param_loc, false); if (!err && corrected_substring && substring_within_range) richloc.add_fixit_replace (fmt_substring_range, corrected_substring); @@ -171,13 +166,13 @@ format_warning_va (const substring_loc &fmt_loc, bool format_warning_at_substring (const substring_loc &fmt_loc, - const source_range *param_range, + location_t param_loc, const char *corrected_substring, int opt, const char *gmsgid, ...) { va_list ap; va_start (ap, gmsgid); - bool warned = format_warning_va (fmt_loc, param_range, corrected_substring, + bool warned = format_warning_va (fmt_loc, param_loc, corrected_substring, opt, gmsgid, &ap); va_end (ap); diff --git a/gcc/substring-locations.h b/gcc/substring-locations.h index a91cc6c8b4a..3d7796db3e6 100644 --- a/gcc/substring-locations.h +++ b/gcc/substring-locations.h @@ -77,13 +77,13 @@ class substring_loc /* Functions for emitting a warning about a format string. */ extern bool format_warning_va (const substring_loc &fmt_loc, - const source_range *param_range, + location_t param_loc, const char *corrected_substring, int opt, const char *gmsgid, va_list *ap) ATTRIBUTE_GCC_DIAG (5,0); extern bool format_warning_at_substring (const substring_loc &fmt_loc, - const source_range *param_range, + location_t param_loc, const char *corrected_substring, int opt, const char *gmsgid, ...) ATTRIBUTE_GCC_DIAG (5,0); -- 2.30.2