This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[committed] Simplify format_warning_at_substring API
- From: David Malcolm <dmalcolm at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: David Malcolm <dmalcolm at redhat dot com>
- Date: Tue, 17 Oct 2017 15:43:26 -0400
- Subject: [committed] Simplify format_warning_at_substring API
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=dmalcolm at redhat dot com
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 31382E883
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.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Committed to trunk as r253827.
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.
---
gcc/c-family/c-format.c | 45 ++++++++++++++++-------------------
gcc/gimple-ssa-sprintf.c | 60 +++++++++++++++++++++--------------------------
gcc/substring-locations.c | 17 +++++---------
gcc/substring-locations.h | 4 ++--
4 files changed, 55 insertions(+), 71 deletions(-)
diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 0dba979..164d035 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<location_t> *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 7899e09..9770df7 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 %<INT_MAX%>",
dirlen,
@@ -2920,7 +2914,7 @@ format_directive (const pass_sprintf_length::call_info &info,
"bytes causes result to exceed %<INT_MAX%>")
: G_ ("%<%.*s%> directive output between %wu and %wu "
"bytes may cause result to exceed %<INT_MAX%>"));
- 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 433023d..095e5d0 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 a91cc6c..3d7796d 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);
--
1.8.5.3