[v2] c-format: Add -Wformat-int-precision option [PR80060]

Message ID 20211010231020.762020-1-daniil.stas@posteo.net
State New
Headers
Series [v2] c-format: Add -Wformat-int-precision option [PR80060] |

Commit Message

Daniil Stas Oct. 10, 2021, 11:10 p.m. UTC
  This option is enabled by default when -Wformat option is enabled. A
user can specify -Wno-format-int-precision to disable emitting
warnings when passing an argument of an incompatible integer type to
a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when it has
the same precision as the expected type.

Signed-off-by: Daniil Stas <daniil.stas@posteo.net>

gcc/c-family/ChangeLog:

	* c-format.c (check_format_types): Don't emit warnings when
	passing an argument of an incompatible integer type to
	a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when it has
	the same precision as the expected type if
	-Wno-format-int-precision option is specified.
	* c.opt: Add -Wformat-int-precision option.

gcc/ChangeLog:

	* doc/invoke.texi: Add -Wformat-int-precision option description.

gcc/testsuite/ChangeLog:

	* c-c++-common/Wformat-int-precision-1.c: New test.
	* c-c++-common/Wformat-int-precision-2.c: New test.
---
This is an update of patch "c-format: Add -Wformat-same-precision option [PR80060]".
The changes comparing to the first patch version:

- changed the option name to -Wformat-int-precision
- changed the option description as was suggested by Martin
- changed Wformat-int-precision-2.c to used dg-bogus instead of previous invalid
syntax

I also tried to combine the tests into one file with #pragma GCC diagnostic,
but looks like it's not possible. I want to test that when passing just -Wformat
option everything works as before my patch by default. And then in another test
case to check that passing -Wno-format-int-precision disables the warning. But
looks like in GCC you can't toggle the warnings such as
-Wno-format-int-precision individually but only can disable the general
-Wformat option that will disable all the formatting warnings together, which
is not the proper test.

 gcc/c-family/c-format.c                         |  2 +-
 gcc/c-family/c.opt                              |  6 ++++++
 gcc/doc/invoke.texi                             | 17 ++++++++++++++++-
 .../c-c++-common/Wformat-int-precision-1.c      |  7 +++++++
 .../c-c++-common/Wformat-int-precision-2.c      |  7 +++++++
 5 files changed, 37 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wformat-int-precision-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wformat-int-precision-2.c
  

Comments

Daniil Stas Oct. 31, 2021, 2:13 p.m. UTC | #1
On Sun, 10 Oct 2021 23:10:20 +0000
Daniil Stas <daniil.stas@posteo.net> wrote:

> This option is enabled by default when -Wformat option is enabled. A
> user can specify -Wno-format-int-precision to disable emitting
> warnings when passing an argument of an incompatible integer type to
> a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when it has
> the same precision as the expected type.
> 
> Signed-off-by: Daniil Stas <daniil.stas@posteo.net>
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-format.c (check_format_types): Don't emit warnings when
> 	passing an argument of an incompatible integer type to
> 	a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when
> it has the same precision as the expected type if
> 	-Wno-format-int-precision option is specified.
> 	* c.opt: Add -Wformat-int-precision option.
> 
> gcc/ChangeLog:
> 
> 	* doc/invoke.texi: Add -Wformat-int-precision option
> description.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/Wformat-int-precision-1.c: New test.
> 	* c-c++-common/Wformat-int-precision-2.c: New test.
> ---
> This is an update of patch "c-format: Add -Wformat-same-precision
> option [PR80060]". The changes comparing to the first patch version:
> 
> - changed the option name to -Wformat-int-precision
> - changed the option description as was suggested by Martin
> - changed Wformat-int-precision-2.c to used dg-bogus instead of
> previous invalid syntax
> 
> I also tried to combine the tests into one file with #pragma GCC
> diagnostic, but looks like it's not possible. I want to test that
> when passing just -Wformat option everything works as before my patch
> by default. And then in another test case to check that passing
> -Wno-format-int-precision disables the warning. But looks like in GCC
> you can't toggle the warnings such as -Wno-format-int-precision
> individually but only can disable the general -Wformat option that
> will disable all the formatting warnings together, which is not the
> proper test.

Hi,
Can anyone review this patch?
Thank you

--
Daniil
  
Martin Sebor Nov. 5, 2021, 12:25 a.m. UTC | #2
On 10/31/21 8:13 AM, Daniil Stas wrote:
> On Sun, 10 Oct 2021 23:10:20 +0000
> Daniil Stas <daniil.stas@posteo.net> wrote:
> 
>> This option is enabled by default when -Wformat option is enabled. A
>> user can specify -Wno-format-int-precision to disable emitting
>> warnings when passing an argument of an incompatible integer type to
>> a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when it has
>> the same precision as the expected type.
>>
>> Signed-off-by: Daniil Stas <daniil.stas@posteo.net>
>>
>> gcc/c-family/ChangeLog:
>>
>> 	* c-format.c (check_format_types): Don't emit warnings when
>> 	passing an argument of an incompatible integer type to
>> 	a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when
>> it has the same precision as the expected type if
>> 	-Wno-format-int-precision option is specified.
>> 	* c.opt: Add -Wformat-int-precision option.
>>
>> gcc/ChangeLog:
>>
>> 	* doc/invoke.texi: Add -Wformat-int-precision option
>> description.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* c-c++-common/Wformat-int-precision-1.c: New test.
>> 	* c-c++-common/Wformat-int-precision-2.c: New test.
>> ---
>> This is an update of patch "c-format: Add -Wformat-same-precision
>> option [PR80060]". The changes comparing to the first patch version:
>>
>> - changed the option name to -Wformat-int-precision
>> - changed the option description as was suggested by Martin
>> - changed Wformat-int-precision-2.c to used dg-bogus instead of
>> previous invalid syntax
>>
>> I also tried to combine the tests into one file with #pragma GCC
>> diagnostic, but looks like it's not possible. I want to test that
>> when passing just -Wformat option everything works as before my patch
>> by default. And then in another test case to check that passing
>> -Wno-format-int-precision disables the warning. But looks like in GCC
>> you can't toggle the warnings such as -Wno-format-int-precision
>> individually but only can disable the general -Wformat option that
>> will disable all the formatting warnings together, which is not the
>> proper test.
> 
> Hi,
> Can anyone review this patch?
> Thank you

I can't approve the change but it looks pretty good to me.

The documentation should wrap code symbols like int64_t, long,
or printf in @code{} directives.

I don't think the first test needs to be restricted to just
lp64, although I'd expect it to already be covered by the test
suite.  The lp64 selector only tells us that int is 32 bits
and long (and pointer) are 64, but nothing about long long so
I suspect the test might fail on other targets.  There's llp64
that's true for 4 byte ints and longs (but few targets match),
and long_neq_int that's true when long is not the same size as
int. So I think the inverse of the latter might be best, with
int and long as arguments.  testsuite/lib/target-supports.exp
defines these and others.

It might also be a good idea to add another case to the second
test to exercise arguments with different precision to make
sure -Wformat still triggers for those even  with
-Wno-format-int-precision.

The -Wformat warnings are Joseph's domain (CC'd) so either he
or some other C or global reviewer needs to sign off on changes
in this area.  (Please ping the patch weekly until you get
a response.)

Thanks
Martin
  
Daniil Stas Nov. 21, 2021, 2:56 p.m. UTC | #3
On Thu, 4 Nov 2021 18:25:14 -0600
Martin Sebor <msebor@gmail.com> wrote:

> On 10/31/21 8:13 AM, Daniil Stas wrote:
> > On Sun, 10 Oct 2021 23:10:20 +0000
> > Daniil Stas <daniil.stas@posteo.net> wrote:
> >   
> >> This option is enabled by default when -Wformat option is enabled.
> >> A user can specify -Wno-format-int-precision to disable emitting
> >> warnings when passing an argument of an incompatible integer type
> >> to a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when it
> >> has the same precision as the expected type.
> >>
> >> Signed-off-by: Daniil Stas <daniil.stas@posteo.net>
> >>
> >> gcc/c-family/ChangeLog:
> >>
> >> 	* c-format.c (check_format_types): Don't emit warnings when
> >> 	passing an argument of an incompatible integer type to
> >> 	a 'd', 'i', 'o', 'u', 'x', or 'X' conversion specifier when
> >> it has the same precision as the expected type if
> >> 	-Wno-format-int-precision option is specified.
> >> 	* c.opt: Add -Wformat-int-precision option.
> >>
> >> gcc/ChangeLog:
> >>
> >> 	* doc/invoke.texi: Add -Wformat-int-precision option
> >> description.
> >>
> >> gcc/testsuite/ChangeLog:
> >>
> >> 	* c-c++-common/Wformat-int-precision-1.c: New test.
> >> 	* c-c++-common/Wformat-int-precision-2.c: New test.
> >> ---
> >> This is an update of patch "c-format: Add -Wformat-same-precision
> >> option [PR80060]". The changes comparing to the first patch
> >> version:
> >>
> >> - changed the option name to -Wformat-int-precision
> >> - changed the option description as was suggested by Martin
> >> - changed Wformat-int-precision-2.c to used dg-bogus instead of
> >> previous invalid syntax
> >>
> >> I also tried to combine the tests into one file with #pragma GCC
> >> diagnostic, but looks like it's not possible. I want to test that
> >> when passing just -Wformat option everything works as before my
> >> patch by default. And then in another test case to check that
> >> passing -Wno-format-int-precision disables the warning. But looks
> >> like in GCC you can't toggle the warnings such as
> >> -Wno-format-int-precision individually but only can disable the
> >> general -Wformat option that will disable all the formatting
> >> warnings together, which is not the proper test.  
> > 
> > Hi,
> > Can anyone review this patch?
> > Thank you  
> 
> I can't approve the change but it looks pretty good to me.
> 
> The documentation should wrap code symbols like int64_t, long,
> or printf in @code{} directives.
> 
> I don't think the first test needs to be restricted to just
> lp64, although I'd expect it to already be covered by the test
> suite.  The lp64 selector only tells us that int is 32 bits
> and long (and pointer) are 64, but nothing about long long so
> I suspect the test might fail on other targets.  There's llp64
> that's true for 4 byte ints and longs (but few targets match),
> and long_neq_int that's true when long is not the same size as
> int. So I think the inverse of the latter might be best, with
> int and long as arguments.  testsuite/lib/target-supports.exp
> defines these and others.
> 
> It might also be a good idea to add another case to the second
> test to exercise arguments with different precision to make
> sure -Wformat still triggers for those even  with
> -Wno-format-int-precision.
> 
> The -Wformat warnings are Joseph's domain (CC'd) so either he
> or some other C or global reviewer needs to sign off on changes
> in this area.  (Please ping the patch weekly until you get
> a response.)
> 
> Thanks
> Martin

Hi, Martin
Thanks for your response. I've sent an updated patch.

Best regards,
Daniil
  

Patch

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index ca66c81f716..dd4436929f8 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -4243,7 +4243,7 @@  check_format_types (const substring_loc &fmt_loc,
 	  && (!pedantic || i < 2)
 	  && char_type_flag)
 	continue;
-      if (types->scalar_identity_flag
+      if ((types->scalar_identity_flag || !warn_format_int_precision)
 	  && (TREE_CODE (cur_type) == TREE_CODE (wanted_type)
 	      || (INTEGRAL_TYPE_P (cur_type)
 		  && INTEGRAL_TYPE_P (wanted_type)))
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 06457ac739e..f5b4af3f3f6 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -660,6 +660,12 @@  C ObjC C++ LTO ObjC++ Warning Alias(Wformat-overflow=, 1, 0) IntegerRange(0, 2)
 Warn about function calls with format strings that write past the end
 of the destination region.  Same as -Wformat-overflow=1.
 
+Wformat-int-precision
+C ObjC C++ ObjC++ Var(warn_format_int_precision) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=,warn_format >= 1, 0)
+Warn when passing an argument of an incompatible integer type to a 'd', 'i',
+'o', 'u', 'x', or 'X' conversion specifier even when it has the same precision
+as the expected type.
+
 Wformat-security
 C ObjC C++ ObjC++ Var(warn_format_security) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=, warn_format >= 2, 0)
 Warn about possible security problems with format functions.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 8b3ebcfbc4f..05dec6ba832 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -348,7 +348,7 @@  Objective-C and Objective-C++ Dialects}.
 -Werror  -Werror=*  -Wexpansion-to-defined  -Wfatal-errors @gol
 -Wfloat-conversion  -Wfloat-equal  -Wformat  -Wformat=2 @gol
 -Wno-format-contains-nul  -Wno-format-extra-args  @gol
--Wformat-nonliteral  -Wformat-overflow=@var{n} @gol
+-Wformat-nonliteral  -Wformat-overflow=@var{n} -Wformat-int-precision @gol
 -Wformat-security  -Wformat-signedness  -Wformat-truncation=@var{n} @gol
 -Wformat-y2k  -Wframe-address @gol
 -Wframe-larger-than=@var{byte-size}  -Wno-free-nonheap-object @gol
@@ -6056,6 +6056,21 @@  If @option{-Wformat} is specified, also warn if the format string is not a
 string literal and so cannot be checked, unless the format function
 takes its format arguments as a @code{va_list}.
 
+@item -Wformat-int-precision
+@opindex Wformat-int-precision
+@opindex Wno-format-int-precision
+Warn when passing an argument of an incompatible integer type to
+a @samp{d}, @samp{i}, @samp{o}, @samp{u}, @samp{x}, or @samp{X} conversion
+specifier even when it has the same precision as the expected type.
+For example, on targets where int64_t is a typedef for long, the warning
+is issued for the printf call below even when both long and long long have
+the same size and precision.
+
+@smallexample
+  extern int64_t n;
+  printf ("%lli", n);
+@end smallexample
+
 @item -Wformat-security
 @opindex Wformat-security
 @opindex Wno-format-security
diff --git a/gcc/testsuite/c-c++-common/Wformat-int-precision-1.c b/gcc/testsuite/c-c++-common/Wformat-int-precision-1.c
new file mode 100644
index 00000000000..fbc11e4200a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wformat-int-precision-1.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-Wformat" } */
+
+void test ()
+{
+  __builtin_printf ("%lu\n", (long long) 1); /* { dg-warning "expects argument of type" } */
+}
diff --git a/gcc/testsuite/c-c++-common/Wformat-int-precision-2.c b/gcc/testsuite/c-c++-common/Wformat-int-precision-2.c
new file mode 100644
index 00000000000..63536bf5ff2
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wformat-int-precision-2.c
@@ -0,0 +1,7 @@ 
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-Wformat -Wno-format-int-precision" } */
+
+void test ()
+{
+  __builtin_printf ("%lu\n", (long long) 1); /* { dg-bogus "expects argument of type" } */
+}