Hi David,
On 13 January 2017 at 21:04, David Malcolm <dmalc...@redhat.com> wrote:
> c-lex.c: lex_string uses cpp_get_token rather than
> cpp_get_token_with_location, and hence the C family of frontends
> record the physical locations of tokens in string concatenations, rather
> than the virtual locations, discarding any macro expansion information.
>
> The resulting *tree* node from the concatenation does use virtual
> locations (if appropriate).
>
> Hence if we have a macro expansion within a string literal, the macro
> expansion information is only recorded for the initial string token, and
> not for any followup string tokens concatenated with it.
>
> Hence any usage of macros defined in system headers in the concatenated
> string literal (apart from as the first token) will erroneously be
> treated as being within the system header by the substring location
> code, and hence we fail to emit a warning for this bogus code:
>
> void test (const char *msg)
> {
> printf ("size: %" PRIu32 "\n", msg);
> }
>
> format_warning_va determines whether or not the pertinent substring
> (here "%u") is spelled fully within the spelling of the whole format
> string, and if it is (case 1), it uses just the pertinent substring as
> the location of the diagnostic.
>
> In the above case, we have the '%' within the format string, but the
> 'u' is within the macro definition; the location of 'u' is used for the
> caret location, and this is the location used for determining if we're
> inside a system header, and so we erroneously bail out without printing
> the warning.
>
> This patch fixes things by tightening up the logic for case 1 of
> format_warning_va to detect non-trivial situations like this, and to fall
> back on cases 2 and 3, so that the location of the first string token of
> the format string will be used as the primary location of the diagnostics
> (and hence won't be in a system header); the location of the macro
> definition may be printed as a supplementary note (for case 2).
>
> For the test case above, this gives us back the missing warning
> (hitting case 2, and hence printing the macro defn):
>
> pr78304.c: In function ‘test’:
> pr78304.c:9:11: warning: format ‘%u’ expects argument of type
> ‘unsigned int’, but argument 2 has type ‘size_t {aka long unsigned int}’
> [-Wformat=]
> printf ("size: %" PRIu32 "\n", size);
> ^~~~~~~~~
> In file included from pr78304.c:4:0:
> /usr/include/inttypes.h:104:19: note: format string is defined here
> # define PRIu32 "u"
>
> Successfully bootstrapped®rtested on x86_64-pc-linux-gnu;
> adds 34 PASS results to gcc.sum.
>
These 2 tests fail on arm:
gcc.dg/format/pr78304.c (test for warnings, line 9)
gcc.dg/format/pr78304.c -DWIDE (test for warnings, line 9)
Christophe
> Committed to trunk as r244453.
>
> gcc/ChangeLog:
> PR c/78304
> * substring-locations.c (format_warning_va): Strengthen case 1 so
> that both endpoints of the substring must be within the format
> range for just the substring to be printed.
>
> gcc/testsuite/ChangeLog:
> PR c/78304
> * gcc.dg/format/diagnostic-ranges.c (test_macro): Undef INT_FMT.
> (test_macro_2): New test.
> (test_macro_3): New test.
> (test_macro_4): New test.
> (test_non_contiguous_strings): Convert line number to line offset.
> * gcc.dg/format/pr78304-2.c: New test case.
> * gcc.dg/format/pr78304.c: New test case.
> ---
> gcc/substring-locations.c | 2 +
> gcc/testsuite/gcc.dg/format/diagnostic-ranges.c | 53
> ++++++++++++++++++++++++-
> gcc/testsuite/gcc.dg/format/pr78304-2.c | 11 +++++
> gcc/testsuite/gcc.dg/format/pr78304.c | 10 +++++
> 4 files changed, 75 insertions(+), 1 deletion(-)
> create mode 100644 gcc/testsuite/gcc.dg/format/pr78304-2.c
> create mode 100644 gcc/testsuite/gcc.dg/format/pr78304.c
>
> diff --git a/gcc/substring-locations.c b/gcc/substring-locations.c
> index 8b41f2b..e2d8dd7 100644
> --- a/gcc/substring-locations.c
> +++ b/gcc/substring-locations.c
> @@ -118,6 +118,8 @@ format_warning_va (const substring_loc &fmt_loc,
> else
> {
> if (fmt_substring_range.m_start >= fmt_loc_range.m_start
> + && fmt_substring_range.m_start <= fmt_loc_range.m_finish
> + && fmt_substring_range.m_finish >= fmt_loc_range.m_start
> && fmt_substring_range.m_finish <= fmt_loc_range.m_finish)
> /* Case 1. */
> {
> diff --git a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
> b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
> index e5e6ade..cab30f2 100644
> --- a/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
> +++ b/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c
> @@ -254,12 +254,63 @@ void test_macro (const char *msg)
> ~^
> %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);
> + ^~~~~~~~~
> + { dg-end-multiline-output "" } */
> +/* { dg-begin-multiline-output "" }
> + #define PRIu32 "u"
> + ^
> + { dg-end-multiline-output "" } */
> +#undef PRIu32
> +}
> +
> +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 \\*' " } */
> + 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 "" } */
> +#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"
> + ~^
> + %s
> + { dg-end-multiline-output "" } */
> +#undef FMT_STRING
> }
>
> 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 *-*-* } 261 } */
> + /* { dg-message "26: format string is
> defined here" "" { target *-*-* } .-1 } */
> /* { dg-begin-multiline-output "" }
> __builtin_printf(" %" "d ", 0.5);
> ^~~~
> diff --git a/gcc/testsuite/gcc.dg/format/pr78304-2.c
> b/gcc/testsuite/gcc.dg/format/pr78304-2.c
> new file mode 100644
> index 0000000..5ee6d65
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/format/pr78304-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -Wall -Wextra" } */
> +
> +extern int printf (const char *, ...);
> +
> +# define PRIu32 "u"
> +
> +void test (long size)
> +{
> + printf ("size: %" PRIu32 "\n", size); /* { dg-warning "expects argument of
> type" } */
> +}
> diff --git a/gcc/testsuite/gcc.dg/format/pr78304.c
> b/gcc/testsuite/gcc.dg/format/pr78304.c
> new file mode 100644
> index 0000000..d0a96f6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/format/pr78304.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile { target inttypes_types } } */
> +/* { dg-options "-O2 -Wall -Wextra" } */
> +
> +#include <inttypes.h>
> +#include <stdio.h>
> +
> +void test (size_t size)
> +{
> + printf ("size: %" PRIu32 "\n", size); /* { dg-warning "expects argument of
> type" } */
> +}
> --
> 1.8.5.3
>
