public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c-format: Add -Wformat-same-precision option [PR80060]
@ 2021-09-26 21:52 Daniil Stas
  2021-09-30 15:02 ` Martin Sebor
  0 siblings, 1 reply; 7+ messages in thread
From: Daniil Stas @ 2021-09-26 21:52 UTC (permalink / raw)
  To: gcc-patches; +Cc: Daniil Stas

This option is enabled by default when -Wformat option is enabled. A
user can specify -Wno-format-same-precision to disable emitting
warnings about an argument passed to printf-like function having a
different type from the one specified in the format string if the
types precisions are the same.

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

gcc/c-family/ChangeLog:

	* c-format.c (check_format_types): Don't emit warnings about
	type differences with the format string if
	-Wno-format-same-precision is specified and the types have
	the same precision.
	* c.opt: Add -Wformat-same-precision option.

gcc/ChangeLog:

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

gcc/testsuite/ChangeLog:

	* c-c++-common/Wformat-same-precision-1.c: New test.
	* c-c++-common/Wformat-same-precision-2.c: New test.
---
 gcc/c-family/c-format.c                               | 2 +-
 gcc/c-family/c.opt                                    | 5 +++++
 gcc/doc/invoke.texi                                   | 8 +++++++-
 gcc/testsuite/c-c++-common/Wformat-same-precision-1.c | 7 +++++++
 gcc/testsuite/c-c++-common/Wformat-same-precision-2.c | 7 +++++++
 5 files changed, 27 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/Wformat-same-precision-1.c
 create mode 100644 gcc/testsuite/c-c++-common/Wformat-same-precision-2.c

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index b4cb765a9d3..07cdcefbef8 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_same_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 9c151d19870..e7af7365c91 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -656,6 +656,11 @@ 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-same-precision
+C ObjC C++ ObjC++ Var(warn_format_same_precision) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=,warn_format >= 1, 0)
+Warn about type differences with the format string even if the types
+precision is the same.
+
 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 ba98eab68a5..8833f257d75 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -347,7 +347,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-same-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
@@ -6054,6 +6054,12 @@ 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-same-precision
+@opindex Wformat-same-precision
+@opindex Wno-format-same-precision
+If @option{-Wformat} is specified, warn about type differences with the format
+string even if the types precision is the same.
+
 @item -Wformat-security
 @opindex Wformat-security
 @opindex Wno-format-security
diff --git a/gcc/testsuite/c-c++-common/Wformat-same-precision-1.c b/gcc/testsuite/c-c++-common/Wformat-same-precision-1.c
new file mode 100644
index 00000000000..fbc11e4200a
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wformat-same-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-same-precision-2.c b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
new file mode 100644
index 00000000000..17e643e0441
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
@@ -0,0 +1,7 @@
+/* { dg-do compile { target lp64 } } */
+/* { dg-options "-Wformat -Wno-format-same-precision" } */
+
+void test ()
+{
+  __builtin_printf ("%lu\n", (long long) 1); /* { ! dg-warning "expects argument of type" } */
+}
-- 
2.33.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c-format: Add -Wformat-same-precision option [PR80060]
  2021-09-26 21:52 [PATCH] c-format: Add -Wformat-same-precision option [PR80060] Daniil Stas
@ 2021-09-30 15:02 ` Martin Sebor
  2021-10-01 22:41   ` Daniil Stas
  2021-10-10 23:10   ` [PATCH v2] c-format: Add -Wformat-int-precision " Daniil Stas
  0 siblings, 2 replies; 7+ messages in thread
From: Martin Sebor @ 2021-09-30 15:02 UTC (permalink / raw)
  To: Daniil Stas, gcc-patches

On 9/26/21 3:52 PM, Daniil Stas via Gcc-patches wrote:
> This option is enabled by default when -Wformat option is enabled. A
> user can specify -Wno-format-same-precision to disable emitting
> warnings about an argument passed to printf-like function having a
> different type from the one specified in the format string if the
> types precisions are the same.

Having an option to control this -Wformat aspect seems useful so
just a few comments mostly on the wording/naming choices.

Coming up with good names is tricky but I wonder if we can find
one that's clearer than "-Wformat-same-precision".  Precision can
mean a few different things in this context:  in the representation
of integers it refers to the number of value bits.  In that of
floating types, it refers to the number of significand bits.  And
in printf directives, it refers to what comes after the optional
period and what controls the minimum number of digits to format
(or maximum number of characters in a string).  So "same precision"
seems rather vague (and the proposed manual entry doesn't make it
clear).

IIUC, the option is specifically for directives that take integer
arguments and controls whether using an argument of an incompatible
integer type to a conversion specifier like i or x is diagnosed when
the argument has the same precision as the expected type.

With that in mind, would mentioning the word integer (or just int
for short) be an improvement?  E.g., -Wformat-int-precision?

Some more comments on the documentation text are below.

> 
> Signed-off-by: Daniil Stas <daniil.stas@posteo.net>
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-format.c (check_format_types): Don't emit warnings about
> 	type differences with the format string if
> 	-Wno-format-same-precision is specified and the types have
> 	the same precision.
> 	* c.opt: Add -Wformat-same-precision option.
> 
> gcc/ChangeLog:
> 
> 	* doc/invoke.texi: Add -Wformat-same-precision option description.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/Wformat-same-precision-1.c: New test.
> 	* c-c++-common/Wformat-same-precision-2.c: New test.
> ---
>   gcc/c-family/c-format.c                               | 2 +-
>   gcc/c-family/c.opt                                    | 5 +++++
>   gcc/doc/invoke.texi                                   | 8 +++++++-
>   gcc/testsuite/c-c++-common/Wformat-same-precision-1.c | 7 +++++++
>   gcc/testsuite/c-c++-common/Wformat-same-precision-2.c | 7 +++++++
>   5 files changed, 27 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/c-c++-common/Wformat-same-precision-1.c
>   create mode 100644 gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
> 
> diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> index b4cb765a9d3..07cdcefbef8 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_same_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 9c151d19870..e7af7365c91 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -656,6 +656,11 @@ 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-same-precision
> +C ObjC C++ ObjC++ Var(warn_format_same_precision) Warning LangEnabledBy(C ObjC C++ ObjC++,Wformat=,warn_format >= 1, 0)
> +Warn about type differences with the format string even if the types
> +precision is the same.

The grammar doesn't seem quite right here (I recommend to adjust
the text as well along similar lines as the manual, except more
brief as is customary here).


> +
>   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 ba98eab68a5..8833f257d75 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -347,7 +347,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-same-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
> @@ -6054,6 +6054,12 @@ 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-same-precision
> +@opindex Wformat-same-precision
> +@opindex Wno-format-same-precision
> +If @option{-Wformat} is specified, warn about type differences with the format
> +string even if the types precision is the same.
> +

As I mentioned above, the description seems rather vague.  How about
this instead:

   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.  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-same-precision-1.c b/gcc/testsuite/c-c++-common/Wformat-same-precision-1.c
> new file mode 100644
> index 00000000000..fbc11e4200a
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wformat-same-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-same-precision-2.c b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
> new file mode 100644
> index 00000000000..17e643e0441
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
> @@ -0,0 +1,7 @@
> +/* { dg-do compile { target lp64 } } */
> +/* { dg-options "-Wformat -Wno-format-same-precision" } */
> +
> +void test ()
> +{
> +  __builtin_printf ("%lu\n", (long long) 1); /* { ! dg-warning "expects argument of type" } */

I have never seen this syntax used in a dg-warning directive.  Does
it really work?  (Normally, we'd use dg-bogus to verify that a message
is not issued on a line.)

Incidentally, by using #pragma GCC diagnostic to toggle the option
both of these tests can be in the same file.

Martin

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] c-format: Add -Wformat-same-precision option [PR80060]
  2021-09-30 15:02 ` Martin Sebor
@ 2021-10-01 22:41   ` Daniil Stas
  2021-10-10 23:10   ` [PATCH v2] c-format: Add -Wformat-int-precision " Daniil Stas
  1 sibling, 0 replies; 7+ messages in thread
From: Daniil Stas @ 2021-10-01 22:41 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches

Hi, Martin

On Thu, 30 Sep 2021 09:02:28 -0600
Martin Sebor <msebor@gmail.com> wrote:

> On 9/26/21 3:52 PM, Daniil Stas via Gcc-patches wrote:
> > This option is enabled by default when -Wformat option is enabled. A
> > user can specify -Wno-format-same-precision to disable emitting
> > warnings about an argument passed to printf-like function having a
> > different type from the one specified in the format string if the
> > types precisions are the same.  
> 
> Having an option to control this -Wformat aspect seems useful so
> just a few comments mostly on the wording/naming choices.
> 
> Coming up with good names is tricky but I wonder if we can find
> one that's clearer than "-Wformat-same-precision".  Precision can
> mean a few different things in this context:  in the representation
> of integers it refers to the number of value bits.  In that of
> floating types, it refers to the number of significand bits.  And
> in printf directives, it refers to what comes after the optional
> period and what controls the minimum number of digits to format
> (or maximum number of characters in a string).  So "same precision"
> seems rather vague (and the proposed manual entry doesn't make it
> clear).
> 
> IIUC, the option is specifically for directives that take integer
> arguments and controls whether using an argument of an incompatible
> integer type to a conversion specifier like i or x is diagnosed when
> the argument has the same precision as the expected type.
> 
> With that in mind, would mentioning the word integer (or just int
> for short) be an improvement?  E.g., -Wformat-int-precision?
> 

Yes, I like -Wformat-int-precision name too.

> Some more comments on the documentation text are below.
> 
> > 
> > Signed-off-by: Daniil Stas <daniil.stas@posteo.net>
> > 
> > gcc/c-family/ChangeLog:
> > 
> > 	* c-format.c (check_format_types): Don't emit warnings about
> > 	type differences with the format string if
> > 	-Wno-format-same-precision is specified and the types have
> > 	the same precision.
> > 	* c.opt: Add -Wformat-same-precision option.
> > 
> > gcc/ChangeLog:
> > 
> > 	* doc/invoke.texi: Add -Wformat-same-precision option
> > description.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* c-c++-common/Wformat-same-precision-1.c: New test.
> > 	* c-c++-common/Wformat-same-precision-2.c: New test.
> > ---
> >   gcc/c-family/c-format.c                               | 2 +-
> >   gcc/c-family/c.opt                                    | 5 +++++
> >   gcc/doc/invoke.texi                                   | 8 +++++++-
> >   gcc/testsuite/c-c++-common/Wformat-same-precision-1.c | 7 +++++++
> >   gcc/testsuite/c-c++-common/Wformat-same-precision-2.c | 7 +++++++
> >   5 files changed, 27 insertions(+), 2 deletions(-)
> >   create mode 100644
> > gcc/testsuite/c-c++-common/Wformat-same-precision-1.c create mode
> > 100644 gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
> > 
> > diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> > index b4cb765a9d3..07cdcefbef8 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_same_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 9c151d19870..e7af7365c91 100644
> > --- a/gcc/c-family/c.opt
> > +++ b/gcc/c-family/c.opt
> > @@ -656,6 +656,11 @@ 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-same-precision
> > +C ObjC C++ ObjC++ Var(warn_format_same_precision) Warning
> > LangEnabledBy(C ObjC C++ ObjC++,Wformat=,warn_format >= 1, 0) +Warn
> > about type differences with the format string even if the types
> > +precision is the same.  
> 
> The grammar doesn't seem quite right here (I recommend to adjust
> the text as well along similar lines as the manual, except more
> brief as is customary here).
> 
> 
> > +
> >   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
> > ba98eab68a5..8833f257d75 100644 --- a/gcc/doc/invoke.texi
> > +++ b/gcc/doc/invoke.texi
> > @@ -347,7 +347,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-same-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
> > @@ -6054,6 +6054,12 @@ 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-same-precision
> > +@opindex Wformat-same-precision
> > +@opindex Wno-format-same-precision
> > +If @option{-Wformat} is specified, warn about type differences
> > with the format +string even if the types precision is the same.
> > +  
> 
> As I mentioned above, the description seems rather vague.  How about
> this instead:
> 
>    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.  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
> 

Ok, I will reword the documentation.

> >   @item -Wformat-security
> >   @opindex Wformat-security
> >   @opindex Wno-format-security
> > diff --git a/gcc/testsuite/c-c++-common/Wformat-same-precision-1.c
> > b/gcc/testsuite/c-c++-common/Wformat-same-precision-1.c new file
> > mode 100644 index 00000000000..fbc11e4200a
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/Wformat-same-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-same-precision-2.c
> > b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c new file
> > mode 100644 index 00000000000..17e643e0441
> > --- /dev/null
> > +++ b/gcc/testsuite/c-c++-common/Wformat-same-precision-2.c
> > @@ -0,0 +1,7 @@
> > +/* { dg-do compile { target lp64 } } */
> > +/* { dg-options "-Wformat -Wno-format-same-precision" } */
> > +
> > +void test ()
> > +{
> > +  __builtin_printf ("%lu\n", (long long) 1); /* { ! dg-warning
> > "expects argument of type" } */  
> 
> I have never seen this syntax used in a dg-warning directive.  Does
> it really work?  (Normally, we'd use dg-bogus to verify that a message
> is not issued on a line.)
> 
> Incidentally, by using #pragma GCC diagnostic to toggle the option
> both of these tests can be in the same file.

This test case passes with -Wno-format-same-precision option and fails
without it. But yes, maybe it just works by an accident (I could not
find the documentation how to make the inverse of dg-warning and tried
to guess the syntax a little). I will rework the tests too.

Thanks for your suggestions.

Best regards,
Daniil


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2] c-format: Add -Wformat-int-precision option [PR80060]
  2021-09-30 15:02 ` Martin Sebor
  2021-10-01 22:41   ` Daniil Stas
@ 2021-10-10 23:10   ` Daniil Stas
  2021-10-31 14:13     ` Daniil Stas
  1 sibling, 1 reply; 7+ messages in thread
From: Daniil Stas @ 2021-10-10 23:10 UTC (permalink / raw)
  To: gcc-patches; +Cc: Daniil Stas, Martin Sebor

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

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" } */
+}
-- 
2.33.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] c-format: Add -Wformat-int-precision option [PR80060]
  2021-10-10 23:10   ` [PATCH v2] c-format: Add -Wformat-int-precision " Daniil Stas
@ 2021-10-31 14:13     ` Daniil Stas
  2021-11-05  0:25       ` Martin Sebor
  0 siblings, 1 reply; 7+ messages in thread
From: Daniil Stas @ 2021-10-31 14:13 UTC (permalink / raw)
  To: gcc-patches

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] c-format: Add -Wformat-int-precision option [PR80060]
  2021-10-31 14:13     ` Daniil Stas
@ 2021-11-05  0:25       ` Martin Sebor
  2021-11-21 14:56         ` Daniil Stas
  0 siblings, 1 reply; 7+ messages in thread
From: Martin Sebor @ 2021-11-05  0:25 UTC (permalink / raw)
  To: Daniil Stas, gcc-patches; +Cc: Joseph S. Myers

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] c-format: Add -Wformat-int-precision option [PR80060]
  2021-11-05  0:25       ` Martin Sebor
@ 2021-11-21 14:56         ` Daniil Stas
  0 siblings, 0 replies; 7+ messages in thread
From: Daniil Stas @ 2021-11-21 14:56 UTC (permalink / raw)
  To: Martin Sebor; +Cc: gcc-patches, Joseph S. Myers

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-11-21 14:57 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-26 21:52 [PATCH] c-format: Add -Wformat-same-precision option [PR80060] Daniil Stas
2021-09-30 15:02 ` Martin Sebor
2021-10-01 22:41   ` Daniil Stas
2021-10-10 23:10   ` [PATCH v2] c-format: Add -Wformat-int-precision " Daniil Stas
2021-10-31 14:13     ` Daniil Stas
2021-11-05  0:25       ` Martin Sebor
2021-11-21 14:56         ` Daniil Stas

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).