* [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector
@ 2018-10-04 14:17 David Malcolm
2018-10-04 14:33 ` [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856) David Malcolm
2018-10-05 17:41 ` [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector Jeff Law
0 siblings, 2 replies; 10+ messages in thread
From: David Malcolm @ 2018-10-04 14:17 UTC (permalink / raw)
To: gcc-patches, Jason Merrill; +Cc: David Malcolm
Successfully regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/testsuite/ChangeLog:
* lib/multiline.exp (proc dg-end-multiline-output): Check argument
count. If there's a 3rd argument, use dg-process-target on it,
bailing out, or recording expected failures as "maybe_x".
(proc handle-multiline-outputs): Extract "maybe_x", and use it
to convert pass/fail into xpass/xfail.
---
gcc/testsuite/lib/multiline.exp | 30 +++++++++++++++++++++++++-----
1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/gcc/testsuite/lib/multiline.exp b/gcc/testsuite/lib/multiline.exp
index 5f8b62f..6c7ecdf 100644
--- a/gcc/testsuite/lib/multiline.exp
+++ b/gcc/testsuite/lib/multiline.exp
@@ -78,6 +78,8 @@ proc dg-begin-multiline-output { args } {
# Mark the end of an expected multiline output
# All lines up to here since the last dg-begin-multiline-output are
# expected to be seen.
+#
+# dg-end-multiline-output comment [{ target/xfail selector }]
proc dg-end-multiline-output { args } {
global _multiline_last_beginning_line
@@ -85,6 +87,23 @@ proc dg-end-multiline-output { args } {
set line [expr [lindex $args 0] - 1]
verbose "multiline output lines: $_multiline_last_beginning_line-$line" 3
+ if { [llength $args] > 3 } {
+ error "[lindex $args 0]: too many arguments"
+ return
+ }
+
+ set maybe_x ""
+ if { [llength $args] >= 3 } {
+ switch [dg-process-target [lindex $args 2]] {
+ "F" { set maybe_x "x" }
+ "P" { set maybe_x "" }
+ "N" {
+ # If we get "N", this output doesn't apply to us so ignore it.
+ return
+ }
+ }
+ }
+
upvar 1 prog prog
verbose "prog: $prog" 3
# "prog" now contains the filename
@@ -93,8 +112,8 @@ proc dg-end-multiline-output { args } {
set lines [_get_lines $prog $_multiline_last_beginning_line $line]
verbose "lines: $lines" 3
- # Create an entry of the form: first-line, last-line, lines
- set entry [list $_multiline_last_beginning_line $line $lines]
+ # Create an entry of the form: first-line, last-line, lines, maybe_x
+ set entry [list $_multiline_last_beginning_line $line $lines $maybe_x]
global multiline_expected_outputs
lappend multiline_expected_outputs $entry
verbose "within dg-end-multiline-output: multiline_expected_outputs: $multiline_expected_outputs" 3
@@ -118,6 +137,7 @@ proc handle-multiline-outputs { text } {
set start_line [lindex $entry 0]
set end_line [lindex $entry 1]
set multiline [lindex $entry 2]
+ set maybe_x [lindex $entry 3]
verbose " multiline: $multiline" 3
set rexp [_build_multiline_regex $multiline $index]
verbose "rexp: ${rexp}" 4
@@ -130,10 +150,10 @@ proc handle-multiline-outputs { text } {
# Use "regsub" to attempt to prune the pattern from $text
if {[regsub -line $rexp $text "" text]} {
- # Success; the multiline pattern was pruned.
- pass "$title was found: \"$escaped_regex\""
+ # The multiline pattern was pruned.
+ ${maybe_x}pass "$title was found: \"$escaped_regex\""
} else {
- fail "$title not found: \"$escaped_regex\""
+ ${maybe_x}fail "$title not found: \"$escaped_regex\""
}
set index [expr $index + 1]
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856)
2018-10-04 14:17 [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector David Malcolm
@ 2018-10-04 14:33 ` David Malcolm
2018-10-05 17:52 ` Jeff Law
2018-10-08 14:44 ` Jason Merrill
2018-10-05 17:41 ` [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector Jeff Law
1 sibling, 2 replies; 10+ messages in thread
From: David Malcolm @ 2018-10-04 14:33 UTC (permalink / raw)
To: gcc-patches, Jason Merrill; +Cc: David Malcolm
-Wformat in the C++ FE doesn't work as well as it could:
(a) it doesn't report precise locations within the string literal, and
(b) it doesn't underline arguments for those arguments !CAN_HAVE_LOCATION_P,
despite having location wrapper nodes.
For example:
Wformat-ranges.C:32:10: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
32 | printf("hello %s", 42);
| ^~~~~~~~~~
(a) is due to not wiring up the langhook for extracting substring
locations.
This patch uses the one in c-family; it also fixes string literal
parsing so that it records string concatenations (needed for
extracting substring locations from concatenated strings).
(b) is due to the call to maybe_constant_value here:
fargs[j] = maybe_constant_value (argarray[j]);
within build_over_call.
The patch fixes this by building a vec of location_t values when
calling check_function_arguments.
I attempted to eliminate the maybe_constant_value call here, but
it's needed by e.g. check_function_sentinel for detecting NULL,
and that code is in "c-family", so it can't simply call into
maybe_constant_value (which is in "cp").
With this patch, the output for the above example is improved to:
Wformat-ranges.C:32:18: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
32 | printf("hello %s", 42);
| ~^ ~~
| | |
| | int
| char*
| %d
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu (on top of
the multiline.exp patch).
OK for trunk?
gcc/cp/ChangeLog:
PR c++/56856
* call.c (build_over_call): Build a vec of locations of the
arguments before the call to maybe_constant_value, and pass to
check_function_arguments.
* cp-lang.c (LANG_HOOKS_GET_SUBSTRING_LOCATION): Define as
c_get_substring_location.
* parser.c (cp_parser_string_literal): Capture string
concatenation locations.
gcc/ChangeLog:
PR c++/56856
* input.c (expand_location_to_spelling_point): Add param "aspect"
and use rather than hardcoding LOCATION_ASPECT_CARET.
(get_substring_ranges_for_loc): Handle the case of a single token
within a macro expansion.
* input.h (expand_location_to_spelling_point): Add "aspect" param,
defaulting to LOCATION_ASPECT_CARET.
gcc/testsuite/ChangeLog:
PR c++/56856
* g++.dg/ext/builtin4.C: Set expected location for warning to the
correct location within the format string.
* g++.dg/plugin/plugin.exp (plugin_test_list): Add the plugin and
files for testing locations within string literal locations from
the C frontend.
* g++.dg/warn/Wformat-method.C: New test.
* g++.dg/warn/Wformat-pr71863.C: New test.
* g++.dg/warn/Wformat-ranges-c++11.C: New test.
* g++.dg/warn/Wformat-ranges.C: New test, based on
gcc.dg/format/diagnostic-ranges.c.
* gcc.dg/plugin/diagnostic-test-string-literals-1.c
(test_multitoken_macro): Generalize expected output to work with
both C and C++.
* gcc.dg/plugin/diagnostic-test-string-literals-2.c
(test_stringified_token_1): Likewise.
(test_stringified_token_3): Likewise.
---
gcc/cp/call.c | 4 +-
gcc/cp/cp-lang.c | 3 +
gcc/cp/parser.c | 14 +-
gcc/input.c | 44 ++-
gcc/input.h | 5 +-
gcc/testsuite/g++.dg/ext/builtin4.C | 2 +-
gcc/testsuite/g++.dg/plugin/plugin.exp | 5 +
gcc/testsuite/g++.dg/warn/Wformat-method.C | 40 +++
gcc/testsuite/g++.dg/warn/Wformat-pr71863.C | 33 ++
gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C | 18 +
gcc/testsuite/g++.dg/warn/Wformat-ranges.C | 374 +++++++++++++++++++++
gcc/testsuite/gcc.dg/format/diagnostic-ranges.c | 11 +-
.../plugin/diagnostic-test-string-literals-1.c | 6 +-
.../plugin/diagnostic-test-string-literals-2.c | 4 +-
14 files changed, 537 insertions(+), 26 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wformat-method.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wformat-pr71863.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C
create mode 100644 gcc/testsuite/g++.dg/warn/Wformat-ranges.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index b2ca667..747f837 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8188,6 +8188,7 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
{
tree *fargs = (!nargs ? argarray
: (tree *) alloca (nargs * sizeof (tree)));
+ auto_vec<location_t> arglocs (nargs);
for (j = 0; j < nargs; j++)
{
/* For -Wformat undo the implicit passing by hidden reference
@@ -8197,10 +8198,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
fargs[j] = TREE_OPERAND (argarray[j], 0);
else
fargs[j] = maybe_constant_value (argarray[j]);
+ arglocs.quick_push (EXPR_LOC_OR_LOC (argarray[j], input_location));
}
warned_p = check_function_arguments (input_location, fn, TREE_TYPE (fn),
- nargs, fargs, NULL);
+ nargs, fargs, &arglocs);
}
if (DECL_INHERITED_CTOR (fn))
diff --git a/gcc/cp/cp-lang.c b/gcc/cp/cp-lang.c
index 26a1e6d..a0b0102 100644
--- a/gcc/cp/cp-lang.c
+++ b/gcc/cp/cp-lang.c
@@ -83,6 +83,9 @@ static tree cxx_enum_underlying_base_type (const_tree);
#define LANG_HOOKS_RUN_LANG_SELFTESTS selftest::run_cp_tests
#endif /* #if CHECKING_P */
+#undef LANG_HOOKS_GET_SUBSTRING_LOCATION
+#define LANG_HOOKS_GET_SUBSTRING_LOCATION c_get_substring_location
+
/* Each front end provides its own lang hook initializer. */
struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6696f17..3a48ebf 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -4034,6 +4034,7 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
tree value;
size_t count;
struct obstack str_ob;
+ struct obstack loc_ob;
cpp_string str, istr, *strs;
cp_token *tok;
enum cpp_ttype type, curr_type;
@@ -4090,6 +4091,7 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
{
location_t last_tok_loc = tok->location;
gcc_obstack_init (&str_ob);
+ gcc_obstack_init (&loc_ob);
count = 0;
do
@@ -4135,6 +4137,7 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
}
obstack_grow (&str_ob, &str, sizeof (cpp_string));
+ obstack_grow (&loc_ob, &tok->location, sizeof (location_t));
last_tok_loc = tok->location;
@@ -4173,6 +4176,12 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
{
value = build_string (istr.len, (const char *)istr.text);
free (CONST_CAST (unsigned char *, istr.text));
+ if (count > 1)
+ {
+ location_t *locs = (location_t *)obstack_finish (&loc_ob);
+ gcc_assert (g_string_concat_db);
+ g_string_concat_db->record_string_concatenation (count, locs);
+ }
switch (type)
{
@@ -4209,7 +4218,10 @@ cp_parser_string_literal (cp_parser *parser, bool translate, bool wide_ok,
value = error_mark_node;
if (count > 1)
- obstack_free (&str_ob, 0);
+ {
+ obstack_free (&str_ob, 0);
+ obstack_free (&loc_ob, 0);
+ }
return cp_expr (value, loc);
}
diff --git a/gcc/input.c b/gcc/input.c
index d65a82d..b4b7136 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -813,10 +813,10 @@ expand_location (source_location loc)
"<built-in>". */
expanded_location
-expand_location_to_spelling_point (source_location loc)
+expand_location_to_spelling_point (source_location loc,
+ enum location_aspect aspect)
{
- return expand_location_1 (loc, /*expansion_point_p=*/false,
- LOCATION_ASPECT_CARET);
+ return expand_location_1 (loc, /*expansion_point_p=*/false, aspect);
}
/* The rich_location class within libcpp requires a way to expand
@@ -1397,24 +1397,32 @@ get_substring_ranges_for_loc (cpp_reader *pfile,
source_range src_range = get_range_from_loc (line_table, strlocs[i]);
if (src_range.m_start >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
- /* If the string is within a macro expansion, we can't get at the
- end location. */
- return "macro expansion";
-
- if (src_range.m_start >= LINE_MAP_MAX_LOCATION_WITH_COLS)
- /* If so, we can't reliably determine where the token started within
- its line. */
- return "range starts after LINE_MAP_MAX_LOCATION_WITH_COLS";
-
- if (src_range.m_finish >= LINE_MAP_MAX_LOCATION_WITH_COLS)
- /* If so, we can't reliably determine where the token finished within
- its line. */
- return "range ends after LINE_MAP_MAX_LOCATION_WITH_COLS";
+ {
+ /* If the string token was within a macro expansion, then we can
+ cope with it for the simple case where we have a single token.
+ Otherwise, bail out. */
+ if (src_range.m_start != src_range.m_finish)
+ return "macro expansion";
+ }
+ else
+ {
+ if (src_range.m_start >= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ /* If so, we can't reliably determine where the token started within
+ its line. */
+ return "range starts after LINE_MAP_MAX_LOCATION_WITH_COLS";
+
+ if (src_range.m_finish >= LINE_MAP_MAX_LOCATION_WITH_COLS)
+ /* If so, we can't reliably determine where the token finished
+ within its line. */
+ return "range ends after LINE_MAP_MAX_LOCATION_WITH_COLS";
+ }
expanded_location start
- = expand_location_to_spelling_point (src_range.m_start);
+ = expand_location_to_spelling_point (src_range.m_start,
+ LOCATION_ASPECT_START);
expanded_location finish
- = expand_location_to_spelling_point (src_range.m_finish);
+ = expand_location_to_spelling_point (src_range.m_finish,
+ LOCATION_ASPECT_FINISH);
if (start.file != finish.file)
return "range endpoints are in different files";
if (start.line != finish.line)
diff --git a/gcc/input.h b/gcc/input.h
index 4619663..e5d5a09 100644
--- a/gcc/input.h
+++ b/gcc/input.h
@@ -85,7 +85,10 @@ class char_span
extern char_span location_get_source_line (const char *file_path, int line);
extern bool location_missing_trailing_newline (const char *file_path);
-extern expanded_location expand_location_to_spelling_point (source_location);
+extern expanded_location
+expand_location_to_spelling_point (source_location,
+ enum location_aspect aspect
+ = LOCATION_ASPECT_CARET);
extern source_location expansion_point_location_if_in_system_header (source_location);
extern source_location expansion_point_location (source_location);
diff --git a/gcc/testsuite/g++.dg/ext/builtin4.C b/gcc/testsuite/g++.dg/ext/builtin4.C
index 8804b53..6691434 100644
--- a/gcc/testsuite/g++.dg/ext/builtin4.C
+++ b/gcc/testsuite/g++.dg/ext/builtin4.C
@@ -6,5 +6,5 @@
extern "C" int printf(const char*,...);
void foo() {
- printf("%d"); // { dg-warning "expects a matching" }
+ printf("%d"); // { dg-warning "12: expects a matching" }
}
diff --git a/gcc/testsuite/g++.dg/plugin/plugin.exp b/gcc/testsuite/g++.dg/plugin/plugin.exp
index d9f54ab..e7c3b69 100644
--- a/gcc/testsuite/g++.dg/plugin/plugin.exp
+++ b/gcc/testsuite/g++.dg/plugin/plugin.exp
@@ -67,6 +67,11 @@ set plugin_test_list [list \
diagnostic-test-expressions-1.C } \
{ ../../gcc.dg/plugin/diagnostic_plugin_test_inlining.c \
diagnostic-test-inlining-1.C } \
+ { ../../gcc.dg/plugin/diagnostic_plugin_test_string_literals.c \
+ ../../gcc.dg/plugin/diagnostic-test-string-literals-1.c \
+ ../../gcc.dg/plugin/diagnostic-test-string-literals-2.c \
+ ../../gcc.dg/plugin/diagnostic-test-string-literals-3.c \
+ ../../gcc.dg/plugin/diagnostic-test-string-literals-4.c } \
{ show_template_tree_color_plugin.c \
show-template-tree-color.C \
show-template-tree-color-labels.C \
diff --git a/gcc/testsuite/g++.dg/warn/Wformat-method.C b/gcc/testsuite/g++.dg/warn/Wformat-method.C
new file mode 100644
index 0000000..8494bf1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wformat-method.C
@@ -0,0 +1,40 @@
+// { dg-options "-Wformat -fdiagnostics-show-caret" }
+
+class logger
+{
+public:
+ void log (const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+};
+
+void test ()
+{
+ logger out;
+ out.log ("before %s after", 42); // { dg-warning "argument 3 has type 'int'" }
+ /* { dg-begin-multiline-output "" }
+ out.log ("before %s after", 42);
+ ~^ ~~
+ | |
+ char* int
+ %d
+ { dg-end-multiline-output "" } */
+}
+
+template <typename T>
+class logger_2
+{
+public:
+ void log (const char *fmt, ...) __attribute__((format (printf, 2, 3)));
+};
+
+void test_2 ()
+{
+ logger_2<int> out;
+ out.log ("before %s after", 42); // { dg-warning "argument 3 has type 'int'" }
+ /* { dg-begin-multiline-output "" }
+ out.log ("before %s after", 42);
+ ~^ ~~
+ | |
+ char* int
+ %d
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wformat-pr71863.C b/gcc/testsuite/g++.dg/warn/Wformat-pr71863.C
new file mode 100644
index 0000000..537274c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wformat-pr71863.C
@@ -0,0 +1,33 @@
+// { dg-options "-Wformat -fdiagnostics-show-caret" }
+
+void test_1 (void)
+{
+ __builtin_printf ("%s%s", 42, 43); // { dg-warning "argument 2 has type 'int'" }
+ // { dg-warning "argument 3 has type 'int'" "" { target *-*-* } .-1 }
+ /* { dg-begin-multiline-output "" }
+ __builtin_printf ("%s%s", 42, 43);
+ ~^ ~~
+ | |
+ char* int
+ %d
+ { dg-end-multiline-output "" } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_printf ("%s%s", 42, 43);
+ ~^ ~~
+ | |
+ char* int
+ %d
+ { dg-end-multiline-output "" } */
+}
+
+void test_2 (void)
+{
+ __builtin_printf ("before %s after", 6 * 7); // { dg-warning "argument 2 has type 'int'" }
+ /* { dg-begin-multiline-output "" }
+ __builtin_printf ("before %s after", 6 * 7);
+ ~^ ~~~~~
+ | |
+ char* int
+ %d
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C b/gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C
new file mode 100644
index 0000000..a4d3fff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wformat-ranges-c++11.C
@@ -0,0 +1,18 @@
+// { dg-do compile { target c++11 } }
+/* { dg-options "-Wformat -fdiagnostics-show-caret" } */
+
+/* C++11-specific format tests. */
+
+#define printf __builtin_printf
+
+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\\*' " } */
+/* { dg-begin-multiline-output "" }
+ printf(u8"hello %i", msg);
+ ~^ ~~~
+ | |
+ int const char*
+ %s
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/g++.dg/warn/Wformat-ranges.C b/gcc/testsuite/g++.dg/warn/Wformat-ranges.C
new file mode 100644
index 0000000..96dfa97
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wformat-ranges.C
@@ -0,0 +1,374 @@
+/* { dg-options "-Wformat -fdiagnostics-show-caret" } */
+
+/* This is a copy of gcc.dg/format/diagnostic-ranges.c
+ with the following changes:
+ - removal of "format.h"
+ - "char \\*" -> "char\\*" (space removal)
+ - move of test_u8 to Wformat-ranges-c++11.C. */
+
+#define printf __builtin_printf
+typedef __SIZE_TYPE__ size_t;
+typedef __SIZE_TYPE__ ssize_t;
+
+extern ssize_t strfmon (char *__restrict __s, size_t __maxsize,
+ const char *__restrict, ...)
+ __attribute__ ((__format__ (__strfmon__, 3, 4)));
+
+/* See PR 52952. */
+
+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\\*' " } */
+
+/* { dg-begin-multiline-output "" }
+ printf("hello %i", msg);
+ ~^ ~~~
+ | |
+ int const char*
+ %s
+ { dg-end-multiline-output "" } */
+
+
+ printf("hello %s", 42); /* { dg-warning "format '%s' expects argument of type 'char\\*', but argument 2 has type 'int'" } */
+/* { dg-begin-multiline-output "" }
+ printf("hello %s", 42);
+ ~^ ~~
+ | |
+ | int
+ char*
+ %d
+ { dg-end-multiline-output "" } */
+
+ printf("hello %i", (long)0); /* { dg-warning "format '%i' expects argument of type 'int', but argument 2 has type 'long int' " } */
+/* { dg-begin-multiline-output "" }
+ printf("hello %i", (long)0);
+ ~^ ~~~~~~~
+ | |
+ int long int
+ %li
+ { 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);
+/* { dg-begin-multiline-output "" }
+ printf ("arg0: %i arg1: %s arg 2: %i",
+ ~^
+ |
+ char*
+ %d
+ 100, 101, 102);
+ ~~~
+ |
+ int
+ { 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",
+ ~^
+ |
+ char*
+ %d
+ 100, i + j, 102);
+ ~~~~~
+ |
+ int
+ { dg-end-multiline-output "" } */
+}
+
+void multiline_format_string (void) {
+ printf ("before the fmt specifier"
+ "%"
+ "d" /* { dg-warning "12: format '%d' expects a matching 'int' argument" } */
+ "after the fmt specifier");
+/* { dg-begin-multiline-output "" }
+ "%"
+ ~~
+ "d"
+ ~^
+ |
+ int
+ { 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\\*' " } */
+
+/* { dg-begin-multiline-output "" }
+ printf("hello \x25\x69", msg);
+ ~~~~^~~~ ~~~
+ | |
+ int const char*
+ \x25s
+ { 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\\*' " } */
+
+/* { dg-begin-multiline-output "" }
+ printf("hello \045\151", msg);
+ ~~~~^~~~ ~~~
+ | |
+ int const char*
+ \045s
+ { 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",
+ ~~~~~~~~^~~~
+ |
+ int
+ \x25" "s
+ msg);
+ ~~~
+ |
+ const char*
+ { 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);
+ ~^ ~~~~~~~~~~~~~~~
+ | |
+ char* long int
+ %ld
+ { dg-end-multiline-output "" } */
+}
+
+void test_field_width_specifier (long l, int i1, int i2)
+{
+ printf (" %*.*d ", l, i1, i2); /* { dg-warning "14: field width specifier '\\*' expects argument of type 'int', but argument 2 has type 'long int'" } */
+/* { dg-begin-multiline-output "" }
+ printf (" %*.*d ", l, i1, i2);
+ ~^~~~ ~
+ | |
+ int long int
+ { dg-end-multiline-output "" } */
+}
+
+/* PR c/72857. */
+
+void test_field_width_specifier_2 (char *d, long foo, long bar)
+{
+ __builtin_sprintf (d, " %*ld ", foo, foo); /* { dg-warning "28: field width specifier '\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_sprintf (d, " %*ld ", foo, foo);
+ ~^~~ ~~~
+ | |
+ int long int
+ { dg-end-multiline-output "" } */
+
+ __builtin_sprintf (d, " %*ld ", foo + bar, foo); /* { dg-warning "28: field width specifier '\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_sprintf (d, " %*ld ", foo + bar, foo);
+ ~^~~ ~~~~~~~~~
+ | |
+ int long int
+ { dg-end-multiline-output "" } */
+}
+
+void test_field_precision_specifier (char *d, long foo, long bar)
+{
+ __builtin_sprintf (d, " %.*ld ", foo, foo); /* { dg-warning "29: field precision specifier '\\.\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_sprintf (d, " %.*ld ", foo, foo);
+ ~~^~~ ~~~
+ | |
+ int long int
+ { dg-end-multiline-output "" } */
+
+ __builtin_sprintf (d, " %.*ld ", foo + bar, foo); /* { dg-warning "29: field precision specifier '\\.\\*' expects argument of type 'int', but argument 3 has type 'long int'" } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_sprintf (d, " %.*ld ", foo + bar, foo);
+ ~~^~~ ~~~~~~~~~
+ | |
+ int long int
+ { 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 "13: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);
+ ^~~~~~~~~~~~~~~~~~~~~~~~~ ~~~
+ |
+ const char*
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define INT_FMT "%i"
+ ~^
+ |
+ int
+ %s
+ { dg-end-multiline-output "" } */
+#undef INT_FMT
+}
+
+void test_macro_2 (const char *msg)
+{
+#define PRIu32 "u" /* { dg-message "17: format string is defined here" } */
+ printf("hello %" PRIu32 " world", msg); /* { dg-warning "10: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'const char\\*' " } */
+/* { dg-begin-multiline-output "" }
+ printf("hello %" PRIu32 " world", msg);
+ ^~~~~~~~~~~~~~~~~~~~~~~~~ ~~~
+ |
+ const char*
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define PRIu32 "u"
+ ^
+ |
+ unsigned int
+ { dg-end-multiline-output "" } */
+#undef PRIu32
+}
+
+void test_macro_3 (const char *msg)
+{
+#define FMT_STRING "hello %i world" // { dg-line test_macro_3_macro_line }
+/* { dg-warning "20: format '%i' expects argument of type 'int', but argument 2 has type 'const char\\*'" "" { target *-*-* } .-1 } */
+ printf(FMT_STRING, msg); /* { dg-message "10: in expansion of macro 'FMT_STRING" } */
+/* { dg-begin-multiline-output "" }
+ #define FMT_STRING "hello %i world"
+ ^~~~~~~~~~~~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ printf(FMT_STRING, msg);
+ ^~~~~~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-message "28: format string is defined here" "" { target *-*-* } test_macro_3_macro_line } */
+/* { dg-begin-multiline-output "" }
+ #define FMT_STRING "hello %i world"
+ ~^
+ |
+ int
+ %s
+ { dg-end-multiline-output "" } */
+#undef FMT_STRING
+}
+
+void test_macro_4 (const char *msg)
+{
+#define FMT_STRING "hello %i world" /* { dg-warning "20: format '%i' expects argument of type 'int', but argument 2 has type 'const char\\*' " } */
+ printf(FMT_STRING "\n", msg); /* { dg-message "10: in expansion of macro 'FMT_STRING" } */
+/* { dg-begin-multiline-output "" }
+ #define FMT_STRING "hello %i world"
+ ^
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ printf(FMT_STRING "\n", msg);
+ ^~~~~~~~~~
+ { dg-end-multiline-output "" } */
+/* { dg-begin-multiline-output "" }
+ #define FMT_STRING "hello %i world"
+ ~^
+ |
+ int
+ %s
+ { dg-end-multiline-output "" } */
+#undef FMT_STRING
+}
+
+void test_non_contiguous_strings (void)
+{
+ __builtin_printf(" %" "d ", 0.5); /* { dg-warning "26: format .%d. expects argument of type .int., but argument 2 has type .double." } */
+ /* { dg-begin-multiline-output "" }
+ __builtin_printf(" %" "d ", 0.5);
+ ~~~~^ ~~~
+ | |
+ int double
+ %" "f
+ { 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);
+ ^ ~~~
+ |
+ double
+ { dg-end-multiline-output "" } */
+}
diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
index 84535f0..2c33ce2 100644
--- a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
+++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
@@ -314,7 +314,8 @@ void test_macro_2 (const char *msg)
void test_macro_3 (const char *msg)
{
-#define FMT_STRING "hello %i world" /* { dg-warning "20: format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*' " } */
+#define FMT_STRING "hello %i world" /* { dg-line test_macro_3_macro_line } */
+ /* { dg-warning "20: format '%i' expects argument of type 'int', but argument 2 has type 'const char \\*'" "" { target *-*-*} .-1 } */
printf(FMT_STRING, msg); /* { dg-message "10: in expansion of macro 'FMT_STRING" } */
/* { dg-begin-multiline-output "" }
#define FMT_STRING "hello %i world"
@@ -324,6 +325,14 @@ void test_macro_3 (const char *msg)
printf(FMT_STRING, msg);
^~~~~~~~~~
{ dg-end-multiline-output "" } */
+/* { dg-message "28: format string is defined here" "" { target *-*-* } test_macro_3_macro_line } */
+/* { dg-begin-multiline-output "" }
+ #define FMT_STRING "hello %i world"
+ ~^
+ |
+ int
+ %s
+ { dg-end-multiline-output "" } */
#undef FMT_STRING
}
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
index bea86af..73770aa 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c
@@ -251,7 +251,11 @@ test_multitoken_macro (void)
/* { dg-begin-multiline-output "" }
#define RANGE ("0123456789")
^~~~~~~~~~~~~~
- { dg-end-multiline-output "" } */
+ { dg-end-multiline-output "" { target c } } */
+/* { dg-begin-multiline-output "" }
+ #define RANGE ("0123456789")
+ ~^~~~~~~~~~~~~
+ { dg-end-multiline-output "" { target c++ } } */
/* { dg-begin-multiline-output "" }
__emit_string_literal_range (RANGE, 4, 3, 6);
^~~~~
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-2.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-2.c
index e9d98f4..260c47d 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-2.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-2.c
@@ -12,7 +12,7 @@ test_stringified_token_1 (int x)
{
#define STRINGIFY(EXPR) #EXPR
- __emit_string_literal_range (STRINGIFY(x > 0), /* { dg-error "unable to read substring location: macro expansion" } */
+ __emit_string_literal_range (STRINGIFY(x > 0), /* { dg-error "macro expansion|cpp_interpret_string_1 failed" } */
0, 0, 4);
#undef STRINGIFY
@@ -43,7 +43,7 @@ test_stringified_token_3 (int x)
#define XSTR(s) STR(s)
#define STR(s) #s
#define FOO 123456789
- __emit_string_literal_range (XSTR (FOO), /* { dg-error "unable to read substring location: macro expansion" } */
+ __emit_string_literal_range (XSTR (FOO), /* { dg-error "macro expansion|cpp_interpret_string_1 failed" } */
2, 2, 3);
#undef XSTR
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector
2018-10-04 14:17 [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector David Malcolm
2018-10-04 14:33 ` [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856) David Malcolm
@ 2018-10-05 17:41 ` Jeff Law
1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2018-10-05 17:41 UTC (permalink / raw)
To: David Malcolm, gcc-patches, Jason Merrill
On 10/4/18 9:00 AM, David Malcolm wrote:
> Successfully regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
>
> gcc/testsuite/ChangeLog:
> * lib/multiline.exp (proc dg-end-multiline-output): Check argument
> count. If there's a 3rd argument, use dg-process-target on it,
> bailing out, or recording expected failures as "maybe_x".
> (proc handle-multiline-outputs): Extract "maybe_x", and use it
> to convert pass/fail into xpass/xfail.
OK
jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856)
2018-10-04 14:33 ` [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856) David Malcolm
@ 2018-10-05 17:52 ` Jeff Law
2018-10-08 14:44 ` Jason Merrill
1 sibling, 0 replies; 10+ messages in thread
From: Jeff Law @ 2018-10-05 17:52 UTC (permalink / raw)
To: David Malcolm, gcc-patches, Jason Merrill
On 10/4/18 9:00 AM, David Malcolm wrote:
> -Wformat in the C++ FE doesn't work as well as it could:
> (a) it doesn't report precise locations within the string literal, and
> (b) it doesn't underline arguments for those arguments !CAN_HAVE_LOCATION_P,
> despite having location wrapper nodes.
>
> For example:
>
> Wformat-ranges.C:32:10: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
> 32 | printf("hello %s", 42);
> | ^~~~~~~~~~
>
> (a) is due to not wiring up the langhook for extracting substring
> locations.
>
> This patch uses the one in c-family; it also fixes string literal
> parsing so that it records string concatenations (needed for
> extracting substring locations from concatenated strings).
>
> (b) is due to the call to maybe_constant_value here:
> fargs[j] = maybe_constant_value (argarray[j]);
> within build_over_call.
>
> The patch fixes this by building a vec of location_t values when
> calling check_function_arguments.
> I attempted to eliminate the maybe_constant_value call here, but
> it's needed by e.g. check_function_sentinel for detecting NULL,
> and that code is in "c-family", so it can't simply call into
> maybe_constant_value (which is in "cp").
>
> With this patch, the output for the above example is improved to:
>
> Wformat-ranges.C:32:18: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
> 32 | printf("hello %s", 42);
> | ~^ ~~
> | | |
> | | int
> | char*
> | %d
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu (on top of
> the multiline.exp patch).
>
> OK for trunk?
>
> gcc/cp/ChangeLog:
> PR c++/56856
> * call.c (build_over_call): Build a vec of locations of the
> arguments before the call to maybe_constant_value, and pass to
> check_function_arguments.
> * cp-lang.c (LANG_HOOKS_GET_SUBSTRING_LOCATION): Define as
> c_get_substring_location.
> * parser.c (cp_parser_string_literal): Capture string
> concatenation locations.
>
> gcc/ChangeLog:
> PR c++/56856
> * input.c (expand_location_to_spelling_point): Add param "aspect"
> and use rather than hardcoding LOCATION_ASPECT_CARET.
> (get_substring_ranges_for_loc): Handle the case of a single token
> within a macro expansion.
> * input.h (expand_location_to_spelling_point): Add "aspect" param,
> defaulting to LOCATION_ASPECT_CARET.
>
> gcc/testsuite/ChangeLog:
> PR c++/56856
> * g++.dg/ext/builtin4.C: Set expected location for warning to the
> correct location within the format string.
> * g++.dg/plugin/plugin.exp (plugin_test_list): Add the plugin and
> files for testing locations within string literal locations from
> the C frontend.
> * g++.dg/warn/Wformat-method.C: New test.
> * g++.dg/warn/Wformat-pr71863.C: New test.
> * g++.dg/warn/Wformat-ranges-c++11.C: New test.
> * g++.dg/warn/Wformat-ranges.C: New test, based on
> gcc.dg/format/diagnostic-ranges.c.
> * gcc.dg/plugin/diagnostic-test-string-literals-1.c
> (test_multitoken_macro): Generalize expected output to work with
> both C and C++.
> * gcc.dg/plugin/diagnostic-test-string-literals-2.c
> (test_stringified_token_1): Likewise.
> (test_stringified_token_3): Likewise.
I typically leave the C++ bits to others, but this looks fine to me.
jeff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856)
2018-10-04 14:33 ` [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856) David Malcolm
2018-10-05 17:52 ` Jeff Law
@ 2018-10-08 14:44 ` Jason Merrill
2018-10-08 21:46 ` [PATCH] Folding and check_function_arguments David Malcolm
1 sibling, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2018-10-08 14:44 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches List
On Thu, Oct 4, 2018 at 10:12 AM David Malcolm <dmalcolm@redhat.com> wrote:
>
> -Wformat in the C++ FE doesn't work as well as it could:
> (a) it doesn't report precise locations within the string literal, and
> (b) it doesn't underline arguments for those arguments !CAN_HAVE_LOCATION_P,
> despite having location wrapper nodes.
>
> For example:
>
> Wformat-ranges.C:32:10: warning: format '%s' expects argument of type 'char*', but argument 2 has type 'int' [-Wformat=]
> 32 | printf("hello %s", 42);
> | ^~~~~~~~~~
>
> (a) is due to not wiring up the langhook for extracting substring
> locations.
>
> This patch uses the one in c-family; it also fixes string literal
> parsing so that it records string concatenations (needed for
> extracting substring locations from concatenated strings).
>
> (b) is due to the call to maybe_constant_value here:
> fargs[j] = maybe_constant_value (argarray[j]);
> within build_over_call.
Maybe we should remove that in favor of fold_for_warn in
check_function_arguments.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] Folding and check_function_arguments
2018-10-08 14:44 ` Jason Merrill
@ 2018-10-08 21:46 ` David Malcolm
2018-10-25 19:08 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: David Malcolm @ 2018-10-08 21:46 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, David Malcolm
On Mon, 2018-10-08 at 10:37 -0400, Jason Merrill wrote:
> On Thu, Oct 4, 2018 at 10:12 AM David Malcolm <dmalcolm@redhat.com>
> wrote:
> >
> > -Wformat in the C++ FE doesn't work as well as it could:
> > (a) it doesn't report precise locations within the string literal,
> > and
> > (b) it doesn't underline arguments for those arguments
> > !CAN_HAVE_LOCATION_P,
> > despite having location wrapper nodes.
> >
> > For example:
> >
> > Wformat-ranges.C:32:10: warning: format '%s' expects argument of
> > type 'char*', but argument 2 has type 'int' [-Wformat=]
> > 32 | printf("hello %s", 42);
> > | ^~~~~~~~~~
> >
> > (a) is due to not wiring up the langhook for extracting substring
> > locations.
> >
> > This patch uses the one in c-family; it also fixes string
> > literal
> > parsing so that it records string concatenations (needed for
> > extracting substring locations from concatenated strings).
> >
> > (b) is due to the call to maybe_constant_value here:
> > fargs[j] = maybe_constant_value (argarray[j]);
> > within build_over_call.
>
> Maybe we should remove that in favor of fold_for_warn in
> check_function_arguments.
>
> Jason
This patch eliminates the arglocs array I introduced to build_over_call
in r264887, and eliminates the call to maybe_constant_value when building
"fargs" (thus retaining location wrapper nodes).
Instead, this patch requires that any checks within
check_function_arguments that need folded arguments do their own folding.
Of the various checks:
(a) check_function_nonnull already calls fold_for_warn,
(b) check_function_format doesn't need folding
(c) check_function_sentinel needs fold_for_warn in one place, which the
patch adds, and
(d) check_function_restrict needs per-argument folding, which the patch
adds. Given that it scans before and after resetting TREE_VISITED on
each argument, it seemed best to make a copy of the array, folding each
argument from the outset, rather than repeatedly calling fold_for_warn;
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for trunk?
gcc/c-family/ChangeLog:
PR c++/56856
* c-common.c (check_function_sentinel): Call fold_for_warn on the
argument.
(check_function_restrict): Rename param "argarray" to
"unfolded_argarray", and make a copy named "argarray", calling
fold_for_warn on each argument.
(check_function_arguments): Add note about responsibility for
folding the arguments.
gcc/cp/ChangeLog:
PR c++/56856
* call.c (build_over_call): Eliminate the "arglocs" array, and the
call to maybe_constant_value when building "fargs".
---
gcc/c-family/c-common.c | 17 +++++++++++++----
gcc/cp/call.c | 6 ++----
2 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index c0198e1..11fa360 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -5297,7 +5297,7 @@ check_function_sentinel (const_tree fntype, int nargs, tree *argarray)
}
/* Validate the sentinel. */
- sentinel = argarray[nargs - 1 - pos];
+ sentinel = fold_for_warn (argarray[nargs - 1 - pos]);
if ((!POINTER_TYPE_P (TREE_TYPE (sentinel))
|| !integer_zerop (sentinel))
/* Although __null (in C++) is only an integer we allow it
@@ -5316,11 +5316,16 @@ check_function_sentinel (const_tree fntype, int nargs, tree *argarray)
static bool
check_function_restrict (const_tree fndecl, const_tree fntype,
- int nargs, tree *argarray)
+ int nargs, tree *unfolded_argarray)
{
int i;
tree parms = TYPE_ARG_TYPES (fntype);
+ /* Call fold_for_warn on all of the arguments. */
+ auto_vec<tree> argarray (nargs);
+ for (i = 0; i < nargs; i++)
+ argarray.quick_push (fold_for_warn (unfolded_argarray[i]));
+
if (fndecl
&& TREE_CODE (fndecl) == FUNCTION_DECL)
{
@@ -5357,7 +5362,7 @@ check_function_restrict (const_tree fndecl, const_tree fntype,
if (POINTER_TYPE_P (type)
&& TYPE_RESTRICT (type)
&& !TYPE_READONLY (TREE_TYPE (type)))
- warned |= warn_for_restrict (i, argarray, nargs);
+ warned |= warn_for_restrict (i, argarray.address (), nargs);
}
for (i = 0; i < nargs; i++)
@@ -5604,7 +5609,11 @@ attribute_fallthrough_p (tree attr)
/* Check for valid arguments being passed to a function with FNTYPE.
There are NARGS arguments in the array ARGARRAY. LOC should be used
for diagnostics. Return true if either -Wnonnull or -Wrestrict has
- been issued. */
+ been issued.
+
+ The arguments in ARGARRAY may not have been folded yet (e.g. for C++,
+ to preserve location wrappers); checks that require folded arguments
+ should call fold_for_warn on them. */
bool
check_function_arguments (location_t loc, const_tree fndecl, const_tree fntype,
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 747f837..51da771 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8188,7 +8188,6 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
{
tree *fargs = (!nargs ? argarray
: (tree *) alloca (nargs * sizeof (tree)));
- auto_vec<location_t> arglocs (nargs);
for (j = 0; j < nargs; j++)
{
/* For -Wformat undo the implicit passing by hidden reference
@@ -8197,12 +8196,11 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
&& TYPE_REF_P (TREE_TYPE (argarray[j])))
fargs[j] = TREE_OPERAND (argarray[j], 0);
else
- fargs[j] = maybe_constant_value (argarray[j]);
- arglocs.quick_push (EXPR_LOC_OR_LOC (argarray[j], input_location));
+ fargs[j] = argarray[j];
}
warned_p = check_function_arguments (input_location, fn, TREE_TYPE (fn),
- nargs, fargs, &arglocs);
+ nargs, fargs, NULL);
}
if (DECL_INHERITED_CTOR (fn))
--
1.8.5.3
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Folding and check_function_arguments
2018-10-08 21:46 ` [PATCH] Folding and check_function_arguments David Malcolm
@ 2018-10-25 19:08 ` Jason Merrill
2018-10-29 12:16 ` Alexander Monakov
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2018-10-25 19:08 UTC (permalink / raw)
To: David Malcolm; +Cc: gcc-patches
On 10/8/18 6:24 PM, David Malcolm wrote:
> On Mon, 2018-10-08 at 10:37 -0400, Jason Merrill wrote:
>> On Thu, Oct 4, 2018 at 10:12 AM David Malcolm <dmalcolm@redhat.com>
>> wrote:
>>>
>>> -Wformat in the C++ FE doesn't work as well as it could:
>>> (a) it doesn't report precise locations within the string literal,
>>> and
>>> (b) it doesn't underline arguments for those arguments
>>> !CAN_HAVE_LOCATION_P,
>>> despite having location wrapper nodes.
>>>
>>> For example:
>>>
>>> Wformat-ranges.C:32:10: warning: format '%s' expects argument of
>>> type 'char*', but argument 2 has type 'int' [-Wformat=]
>>> 32 | printf("hello %s", 42);
>>> | ^~~~~~~~~~
>>>
>>> (a) is due to not wiring up the langhook for extracting substring
>>> locations.
>>>
>>> This patch uses the one in c-family; it also fixes string
>>> literal
>>> parsing so that it records string concatenations (needed for
>>> extracting substring locations from concatenated strings).
>>>
>>> (b) is due to the call to maybe_constant_value here:
>>> fargs[j] = maybe_constant_value (argarray[j]);
>>> within build_over_call.
>>
>> Maybe we should remove that in favor of fold_for_warn in
>> check_function_arguments.
>>
>> Jason
>
> This patch eliminates the arglocs array I introduced to build_over_call
> in r264887, and eliminates the call to maybe_constant_value when building
> "fargs" (thus retaining location wrapper nodes).
>
> Instead, this patch requires that any checks within
> check_function_arguments that need folded arguments do their own folding.
>
> Of the various checks:
> (a) check_function_nonnull already calls fold_for_warn,
> (b) check_function_format doesn't need folding
> (c) check_function_sentinel needs fold_for_warn in one place, which the
> patch adds, and
> (d) check_function_restrict needs per-argument folding, which the patch
> adds. Given that it scans before and after resetting TREE_VISITED on
> each argument, it seemed best to make a copy of the array, folding each
> argument from the outset, rather than repeatedly calling fold_for_warn;
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> OK for trunk?
OK, thanks.
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Folding and check_function_arguments
2018-10-25 19:08 ` Jason Merrill
@ 2018-10-29 12:16 ` Alexander Monakov
2018-10-29 19:26 ` Jason Merrill
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Monakov @ 2018-10-29 12:16 UTC (permalink / raw)
To: Jason Merrill; +Cc: David Malcolm, gcc-patches
On Thu, 25 Oct 2018, Jason Merrill wrote:
> > >
> > > Maybe we should remove that in favor of fold_for_warn in
> > > check_function_arguments.
David, I think your patch also fixes PR 86567.
David, Jason, could you comment on doing something similar (using fold_for_warn
instead of maybe_constant_value) to solve other issues where generation of new
tree uids under maybe_constant_value called in warning context changes code
generation, in particular PR 86586?
Thanks.
Alexander
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Folding and check_function_arguments
2018-10-29 12:16 ` Alexander Monakov
@ 2018-10-29 19:26 ` Jason Merrill
2018-10-29 19:39 ` Alexander Monakov
0 siblings, 1 reply; 10+ messages in thread
From: Jason Merrill @ 2018-10-29 19:26 UTC (permalink / raw)
To: Alexander Monakov; +Cc: David Malcolm, gcc-patches List
On Mon, Oct 29, 2018 at 7:07 AM Alexander Monakov <amonakov@ispras.ru> wrote:
> On Thu, 25 Oct 2018, Jason Merrill wrote:
> > > >
> > > > Maybe we should remove that in favor of fold_for_warn in
> > > > check_function_arguments.
>
> David, I think your patch also fixes PR 86567.
>
> David, Jason, could you comment on doing something similar (using fold_for_warn
> instead of maybe_constant_value) to solve other issues where generation of new
> tree uids under maybe_constant_value called in warning context changes code
> generation, in particular PR 86586?
I don't see how it would help; this change does the same folding, just
a bit later.
Jason
Jason
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Folding and check_function_arguments
2018-10-29 19:26 ` Jason Merrill
@ 2018-10-29 19:39 ` Alexander Monakov
0 siblings, 0 replies; 10+ messages in thread
From: Alexander Monakov @ 2018-10-29 19:39 UTC (permalink / raw)
To: Jason Merrill; +Cc: David Malcolm, gcc-patches List
On Mon, 29 Oct 2018, Jason Merrill wrote:
> On Mon, Oct 29, 2018 at 7:07 AM Alexander Monakov <amonakov@ispras.ru> wrote:
> > On Thu, 25 Oct 2018, Jason Merrill wrote:
> > > > >
> > > > > Maybe we should remove that in favor of fold_for_warn in
> > > > > check_function_arguments.
> >
> > David, I think your patch also fixes PR 86567.
> >
> > David, Jason, could you comment on doing something similar (using fold_for_warn
> > instead of maybe_constant_value) to solve other issues where generation of new
> > tree uids under maybe_constant_value called in warning context changes code
> > generation, in particular PR 86586?
>
> I don't see how it would help; this change does the same folding, just
> a bit later.
If David's patch causes GCC to perform that folding only after main compilation
flow has already instantiated templates, that should help: folding for warning
wouldn't try to instantiate anything not already instantiated, and thus wouldn't
cause allocation of new uids, I think?
(I did check that it helps on the std::vector testcase given in the PR)
I see that my question about fold_for_warn doesn't make sense though.
Thanks.
Alexander
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-29 18:45 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-04 14:17 [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector David Malcolm
2018-10-04 14:33 ` [PATCH 2/2] Support string locations for C++ in -Wformat (PR c++/56856) David Malcolm
2018-10-05 17:52 ` Jeff Law
2018-10-08 14:44 ` Jason Merrill
2018-10-08 21:46 ` [PATCH] Folding and check_function_arguments David Malcolm
2018-10-25 19:08 ` Jason Merrill
2018-10-29 12:16 ` Alexander Monakov
2018-10-29 19:26 ` Jason Merrill
2018-10-29 19:39 ` Alexander Monakov
2018-10-05 17:41 ` [PATCH 1/2] testsuite: multiline.exp: implement optional target/xfail selector Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).