[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

David Malcolm  changed:

   What|Removed |Added

 Status|REOPENED|RESOLVED
 Resolution|--- |FIXED

--- Comment #10 from David Malcolm  ---
Sorry about the broken test.

It should be fixed by r244502 (it fixes it for me on i686-pc-linux-gnu, and on
32-bit sparc-sun-solaris2.12 and i386-pc-solaris2.12).

Marking as resolved again; please reopen if there are still issues.  Thanks.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

--- Comment #9 from David Malcolm  ---
Author: dmalcolm
Date: Mon Jan 16 18:08:45 2017
New Revision: 244502

URL: https://gcc.gnu.org/viewcvs?rev=244502=gcc=rev
Log:
Fix testcases for PR c/78304

The testcases as written made assumptions about size_t and long
being invalid for use with "%u".

We only need some invalid type, so this patch converts them to
attempt a "const char *" with "%u", which should be invalid for
every target.

gcc/testsuite/ChangeLog:
PR c/78304
* gcc.dg/format/pr78304.c: Convert argument from integral type
to a pointer.
* gcc.dg/format/pr78304-2.c: Likewise.


Modified:
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.dg/format/pr78304-2.c
trunk/gcc/testsuite/gcc.dg/format/pr78304.c

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[sch...@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

Andreas Schwab  changed:

   What|Removed |Added

 Status|RESOLVED|REOPENED
 Resolution|FIXED   |---

--- Comment #8 from Andreas Schwab  ---
The test fails on m68k.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

David Malcolm  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #7 from David Malcolm  ---
Should be fixed by r244453.  Marking as resolved.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

--- Comment #6 from David Malcolm  ---
Author: dmalcolm
Date: Fri Jan 13 19:27:43 2017
New Revision: 244453

URL: https://gcc.gnu.org/viewcvs?rev=244453=gcc=rev
Log:
Don't suppress bogus usage of macros from system headers in -Wformat (PR
c/78304)

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.


Added:
trunk/gcc/testsuite/gcc.dg/format/pr78304-2.c
trunk/gcc/testsuite/gcc.dg/format/pr78304.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/substring-locations.c
trunk/gcc/testsuite/ChangeLog
trunk/gcc/testsuite/gcc.dg/format/diagnostic-ranges.c

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

Richard Biener  changed:

   What|Removed |Added

   Priority|P3  |P1

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

--- Comment #5 from David Malcolm  ---
More notes to self:

The locations within the string_concat_db for this concatenation are all
spelling locations, rather than virtual locations.  The reason is that
c-lex.c's lex_string calls cpp_get_token internally rather than
cpp_get_token_with_location.

I experimented with calling the latter, so that it captures the virtual
location for e.g. PRIu32:

--- a/gcc/c-family/c-lex.c
+++ b/gcc/c-family/c-lex.c
@@ -1150,6 +1157,7 @@ lex_string (const cpp_token *tok, tree *valp, bool
objc_string, bool translate)
  for the common case of just one string.  */
   cpp_string str = tok->val.str;
   location_t init_loc = tok->src_loc;
+  location_t virtual_loc;
   cpp_string *strs = 
   location_t *locs = NULL;

@@ -1160,7 +1168,7 @@ lex_string (const cpp_token *tok, tree *valp, bool
objc_string, bool translate)
   bool objc_at_sign_was_seen = false;

  retry:
-  tok = cpp_get_token (parse_in);
+  tok = cpp_get_token_with_location (parse_in, _loc);
   switch (tok->type)
 {
 case CPP_PADDING:
@@ -1203,7 +1211,7 @@ lex_string (const cpp_token *tok, tree *valp, bool
objc_string, bool translate)

   concats++;
   obstack_grow (_ob, >val.str, sizeof (cpp_string));
-  obstack_grow (_ob, >src_loc, sizeof (location_t));
+  obstack_grow (_ob, _loc, sizeof (location_t));

   if (objc_string)
objc_at_sign_was_seen = false;

...with that patch, we get the virtual loc encoded within the concat database.
But then this within format_warning_va:
   fmt_loc.get_location (_substring_loc)
fails, because we don't know how to handle a macro expansion, and doing so
isn't something that seems sane to touch at this point in the development
cycle.

So one (or both?) of the fixes suggested in comment #4 seem appropriate.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

--- Comment #4 from David Malcolm  ---
Notes to self:

PRIu32 etc are described in:
  http://en.cppreference.com/w/c/types/integer#Format_macro_constants

Ideal would be a fix-it hint that suggests the correct macro, but that's clealy
overambitious in stage 3.

(gdb) p fmt_loc
$2 = (const substring_loc &) @0x7fffc4e0: {m_fmt_string_loc = 22834856,
m_string_type = 0x71a48738, m_caret_idx = 7, 
  m_start_idx = 6, m_end_idx = 7}

Indices suggest that we would highlight:

   %u
   ~^

i.e. with caret on the 'u'.

(gdb) p fmt_substring_range
$5 = {m_start = 22835072, m_finish = 5642112}
(gdb) p fmt_loc_range
$6 = {m_start = 22834848, m_finish = 22835104}

(gdb) call inform (fmt_substring_range.m_start, "fmt_substring_range.m_start")
../../src/pr78304.c:8:18: note: fmt_substring_range.m_start
   printf ("size: %" PRIu32 "\n", size);
  ^
(gdb) call inform (fmt_substring_range.m_finish,
"fmt_substring_range.m_finish")
In file included from ../../src/pr78304.c:1:0:
/usr/include/inttypes.h:104:19: note: fmt_substring_range.m_finish
 # define PRIu32  "u"
   ^

...so we have a range that splits between two different files.

Hence we erroneously hit case 1 of the conditional:

  if (fmt_substring_range.m_start >= fmt_loc_range.m_start
  && fmt_substring_range.m_finish <= fmt_loc_range.m_finish)
/* Case 1.  */
{
  substring_within_range = true;
  primary_loc = fmt_substring_loc;
}

...since:

(gdb) call inform (fmt_loc_range.m_start, "fmt_loc_range.m_start")
../../src/pr78304.c:8:11: note: fmt_loc_range.m_start
   printf ("size: %" PRIu32 "\n", size);
   ^
(gdb) call inform (fmt_loc_range.m_finish, "fmt_loc_range.m_finish")
../../src/pr78304.c:8:19: note: fmt_loc_range.m_finish
   printf ("size: %" PRIu32 "\n", size);
   ^

...and hence we use:

(gdb) p fmt_substring_loc
$7 = 2147483653
(gdb) call inform (fmt_substring_loc, "fmt_substring_loc")
In file included from ../../src/pr78304.c:1:0:
/usr/include/inttypes.h:104:19: note: fmt_substring_loc
 # define PRIu32  "u"

Case 1 seems to be missing a couple of conditionals:

--- a/gcc/substring-locations.c
+++ b/gcc/substring-locations.c
@@ -113,11 +120,17 @@ format_warning_va (const substring_loc _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.  */
{

and doing so fixes the issue, by making format_warning_va use case 2 here:

../../src/pr78304.c: In function ‘main’:
../../src/pr78304.c:8: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 ../../src/pr78304.c:1:0:
/usr/include/inttypes.h:104:19: note: format string is defined here
 # define PRIu32  "u"


Alternatively, perhaps the substring location code should detect when there's a
big discontinuity within the requested range (e.g. not it same file), and fail
if this happens, making format_warning_va use case 3.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[dmalcolm at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

David Malcolm  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |dmalcolm at gcc dot 
gnu.org

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

Jakub Jelinek  changed:

   What|Removed |Added

 CC||jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek  ---
If without -Wsystem-headers the warning is suppressed just because the
particular part of the concatenated string comes from system header, then I'd
say that is a clear bug.  For whether -Wsystem-headers applies or not in this
case I'd say it should matter whether any part of the concatenated string comes
from non-system header, or perhaps even whether the corresponding argument
comes from non-system header.  If the whole call is in a system header,
-Wsystem-headers should apply, but in this case that looks wrong.

Also, when the % comes from one place and the following u comes from another,
picking the u location rather than % location is quite unfortunate:

In file included from pr78304.c:1:0:
pr78304.c: In function ‘main’:
/usr/include/inttypes.h:104:19: warning: format ‘%u’ expects argument of type
‘unsigned int’, but argument 2 has type ‘size_t {aka long unsigned int}’
[-Wformat=]
 # define PRIu32  "u"

If the entire %u comes from there, perhaps (but it is still very hard to
decipher, as no macro context is printed), but for the mixed case I think it is
better to go with the location (if any) that can be found both in the macro
context of one of the parts and the other spot.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

--- Comment #2 from Marek Polacek  ---
Even though that with my patch we expansion_point_location_if_in_system_header
the caret location, it still points to the location in the system header, so
the diagnostics is suppressed.

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[mpolacek at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

Marek Polacek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2016-11-17
 CC||mpolacek at gcc dot gnu.org
 Ever confirmed|0   |1

--- Comment #1 from Marek Polacek  ---
It seems this has changed by r239253.  The warning appears with
-Wsystem-headers so it seemed like another instance of PR78000 / PR71613,
except that my recent patch to fix this doesn't work.  Will keep looking...

[Bug c/78304] [7 Regression] -Wformat doesn't warn anymore for inttypes.h format string argument type mismatches

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78304

Richard Biener  changed:

   What|Removed |Added

   Keywords||diagnostic
   Target Milestone|--- |7.0
Summary|-Wformat doesn't warn   |[7 Regression] -Wformat
   |anymore for inttypes.h  |doesn't warn anymore for
   |format string argument type |inttypes.h format string
   |mismatches  |argument type mismatches