c-format: Add -Wformat-same-precision option [PR80060]

  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
  

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
  

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