-Wformat-overflow handling for %b and %B directives in C2X standard

  Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
directives in the sprintf function. I've added a relevant issue in bugzilla
(bug #105129).
I attach a patch with a possible solution to the letter.
  

On Sat, Apr 02, 2022 at 12:19:47AM +0500, Frolov Daniil via Gcc-patches wrote:
> Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
> directives in the sprintf function. I've added a relevant issue in bugzilla
> (bug #105129).
> I attach a patch with a possible solution to the letter.

Thanks for the patch.  Support for C2X %b, %B formats is relatively new
(Oct 2021) so it looks like gimple-ssa-sprintf.cc hasn't caught up.

This is not a regression, so should probably wait till GCC 13.  Anyway...

> From 2051344e9500651f6e94c44cbc7820715382b957 Mon Sep 17 00:00:00 2001
> From: Frolov Daniil <frolov.da@phystech.edu>
> Date: Fri, 1 Apr 2022 00:47:03 +0500
> Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> 
> testsuite: add tests to check -Wformat-overflow on %b.
> Wformat-overflow1.c is compiled using -std=c2x so warning has to
> be throwed
> 
> Wformat-overflow2.c doesn't throw warnings cause c2x std isn't
> used
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-sprintf.cc
>     (check_std_c2x): New function
> 	(fmtresult::type_max_digits): add base == 2 handling
> 	(tree_digits): add handle for base == 2
> 	(format_integer): now handle %b and %B using base = 2
> 	(parse_directive): add cases to handle %b and %B directives
> 	(compute_format_length): add handling for base = 2

The descriptions should start with a capital letter and end with a period,
like "Handle base == 2."
 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wformat-overflow1.c: New test. (using -std=c2x)
> 	* gcc.dg/Wformat-overflow2.c: New test. (-std=c11 no warning)

You can just say "New test."

> ---
>  gcc/gimple-ssa-sprintf.cc                | 42 ++++++++++++++++++++----
>  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 ++++++++++++++++
>  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 +++++++++
>  3 files changed, 79 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
>  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> 
> diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> index c93f12f90b5..7f68c2b6e51 100644
> --- a/gcc/gimple-ssa-sprintf.cc
> +++ b/gcc/gimple-ssa-sprintf.cc
> @@ -107,6 +107,15 @@ namespace {
>  
>  static int warn_level;
>  
> +/* b_overflow_flag depends on the current standart when using gcc */

"standard"

/* Comments should be formatted like this.  */

> +static bool b_overflow_flag;
> +
> +/* check is current standart version equals C2X*/
> +static bool check_std_c2x () 
> +{
> +  return !strcmp (lang_hooks.name, "GNU C2X");
> +}

Is this really needed?  ISTM that this new checking shouldn't depend on
-std=c2x.  If not using C2X, you only get a warning if -Wpedantic.  So
I think you should remove b_overflow_flag.

>  /* The minimum, maximum, likely, and unlikely maximum number of bytes
>     of output either a formatting function or an individual directive
>     can result in.  */
> @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
>    unsigned prec = TYPE_PRECISION (type);
>    switch (base)
>      {
> +    case 2:
> +      return prec;
>      case 8:
>        return (prec + 2) / 3;
>      case 10:
> @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec, bool plus, bool prefix)
>  
>    /* Adjust a non-zero value for the base prefix, either hexadecimal,
>       or, unless precision has resulted in a leading zero, also octal.  */
> -  if (prefix && absval && (base == 16 || prec <= ndigs))
> +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
>      {
>        if (base == 8)
>  	res += 1;
> -      else if (base == 16)
> +      else if (base == 16 || base == 2) /*0x...(0X...) and 0b...(0B...)*/
>  	res += 2;
>      }
>  
> @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>      case 'u':
>        base = 10;
>        break;
> +    case 'b':
> +    case 'B':
> +      base = 2;
> +      break;
>      case 'o':
>        base = 8;
>        break;
> @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  
>        /* Bump up the counters if WIDTH is greater than LEN.  */
>        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +					 (sign | maybebase) + (base == 2 || base == 16));
>        /* Bump up the counters again if PRECision is greater still.  */
>        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +					 (sign | maybebase) + (base == 2 || base == 16));
>  
>        return res;
>      }
> @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  	  if (res.range.min == 1)
>  	    res.range.likely += base == 8 ? 1 : 2;
>  	  else if (res.range.min == 2
> -		   && base == 16
> +		   && (base == 16 || base == 2)
>  		   && (dir.width[0] == 2 || dir.prec[0] == 2))
>  	    ++res.range.likely;
>  	}
> @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  
>    res.range.unlikely = res.range.max;
>    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> -				     (sign | maybebase) + (base == 16));
> +				     (sign | maybebase) + (base == 2 || base == 16));
>    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> -				     (sign | maybebase) + (base == 16));
> +				     (sign | maybebase) + (base == 2 || base == 16));
>  
>    return res;
>  }
> @@ -3680,6 +3695,8 @@ parse_directive (call_info &info,
>        ++pf;
>        break;
>      }
> +  
> +  

Drop this spurious change.

>    switch (target_to_host (*pf))
>      {
> @@ -3713,6 +3730,14 @@ parse_directive (call_info &info,
>      case 'X':
>        dir.fmtfunc = format_integer;
>        break;
> +    
> +    case 'b':
> +    case 'B':
> +      if (b_overflow_flag) {
> +        dir.fmtfunc = format_integer;
> +        break;
> +      }
> +      return 0;
>  
>      case 'p':
>        /* The %p output is implementation-defined.  It's possible
> @@ -4038,6 +4063,9 @@ compute_format_length (call_info &info, format_result *res,
>  
>    bool success = true;
>  
> +  /* Check for GNU C2X standart */
> +  b_overflow_flag = check_std_c2x ();
> +
>    for (const char *pf = info.fmtstr; ; ++dirno)
>      {
>        directive dir (&info, dirno);
> diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> new file mode 100644
> index 00000000000..cf9766fae14
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> @@ -0,0 +1,28 @@
> +/*
> +    { dg-do compile }
> +    { dg-options "-Wformat-overflow -std=c2x" }
> +*/
> +
> +extern int sprintf (char* restrict, const char* restrict, ...);
> +
> +void test_warn () {
> +
> +    int n = __INT_MAX__;
> +    char dst [5] = {0};
> +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> +
> +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> +
> +}
> +
> +void test_no_warn () {
> +
> +    char dst [5] = {0};
> +    int n = 8;
> +    sprintf (dst, "%b", n);
> +
> +    char another_dst [34] = {0};
> +    n = __INT_MAX__;
> +    sprintf (another_dst, "%#b", n);
> +
> +}
> diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> new file mode 100644
> index 00000000000..c6b1d9062a6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> @@ -0,0 +1,16 @@
> +/*
> +    { dg-do compile }
> +    { dg-options "-Wformat-overflow -std=c11" }
> +*/
> +
> +extern int sprintf (char* restrict, const char* restrict, ...);
> +
> +void test_no_warn () {
> +
> +    /*There is no reason to throw warning if std < c2x*/
> +
> +    char dst [5] = {0};
> +    int n = 32;
> +    sprintf (dst, "%b", n);
> +
> +}
> -- 
> 2.25.1
> 


Marek
  

Hello! Thanks for your feedback. I've tried to take into account your
comments. New patch applied to the letter.

The only thing I have not removed is the check_std_c2x () function. From my
point of view -Wformat-overflow shouldn't be thrown if the standard < C2X.
So it's protection for false triggering.

сб, 2 апр. 2022 г. в 01:15, Marek Polacek <polacek@redhat.com>:

> On Sat, Apr 02, 2022 at 12:19:47AM +0500, Frolov Daniil via Gcc-patches
> wrote:
> > Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
> > directives in the sprintf function. I've added a relevant issue in
> bugzilla
> > (bug #105129).
> > I attach a patch with a possible solution to the letter.
>
> Thanks for the patch.  Support for C2X %b, %B formats is relatively new
> (Oct 2021) so it looks like gimple-ssa-sprintf.cc hasn't caught up.
>
> This is not a regression, so should probably wait till GCC 13.  Anyway...
>
> > From 2051344e9500651f6e94c44cbc7820715382b957 Mon Sep 17 00:00:00 2001
> > From: Frolov Daniil <frolov.da@phystech.edu>
> > Date: Fri, 1 Apr 2022 00:47:03 +0500
> > Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> >
> > testsuite: add tests to check -Wformat-overflow on %b.
> > Wformat-overflow1.c is compiled using -std=c2x so warning has to
> > be throwed
> >
> > Wformat-overflow2.c doesn't throw warnings cause c2x std isn't
> > used
> >
> > gcc/ChangeLog:
> >
> >       * gimple-ssa-sprintf.cc
> >     (check_std_c2x): New function
> >       (fmtresult::type_max_digits): add base == 2 handling
> >       (tree_digits): add handle for base == 2
> >       (format_integer): now handle %b and %B using base = 2
> >       (parse_directive): add cases to handle %b and %B directives
> >       (compute_format_length): add handling for base = 2
>
> The descriptions should start with a capital letter and end with a period,
> like "Handle base == 2."
>
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/Wformat-overflow1.c: New test. (using -std=c2x)
> >       * gcc.dg/Wformat-overflow2.c: New test. (-std=c11 no warning)
>
> You can just say "New test."
>
> > ---
> >  gcc/gimple-ssa-sprintf.cc                | 42 ++++++++++++++++++++----
> >  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 ++++++++++++++++
> >  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 +++++++++
> >  3 files changed, 79 insertions(+), 7 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> >
> > diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> > index c93f12f90b5..7f68c2b6e51 100644
> > --- a/gcc/gimple-ssa-sprintf.cc
> > +++ b/gcc/gimple-ssa-sprintf.cc
> > @@ -107,6 +107,15 @@ namespace {
> >
> >  static int warn_level;
> >
> > +/* b_overflow_flag depends on the current standart when using gcc */
>
> "standard"
>
> /* Comments should be formatted like this.  */
>
> > +static bool b_overflow_flag;
> > +
> > +/* check is current standart version equals C2X*/
> > +static bool check_std_c2x ()
> > +{
> > +  return !strcmp (lang_hooks.name, "GNU C2X");
> > +}
>
> Is this really needed?  ISTM that this new checking shouldn't depend on
> -std=c2x.  If not using C2X, you only get a warning if -Wpedantic.  So
> I think you should remove b_overflow_flag.
>
> >  /* The minimum, maximum, likely, and unlikely maximum number of bytes
> >     of output either a formatting function or an individual directive
> >     can result in.  */
> > @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
> >    unsigned prec = TYPE_PRECISION (type);
> >    switch (base)
> >      {
> > +    case 2:
> > +      return prec;
> >      case 8:
> >        return (prec + 2) / 3;
> >      case 10:
> > @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec,
> bool plus, bool prefix)
> >
> >    /* Adjust a non-zero value for the base prefix, either hexadecimal,
> >       or, unless precision has resulted in a leading zero, also octal.
> */
> > -  if (prefix && absval && (base == 16 || prec <= ndigs))
> > +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
> >      {
> >        if (base == 8)
> >       res += 1;
> > -      else if (base == 16)
> > +      else if (base == 16 || base == 2) /*0x...(0X...) and
> 0b...(0B...)*/
> >       res += 2;
> >      }
> >
> > @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >      case 'u':
> >        base = 10;
> >        break;
> > +    case 'b':
> > +    case 'B':
> > +      base = 2;
> > +      break;
> >      case 'o':
> >        base = 8;
> >        break;
> > @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >
> >        /* Bump up the counters if WIDTH is greater than LEN.  */
> >        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > -                                      (sign | maybebase) + (base ==
> 16));
> > +                                      (sign | maybebase) + (base == 2
> || base == 16));
> >        /* Bump up the counters again if PRECision is greater still.  */
> >        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > -                                      (sign | maybebase) + (base ==
> 16));
> > +                                      (sign | maybebase) + (base == 2
> || base == 16));
> >
> >        return res;
> >      }
> > @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >         if (res.range.min == 1)
> >           res.range.likely += base == 8 ? 1 : 2;
> >         else if (res.range.min == 2
> > -                && base == 16
> > +                && (base == 16 || base == 2)
> >                  && (dir.width[0] == 2 || dir.prec[0] == 2))
> >           ++res.range.likely;
> >       }
> > @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg,
> pointer_query &ptr_qry)
> >
> >    res.range.unlikely = res.range.max;
> >    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > -                                  (sign | maybebase) + (base == 16));
> > +                                  (sign | maybebase) + (base == 2 ||
> base == 16));
> >    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > -                                  (sign | maybebase) + (base == 16));
> > +                                  (sign | maybebase) + (base == 2 ||
> base == 16));
> >
> >    return res;
> >  }
> > @@ -3680,6 +3695,8 @@ parse_directive (call_info &info,
> >        ++pf;
> >        break;
> >      }
> > +
> > +
>
> Drop this spurious change.
>
> >    switch (target_to_host (*pf))
> >      {
> > @@ -3713,6 +3730,14 @@ parse_directive (call_info &info,
> >      case 'X':
> >        dir.fmtfunc = format_integer;
> >        break;
> > +
> > +    case 'b':
> > +    case 'B':
> > +      if (b_overflow_flag) {
> > +        dir.fmtfunc = format_integer;
> > +        break;
> > +      }
> > +      return 0;
> >
> >      case 'p':
> >        /* The %p output is implementation-defined.  It's possible
> > @@ -4038,6 +4063,9 @@ compute_format_length (call_info &info,
> format_result *res,
> >
> >    bool success = true;
> >
> > +  /* Check for GNU C2X standart */
> > +  b_overflow_flag = check_std_c2x ();
> > +
> >    for (const char *pf = info.fmtstr; ; ++dirno)
> >      {
> >        directive dir (&info, dirno);
> > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > new file mode 100644
> > index 00000000000..cf9766fae14
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > @@ -0,0 +1,28 @@
> > +/*
> > +    { dg-do compile }
> > +    { dg-options "-Wformat-overflow -std=c2x" }
> > +*/
> > +
> > +extern int sprintf (char* restrict, const char* restrict, ...);
> > +
> > +void test_warn () {
> > +
> > +    int n = __INT_MAX__;
> > +    char dst [5] = {0};
> > +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> > +
> > +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> > +
> > +}
> > +
> > +void test_no_warn () {
> > +
> > +    char dst [5] = {0};
> > +    int n = 8;
> > +    sprintf (dst, "%b", n);
> > +
> > +    char another_dst [34] = {0};
> > +    n = __INT_MAX__;
> > +    sprintf (another_dst, "%#b", n);
> > +
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > new file mode 100644
> > index 00000000000..c6b1d9062a6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > @@ -0,0 +1,16 @@
> > +/*
> > +    { dg-do compile }
> > +    { dg-options "-Wformat-overflow -std=c11" }
> > +*/
> > +
> > +extern int sprintf (char* restrict, const char* restrict, ...);
> > +
> > +void test_no_warn () {
> > +
> > +    /*There is no reason to throw warning if std < c2x*/
> > +
> > +    char dst [5] = {0};
> > +    int n = 32;
> > +    sprintf (dst, "%b", n);
> > +
> > +}
> > --
> > 2.25.1
> >
>
>
> Marek
>
>
  

On Thu, Apr 07, 2022 at 02:10:48AM +0500, Frolov Daniil wrote:
> Hello! Thanks for your feedback. I've tried to take into account your
> comments. New patch applied to the letter.

Thanks.
 
> The only thing I have not removed is the check_std_c2x () function. From my
> point of view -Wformat-overflow shouldn't be thrown if the standard < C2X.
> So it's protection for false triggering.

Sorry but I still think that is the wrong behavior.  If you want to warn
about C2X constructs in pre-C2X modes, use -Wpedantic.  But if you want
to use %b/%B as an extension in older dialects, that's OK too, so I don't
know why users would want -Wformat-overflow disabled in that case.  But
perhaps other people disagree with me.

> сб, 2 апр. 2022 г. в 01:15, Marek Polacek <polacek@redhat.com>:
> 
> > On Sat, Apr 02, 2022 at 12:19:47AM +0500, Frolov Daniil via Gcc-patches
> > wrote:
> > > Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
> > > directives in the sprintf function. I've added a relevant issue in
> > bugzilla
> > > (bug #105129).
> > > I attach a patch with a possible solution to the letter.
> >
> > Thanks for the patch.  Support for C2X %b, %B formats is relatively new
> > (Oct 2021) so it looks like gimple-ssa-sprintf.cc hasn't caught up.
> >
> > This is not a regression, so should probably wait till GCC 13.  Anyway...
> >
> > > From 2051344e9500651f6e94c44cbc7820715382b957 Mon Sep 17 00:00:00 2001
> > > From: Frolov Daniil <frolov.da@phystech.edu>
> > > Date: Fri, 1 Apr 2022 00:47:03 +0500
> > > Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> > >
> > > testsuite: add tests to check -Wformat-overflow on %b.
> > > Wformat-overflow1.c is compiled using -std=c2x so warning has to
> > > be throwed
> > >
> > > Wformat-overflow2.c doesn't throw warnings cause c2x std isn't
> > > used
> > >
> > > gcc/ChangeLog:
> > >
> > >       * gimple-ssa-sprintf.cc
> > >     (check_std_c2x): New function
> > >       (fmtresult::type_max_digits): add base == 2 handling
> > >       (tree_digits): add handle for base == 2
> > >       (format_integer): now handle %b and %B using base = 2
> > >       (parse_directive): add cases to handle %b and %B directives
> > >       (compute_format_length): add handling for base = 2
> >
> > The descriptions should start with a capital letter and end with a period,
> > like "Handle base == 2."
> >
> > > gcc/testsuite/ChangeLog:
> > >
> > >       * gcc.dg/Wformat-overflow1.c: New test. (using -std=c2x)
> > >       * gcc.dg/Wformat-overflow2.c: New test. (-std=c11 no warning)
> >
> > You can just say "New test."
> >
> > > ---
> > >  gcc/gimple-ssa-sprintf.cc                | 42 ++++++++++++++++++++----
> > >  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 ++++++++++++++++
> > >  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 +++++++++
> > >  3 files changed, 79 insertions(+), 7 deletions(-)
> > >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > >
> > > diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> > > index c93f12f90b5..7f68c2b6e51 100644
> > > --- a/gcc/gimple-ssa-sprintf.cc
> > > +++ b/gcc/gimple-ssa-sprintf.cc
> > > @@ -107,6 +107,15 @@ namespace {
> > >
> > >  static int warn_level;
> > >
> > > +/* b_overflow_flag depends on the current standart when using gcc */
> >
> > "standard"
> >
> > /* Comments should be formatted like this.  */
> >
> > > +static bool b_overflow_flag;
> > > +
> > > +/* check is current standart version equals C2X*/
> > > +static bool check_std_c2x ()
> > > +{
> > > +  return !strcmp (lang_hooks.name, "GNU C2X");
> > > +}
> >
> > Is this really needed?  ISTM that this new checking shouldn't depend on
> > -std=c2x.  If not using C2X, you only get a warning if -Wpedantic.  So
> > I think you should remove b_overflow_flag.
> >
> > >  /* The minimum, maximum, likely, and unlikely maximum number of bytes
> > >     of output either a formatting function or an individual directive
> > >     can result in.  */
> > > @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
> > >    unsigned prec = TYPE_PRECISION (type);
> > >    switch (base)
> > >      {
> > > +    case 2:
> > > +      return prec;
> > >      case 8:
> > >        return (prec + 2) / 3;
> > >      case 10:
> > > @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec,
> > bool plus, bool prefix)
> > >
> > >    /* Adjust a non-zero value for the base prefix, either hexadecimal,
> > >       or, unless precision has resulted in a leading zero, also octal.
> > */
> > > -  if (prefix && absval && (base == 16 || prec <= ndigs))
> > > +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
> > >      {
> > >        if (base == 8)
> > >       res += 1;
> > > -      else if (base == 16)
> > > +      else if (base == 16 || base == 2) /*0x...(0X...) and
> > 0b...(0B...)*/
> > >       res += 2;
> > >      }
> > >
> > > @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg,
> > pointer_query &ptr_qry)
> > >      case 'u':
> > >        base = 10;
> > >        break;
> > > +    case 'b':
> > > +    case 'B':
> > > +      base = 2;
> > > +      break;
> > >      case 'o':
> > >        base = 8;
> > >        break;
> > > @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg,
> > pointer_query &ptr_qry)
> > >
> > >        /* Bump up the counters if WIDTH is greater than LEN.  */
> > >        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > > -                                      (sign | maybebase) + (base ==
> > 16));
> > > +                                      (sign | maybebase) + (base == 2
> > || base == 16));
> > >        /* Bump up the counters again if PRECision is greater still.  */
> > >        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > > -                                      (sign | maybebase) + (base ==
> > 16));
> > > +                                      (sign | maybebase) + (base == 2
> > || base == 16));
> > >
> > >        return res;
> > >      }
> > > @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg,
> > pointer_query &ptr_qry)
> > >         if (res.range.min == 1)
> > >           res.range.likely += base == 8 ? 1 : 2;
> > >         else if (res.range.min == 2
> > > -                && base == 16
> > > +                && (base == 16 || base == 2)
> > >                  && (dir.width[0] == 2 || dir.prec[0] == 2))
> > >           ++res.range.likely;
> > >       }
> > > @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg,
> > pointer_query &ptr_qry)
> > >
> > >    res.range.unlikely = res.range.max;
> > >    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > > -                                  (sign | maybebase) + (base == 16));
> > > +                                  (sign | maybebase) + (base == 2 ||
> > base == 16));
> > >    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > > -                                  (sign | maybebase) + (base == 16));
> > > +                                  (sign | maybebase) + (base == 2 ||
> > base == 16));
> > >
> > >    return res;
> > >  }
> > > @@ -3680,6 +3695,8 @@ parse_directive (call_info &info,
> > >        ++pf;
> > >        break;
> > >      }
> > > +
> > > +
> >
> > Drop this spurious change.
> >
> > >    switch (target_to_host (*pf))
> > >      {
> > > @@ -3713,6 +3730,14 @@ parse_directive (call_info &info,
> > >      case 'X':
> > >        dir.fmtfunc = format_integer;
> > >        break;
> > > +
> > > +    case 'b':
> > > +    case 'B':
> > > +      if (b_overflow_flag) {
> > > +        dir.fmtfunc = format_integer;
> > > +        break;
> > > +      }
> > > +      return 0;
> > >
> > >      case 'p':
> > >        /* The %p output is implementation-defined.  It's possible
> > > @@ -4038,6 +4063,9 @@ compute_format_length (call_info &info,
> > format_result *res,
> > >
> > >    bool success = true;
> > >
> > > +  /* Check for GNU C2X standart */
> > > +  b_overflow_flag = check_std_c2x ();
> > > +
> > >    for (const char *pf = info.fmtstr; ; ++dirno)
> > >      {
> > >        directive dir (&info, dirno);
> > > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > new file mode 100644
> > > index 00000000000..cf9766fae14
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > @@ -0,0 +1,28 @@
> > > +/*
> > > +    { dg-do compile }
> > > +    { dg-options "-Wformat-overflow -std=c2x" }
> > > +*/
> > > +
> > > +extern int sprintf (char* restrict, const char* restrict, ...);
> > > +
> > > +void test_warn () {
> > > +
> > > +    int n = __INT_MAX__;
> > > +    char dst [5] = {0};
> > > +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> > > +
> > > +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> > > +
> > > +}
> > > +
> > > +void test_no_warn () {
> > > +
> > > +    char dst [5] = {0};
> > > +    int n = 8;
> > > +    sprintf (dst, "%b", n);
> > > +
> > > +    char another_dst [34] = {0};
> > > +    n = __INT_MAX__;
> > > +    sprintf (another_dst, "%#b", n);
> > > +
> > > +}
> > > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > new file mode 100644
> > > index 00000000000..c6b1d9062a6
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > @@ -0,0 +1,16 @@
> > > +/*
> > > +    { dg-do compile }
> > > +    { dg-options "-Wformat-overflow -std=c11" }
> > > +*/
> > > +
> > > +extern int sprintf (char* restrict, const char* restrict, ...);
> > > +
> > > +void test_no_warn () {
> > > +
> > > +    /*There is no reason to throw warning if std < c2x*/
> > > +
> > > +    char dst [5] = {0};
> > > +    int n = 32;
> > > +    sprintf (dst, "%b", n);
> > > +
> > > +}
> > > --
> > > 2.25.1
> > >
> >
> >
> > Marek
> >
> >

> From 7b14a2aa909194841fa916f2db5d8aa1e4a6367e Mon Sep 17 00:00:00 2001
> From: Frolov Daniil <frolov.da@phystech.edu>
> Date: Thu, 7 Apr 2022 02:05:58 +0500
> Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-sprintf.cc (check_std_c2x): Handle base == 2.
> 	(fmtresult::type_max_digits): Handle base == 2.
> 	(tree_digits): Handle base == 2.
> 	(format_integer): Handle base == 2.
> 	(parse_directive): Add cases for %b and %B directives.
> 	(compute_format_length): Handle base == 2.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wformat-overflow1.c: New test.
> 	* gcc.dg/Wformat-overflow2.c: New test.
> ---
>  gcc/gimple-ssa-sprintf.cc                | 40 +++++++++++++++++++-----
>  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 +++++++++++++++++
>  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 ++++++++++
>  3 files changed, 77 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
>  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> 
> diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> index c93f12f90b5..6a14a004483 100644
> --- a/gcc/gimple-ssa-sprintf.cc
> +++ b/gcc/gimple-ssa-sprintf.cc
> @@ -107,6 +107,15 @@ namespace {
>  
>  static int warn_level;
>  
> +/* The b_overflow_flag depends on the current standard when using gcc. */
> +static bool b_overflow_flag;
> +
> +/* Check is current standard version equals C2X. */
> +static bool check_std_c2x () 
> +{
> +  return !strcmp (lang_hooks.name, "GNU C2X");
> +}
> +
>  /* The minimum, maximum, likely, and unlikely maximum number of bytes
>     of output either a formatting function or an individual directive
>     can result in.  */
> @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
>    unsigned prec = TYPE_PRECISION (type);
>    switch (base)
>      {
> +    case 2:
> +      return prec;
>      case 8:
>        return (prec + 2) / 3;
>      case 10:
> @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec, bool plus, bool prefix)
>  
>    /* Adjust a non-zero value for the base prefix, either hexadecimal,
>       or, unless precision has resulted in a leading zero, also octal.  */
> -  if (prefix && absval && (base == 16 || prec <= ndigs))
> +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
>      {
>        if (base == 8)
>  	res += 1;
> -      else if (base == 16)
> +      else if (base == 16 || base == 2) /* 0x...(0X...) and 0b...(0B...) */
>  	res += 2;
>      }
>  
> @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>      case 'u':
>        base = 10;
>        break;
> +    case 'b':
> +    case 'B':
> +      base = 2;
> +      break;
>      case 'o':
>        base = 8;
>        break;
> @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  
>        /* Bump up the counters if WIDTH is greater than LEN.  */
>        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +					 (sign | maybebase) + (base == 2 || base == 16));
>        /* Bump up the counters again if PRECision is greater still.  */
>        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +					 (sign | maybebase) + (base == 2 || base == 16));
>  
>        return res;
>      }
> @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  	  if (res.range.min == 1)
>  	    res.range.likely += base == 8 ? 1 : 2;
>  	  else if (res.range.min == 2
> -		   && base == 16
> +		   && (base == 16 || base == 2)
>  		   && (dir.width[0] == 2 || dir.prec[0] == 2))
>  	    ++res.range.likely;
>  	}
> @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  
>    res.range.unlikely = res.range.max;
>    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> -				     (sign | maybebase) + (base == 16));
> +				     (sign | maybebase) + (base == 2 || base == 16));
>    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> -				     (sign | maybebase) + (base == 16));
> +				     (sign | maybebase) + (base == 2 || base == 16));
>  
>    return res;
>  }
> @@ -3713,6 +3728,14 @@ parse_directive (call_info &info,
>      case 'X':
>        dir.fmtfunc = format_integer;
>        break;
> +    
> +    case 'b':
> +    case 'B':
> +      if (b_overflow_flag) {
> +        dir.fmtfunc = format_integer;
> +        break;
> +      }
> +      return 0;
>  
>      case 'p':
>        /* The %p output is implementation-defined.  It's possible
> @@ -4038,6 +4061,9 @@ compute_format_length (call_info &info, format_result *res,
>  
>    bool success = true;
>  
> +  /* Check for GNU C2X standard */
> +  b_overflow_flag = check_std_c2x ();
> +
>    for (const char *pf = info.fmtstr; ; ++dirno)
>      {
>        directive dir (&info, dirno);
> diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> new file mode 100644
> index 00000000000..cf9766fae14
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> @@ -0,0 +1,28 @@
> +/*
> +    { dg-do compile }
> +    { dg-options "-Wformat-overflow -std=c2x" }
> +*/
> +
> +extern int sprintf (char* restrict, const char* restrict, ...);
> +
> +void test_warn () {
> +
> +    int n = __INT_MAX__;
> +    char dst [5] = {0};
> +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> +
> +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> +
> +}
> +
> +void test_no_warn () {
> +
> +    char dst [5] = {0};
> +    int n = 8;
> +    sprintf (dst, "%b", n);
> +
> +    char another_dst [34] = {0};
> +    n = __INT_MAX__;
> +    sprintf (another_dst, "%#b", n);
> +
> +}
> diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> new file mode 100644
> index 00000000000..c6b1d9062a6
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> @@ -0,0 +1,16 @@
> +/*
> +    { dg-do compile }
> +    { dg-options "-Wformat-overflow -std=c11" }
> +*/
> +
> +extern int sprintf (char* restrict, const char* restrict, ...);
> +
> +void test_no_warn () {
> +
> +    /*There is no reason to throw warning if std < c2x*/
> +
> +    char dst [5] = {0};
> +    int n = 32;
> +    sprintf (dst, "%b", n);
> +
> +}
> -- 
> 2.25.1
> 


Marek
  

вт, 12 апр. 2022 г. в 00:56, Marek Polacek <polacek@redhat.com>:

>
> On Thu, Apr 07, 2022 at 02:10:48AM +0500, Frolov Daniil wrote:
> > Hello! Thanks for your feedback. I've tried to take into account your
> > comments. New patch applied to the letter.
>
> Thanks.
>
> > The only thing I have not removed is the check_std_c2x () function. From my
> > point of view -Wformat-overflow shouldn't be thrown if the standard < C2X.
> > So it's protection for false triggering.
>
> Sorry but I still think that is the wrong behavior.  If you want to warn
> about C2X constructs in pre-C2X modes, use -Wpedantic.  But if you want
> to use %b/%B as an extension in older dialects, that's OK too, so I don't
> know why users would want -Wformat-overflow disabled in that case.  But
> perhaps other people disagree with me.
>
Hi! Sorry for the late reply. If we want to look at it as on extension
then I am agreed with you.
Removed this function in new patch.
> > сб, 2 апр. 2022 г. в 01:15, Marek Polacek <polacek@redhat.com>:
> >
> > > On Sat, Apr 02, 2022 at 12:19:47AM +0500, Frolov Daniil via Gcc-patches
> > > wrote:
> > > > Hello, I've noticed that -Wformat-overflow doesn't handle %b and %B
> > > > directives in the sprintf function. I've added a relevant issue in
> > > bugzilla
> > > > (bug #105129).
> > > > I attach a patch with a possible solution to the letter.
> > >
> > > Thanks for the patch.  Support for C2X %b, %B formats is relatively new
> > > (Oct 2021) so it looks like gimple-ssa-sprintf.cc hasn't caught up.
> > >
> > > This is not a regression, so should probably wait till GCC 13.  Anyway...
> > >
> > > > From 2051344e9500651f6e94c44cbc7820715382b957 Mon Sep 17 00:00:00 2001
> > > > From: Frolov Daniil <frolov.da@phystech.edu>
> > > > Date: Fri, 1 Apr 2022 00:47:03 +0500
> > > > Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> > > >
> > > > testsuite: add tests to check -Wformat-overflow on %b.
> > > > Wformat-overflow1.c is compiled using -std=c2x so warning has to
> > > > be throwed
> > > >
> > > > Wformat-overflow2.c doesn't throw warnings cause c2x std isn't
> > > > used
> > > >
> > > > gcc/ChangeLog:
> > > >
> > > >       * gimple-ssa-sprintf.cc
> > > >     (check_std_c2x): New function
> > > >       (fmtresult::type_max_digits): add base == 2 handling
> > > >       (tree_digits): add handle for base == 2
> > > >       (format_integer): now handle %b and %B using base = 2
> > > >       (parse_directive): add cases to handle %b and %B directives
> > > >       (compute_format_length): add handling for base = 2
> > >
> > > The descriptions should start with a capital letter and end with a period,
> > > like "Handle base == 2."
> > >
> > > > gcc/testsuite/ChangeLog:
> > > >
> > > >       * gcc.dg/Wformat-overflow1.c: New test. (using -std=c2x)
> > > >       * gcc.dg/Wformat-overflow2.c: New test. (-std=c11 no warning)
> > >
> > > You can just say "New test."
> > >
> > > > ---
> > > >  gcc/gimple-ssa-sprintf.cc                | 42 ++++++++++++++++++++----
> > > >  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 ++++++++++++++++
> > > >  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 +++++++++
> > > >  3 files changed, 79 insertions(+), 7 deletions(-)
> > > >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > >
> > > > diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> > > > index c93f12f90b5..7f68c2b6e51 100644
> > > > --- a/gcc/gimple-ssa-sprintf.cc
> > > > +++ b/gcc/gimple-ssa-sprintf.cc
> > > > @@ -107,6 +107,15 @@ namespace {
> > > >
> > > >  static int warn_level;
> > > >
> > > > +/* b_overflow_flag depends on the current standart when using gcc */
> > >
> > > "standard"
> > >
> > > /* Comments should be formatted like this.  */
> > >
> > > > +static bool b_overflow_flag;
> > > > +
> > > > +/* check is current standart version equals C2X*/
> > > > +static bool check_std_c2x ()
> > > > +{
> > > > +  return !strcmp (lang_hooks.name, "GNU C2X");
> > > > +}
> > >
> > > Is this really needed?  ISTM that this new checking shouldn't depend on
> > > -std=c2x.  If not using C2X, you only get a warning if -Wpedantic.  So
> > > I think you should remove b_overflow_flag.
> > >
> > > >  /* The minimum, maximum, likely, and unlikely maximum number of bytes
> > > >     of output either a formatting function or an individual directive
> > > >     can result in.  */
> > > > @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
> > > >    unsigned prec = TYPE_PRECISION (type);
> > > >    switch (base)
> > > >      {
> > > > +    case 2:
> > > > +      return prec;
> > > >      case 8:
> > > >        return (prec + 2) / 3;
> > > >      case 10:
> > > > @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec,
> > > bool plus, bool prefix)
> > > >
> > > >    /* Adjust a non-zero value for the base prefix, either hexadecimal,
> > > >       or, unless precision has resulted in a leading zero, also octal.
> > > */
> > > > -  if (prefix && absval && (base == 16 || prec <= ndigs))
> > > > +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
> > > >      {
> > > >        if (base == 8)
> > > >       res += 1;
> > > > -      else if (base == 16)
> > > > +      else if (base == 16 || base == 2) /*0x...(0X...) and
> > > 0b...(0B...)*/
> > > >       res += 2;
> > > >      }
> > > >
> > > > @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg,
> > > pointer_query &ptr_qry)
> > > >      case 'u':
> > > >        base = 10;
> > > >        break;
> > > > +    case 'b':
> > > > +    case 'B':
> > > > +      base = 2;
> > > > +      break;
> > > >      case 'o':
> > > >        base = 8;
> > > >        break;
> > > > @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg,
> > > pointer_query &ptr_qry)
> > > >
> > > >        /* Bump up the counters if WIDTH is greater than LEN.  */
> > > >        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > > > -                                      (sign | maybebase) + (base ==
> > > 16));
> > > > +                                      (sign | maybebase) + (base == 2
> > > || base == 16));
> > > >        /* Bump up the counters again if PRECision is greater still.  */
> > > >        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > > > -                                      (sign | maybebase) + (base ==
> > > 16));
> > > > +                                      (sign | maybebase) + (base == 2
> > > || base == 16));
> > > >
> > > >        return res;
> > > >      }
> > > > @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg,
> > > pointer_query &ptr_qry)
> > > >         if (res.range.min == 1)
> > > >           res.range.likely += base == 8 ? 1 : 2;
> > > >         else if (res.range.min == 2
> > > > -                && base == 16
> > > > +                && (base == 16 || base == 2)
> > > >                  && (dir.width[0] == 2 || dir.prec[0] == 2))
> > > >           ++res.range.likely;
> > > >       }
> > > > @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg,
> > > pointer_query &ptr_qry)
> > > >
> > > >    res.range.unlikely = res.range.max;
> > > >    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > > > -                                  (sign | maybebase) + (base == 16));
> > > > +                                  (sign | maybebase) + (base == 2 ||
> > > base == 16));
> > > >    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > > > -                                  (sign | maybebase) + (base == 16));
> > > > +                                  (sign | maybebase) + (base == 2 ||
> > > base == 16));
> > > >
> > > >    return res;
> > > >  }
> > > > @@ -3680,6 +3695,8 @@ parse_directive (call_info &info,
> > > >        ++pf;
> > > >        break;
> > > >      }
> > > > +
> > > > +
> > >
> > > Drop this spurious change.
> > >
> > > >    switch (target_to_host (*pf))
> > > >      {
> > > > @@ -3713,6 +3730,14 @@ parse_directive (call_info &info,
> > > >      case 'X':
> > > >        dir.fmtfunc = format_integer;
> > > >        break;
> > > > +
> > > > +    case 'b':
> > > > +    case 'B':
> > > > +      if (b_overflow_flag) {
> > > > +        dir.fmtfunc = format_integer;
> > > > +        break;
> > > > +      }
> > > > +      return 0;
> > > >
> > > >      case 'p':
> > > >        /* The %p output is implementation-defined.  It's possible
> > > > @@ -4038,6 +4063,9 @@ compute_format_length (call_info &info,
> > > format_result *res,
> > > >
> > > >    bool success = true;
> > > >
> > > > +  /* Check for GNU C2X standart */
> > > > +  b_overflow_flag = check_std_c2x ();
> > > > +
> > > >    for (const char *pf = info.fmtstr; ; ++dirno)
> > > >      {
> > > >        directive dir (&info, dirno);
> > > > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > > new file mode 100644
> > > > index 00000000000..cf9766fae14
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > > > @@ -0,0 +1,28 @@
> > > > +/*
> > > > +    { dg-do compile }
> > > > +    { dg-options "-Wformat-overflow -std=c2x" }
> > > > +*/
> > > > +
> > > > +extern int sprintf (char* restrict, const char* restrict, ...);
> > > > +
> > > > +void test_warn () {
> > > > +
> > > > +    int n = __INT_MAX__;
> > > > +    char dst [5] = {0};
> > > > +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> > > > +
> > > > +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> > > > +
> > > > +}
> > > > +
> > > > +void test_no_warn () {
> > > > +
> > > > +    char dst [5] = {0};
> > > > +    int n = 8;
> > > > +    sprintf (dst, "%b", n);
> > > > +
> > > > +    char another_dst [34] = {0};
> > > > +    n = __INT_MAX__;
> > > > +    sprintf (another_dst, "%#b", n);
> > > > +
> > > > +}
> > > > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > > new file mode 100644
> > > > index 00000000000..c6b1d9062a6
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > > > @@ -0,0 +1,16 @@
> > > > +/*
> > > > +    { dg-do compile }
> > > > +    { dg-options "-Wformat-overflow -std=c11" }
> > > > +*/
> > > > +
> > > > +extern int sprintf (char* restrict, const char* restrict, ...);
> > > > +
> > > > +void test_no_warn () {
> > > > +
> > > > +    /*There is no reason to throw warning if std < c2x*/
> > > > +
> > > > +    char dst [5] = {0};
> > > > +    int n = 32;
> > > > +    sprintf (dst, "%b", n);
> > > > +
> > > > +}
> > > > --
> > > > 2.25.1
> > > >
> > >
> > >
> > > Marek
> > >
> > >
>
> > From 7b14a2aa909194841fa916f2db5d8aa1e4a6367e Mon Sep 17 00:00:00 2001
> > From: Frolov Daniil <frolov.da@phystech.edu>
> > Date: Thu, 7 Apr 2022 02:05:58 +0500
> > Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> >
> > gcc/ChangeLog:
> >
> >       * gimple-ssa-sprintf.cc (check_std_c2x): Handle base == 2.
> >       (fmtresult::type_max_digits): Handle base == 2.
> >       (tree_digits): Handle base == 2.
> >       (format_integer): Handle base == 2.
> >       (parse_directive): Add cases for %b and %B directives.
> >       (compute_format_length): Handle base == 2.
> >
> > gcc/testsuite/ChangeLog:
> >
> >       * gcc.dg/Wformat-overflow1.c: New test.
> >       * gcc.dg/Wformat-overflow2.c: New test.
> > ---
> >  gcc/gimple-ssa-sprintf.cc                | 40 +++++++++++++++++++-----
> >  gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28 +++++++++++++++++
> >  gcc/testsuite/gcc.dg/Wformat-overflow2.c | 16 ++++++++++
> >  3 files changed, 77 insertions(+), 7 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
> >  create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow2.c
> >
> > diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
> > index c93f12f90b5..6a14a004483 100644
> > --- a/gcc/gimple-ssa-sprintf.cc
> > +++ b/gcc/gimple-ssa-sprintf.cc
> > @@ -107,6 +107,15 @@ namespace {
> >
> >  static int warn_level;
> >
> > +/* The b_overflow_flag depends on the current standard when using gcc. */
> > +static bool b_overflow_flag;
> > +
> > +/* Check is current standard version equals C2X. */
> > +static bool check_std_c2x ()
> > +{
> > +  return !strcmp (lang_hooks.name, "GNU C2X");
> > +}
> > +
> >  /* The minimum, maximum, likely, and unlikely maximum number of bytes
> >     of output either a formatting function or an individual directive
> >     can result in.  */
> > @@ -535,6 +544,8 @@ fmtresult::type_max_digits (tree type, int base)
> >    unsigned prec = TYPE_PRECISION (type);
> >    switch (base)
> >      {
> > +    case 2:
> > +      return prec;
> >      case 8:
> >        return (prec + 2) / 3;
> >      case 10:
> > @@ -857,11 +868,11 @@ tree_digits (tree x, int base, HOST_WIDE_INT prec, bool plus, bool prefix)
> >
> >    /* Adjust a non-zero value for the base prefix, either hexadecimal,
> >       or, unless precision has resulted in a leading zero, also octal.  */
> > -  if (prefix && absval && (base == 16 || prec <= ndigs))
> > +  if (prefix && absval && (base == 2 || base == 16 || prec <= ndigs))
> >      {
> >        if (base == 8)
> >       res += 1;
> > -      else if (base == 16)
> > +      else if (base == 16 || base == 2) /* 0x...(0X...) and 0b...(0B...) */
> >       res += 2;
> >      }
> >
> > @@ -1229,6 +1240,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
> >      case 'u':
> >        base = 10;
> >        break;
> > +    case 'b':
> > +    case 'B':
> > +      base = 2;
> > +      break;
> >      case 'o':
> >        base = 8;
> >        break;
> > @@ -1351,10 +1366,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
> >
> >        /* Bump up the counters if WIDTH is greater than LEN.  */
> >        res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > -                                      (sign | maybebase) + (base == 16));
> > +                                      (sign | maybebase) + (base == 2 || base == 16));
> >        /* Bump up the counters again if PRECision is greater still.  */
> >        res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > -                                      (sign | maybebase) + (base == 16));
> > +                                      (sign | maybebase) + (base == 2 || base == 16));
> >
> >        return res;
> >      }
> > @@ -1503,7 +1518,7 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
> >         if (res.range.min == 1)
> >           res.range.likely += base == 8 ? 1 : 2;
> >         else if (res.range.min == 2
> > -                && base == 16
> > +                && (base == 16 || base == 2)
> >                  && (dir.width[0] == 2 || dir.prec[0] == 2))
> >           ++res.range.likely;
> >       }
> > @@ -1511,9 +1526,9 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
> >
> >    res.range.unlikely = res.range.max;
> >    res.adjust_for_width_or_precision (dir.width, dirtype, base,
> > -                                  (sign | maybebase) + (base == 16));
> > +                                  (sign | maybebase) + (base == 2 || base == 16));
> >    res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> > -                                  (sign | maybebase) + (base == 16));
> > +                                  (sign | maybebase) + (base == 2 || base == 16));
> >
> >    return res;
> >  }
> > @@ -3713,6 +3728,14 @@ parse_directive (call_info &info,
> >      case 'X':
> >        dir.fmtfunc = format_integer;
> >        break;
> > +
> > +    case 'b':
> > +    case 'B':
> > +      if (b_overflow_flag) {
> > +        dir.fmtfunc = format_integer;
> > +        break;
> > +      }
> > +      return 0;
> >
> >      case 'p':
> >        /* The %p output is implementation-defined.  It's possible
> > @@ -4038,6 +4061,9 @@ compute_format_length (call_info &info, format_result *res,
> >
> >    bool success = true;
> >
> > +  /* Check for GNU C2X standard */
> > +  b_overflow_flag = check_std_c2x ();
> > +
> >    for (const char *pf = info.fmtstr; ; ++dirno)
> >      {
> >        directive dir (&info, dirno);
> > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow1.c b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > new file mode 100644
> > index 00000000000..cf9766fae14
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow1.c
> > @@ -0,0 +1,28 @@
> > +/*
> > +    { dg-do compile }
> > +    { dg-options "-Wformat-overflow -std=c2x" }
> > +*/
> > +
> > +extern int sprintf (char* restrict, const char* restrict, ...);
> > +
> > +void test_warn () {
> > +
> > +    int n = __INT_MAX__;
> > +    char dst [5] = {0};
> > +    sprintf (dst, "%b", n);  /* { dg-warning "-Wformat-overflow" } */
> > +
> > +    sprintf (dst, "%#b", n); /* { dg-warning "-Wformat-overflow" } */
> > +
> > +}
> > +
> > +void test_no_warn () {
> > +
> > +    char dst [5] = {0};
> > +    int n = 8;
> > +    sprintf (dst, "%b", n);
> > +
> > +    char another_dst [34] = {0};
> > +    n = __INT_MAX__;
> > +    sprintf (another_dst, "%#b", n);
> > +
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/Wformat-overflow2.c b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > new file mode 100644
> > index 00000000000..c6b1d9062a6
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/Wformat-overflow2.c
> > @@ -0,0 +1,16 @@
> > +/*
> > +    { dg-do compile }
> > +    { dg-options "-Wformat-overflow -std=c11" }
> > +*/
> > +
> > +extern int sprintf (char* restrict, const char* restrict, ...);
> > +
> > +void test_no_warn () {
> > +
> > +    /*There is no reason to throw warning if std < c2x*/
> > +
> > +    char dst [5] = {0};
> > +    int n = 32;
> > +    sprintf (dst, "%b", n);
> > +
> > +}
> > --
> > 2.25.1
> >
>
>
> Marek
>

Thanks
  

On Mon, Aug 15, 2022 at 07:42:39PM +0300, Frolov Daniil wrote:
> вт, 12 апр. 2022 г. в 00:56, Marek Polacek <polacek@redhat.com>:
> 
> >
> > On Thu, Apr 07, 2022 at 02:10:48AM +0500, Frolov Daniil wrote:
> > > Hello! Thanks for your feedback. I've tried to take into account your
> > > comments. New patch applied to the letter.
> >
> > Thanks.
> >
> > > The only thing I have not removed is the check_std_c2x () function. From my
> > > point of view -Wformat-overflow shouldn't be thrown if the standard < C2X.
> > > So it's protection for false triggering.
> >
> > Sorry but I still think that is the wrong behavior.  If you want to warn
> > about C2X constructs in pre-C2X modes, use -Wpedantic.  But if you want
> > to use %b/%B as an extension in older dialects, that's OK too, so I don't
> > know why users would want -Wformat-overflow disabled in that case.  But
> > perhaps other people disagree with me.
> >
> Hi! Sorry for the late reply. If we want to look at it as on extension
> then I am agreed with you.
> Removed this function in new patch.

Thanks, the patch looks good to me (I have one comment though), but I can't
approve it.

> @@ -1229,6 +1231,10 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>      case 'u':
>        base = 10;
>        break;
> +    case 'b':
> +    case 'B':
> +      base = 2;
> +      break;
>      case 'o':
>        base = 8;
>        break;
> @@ -1348,13 +1354,12 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  	}
>  
>        res.range.unlikely = res.range.max;
> +      unsigned adj = (sign | maybebase) + (base == 2 || base == 16);

We have this same line here and ...
  
>        /* Bump up the counters if WIDTH is greater than LEN.  */
> -      res.adjust_for_width_or_precision (dir.width, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +      res.adjust_for_width_or_precision (dir.width, dirtype, base, adj);
>        /* Bump up the counters again if PRECision is greater still.  */
> -      res.adjust_for_width_or_precision (dir.prec, dirtype, base,
> -					 (sign | maybebase) + (base == 16));
> +      res.adjust_for_width_or_precision (dir.prec, dirtype, base, adj);
>  
>        return res;
>      }
> @@ -1503,17 +1508,16 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
>  	  if (res.range.min == 1)
>  	    res.range.likely += base == 8 ? 1 : 2;
>  	  else if (res.range.min == 2
> -		   && base == 16
> +		   && (base == 16 || base == 2)
>  		   && (dir.width[0] == 2 || dir.prec[0] == 2))
>  	    ++res.range.likely;
>  	}
>      }
>  
> +  unsigned adj = (sign | maybebase) + (base == 2 || base == 16);

... here, but sign, maybebase, and base couldn't have changed meanwhile.

So can we compute 'adj' just once after we've determined the base and sign,
and make it const?  And I think that if 'maybebase' is never changed in the
function, it ought to be made const as well.

Thanks,

Marek
  

On 9/1/22 03:41, Даниил Александрович Фролов via Gcc-patches wrote:
> Subject:
> Re: -Wformat-overflow handling for %b and %B directives in C2X standard
> From:
> Даниил Александрович Фролов via Gcc-patches <gcc-patches@gcc.gnu.org>
> Date:
> 9/1/22, 03:41
> 
> To:
> Marek Polacek <polacek@redhat.com>
> CC:
> "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>
> 
> 
>  From eb9e8241d99145020ec5c050c918c1ad3abc2701 Mon Sep 17 00:00:00 2001
> From: Frolov Daniil<frolov.da@phystech.edu>
> Date: Thu, 1 Sep 2022 10:55:01 +0300
> Subject: [PATCH] Support %b, %B for -Wformat-overflow (sprintf, snprintf)
> 
> gcc/ChangeLog:
> 
> 	* gimple-ssa-sprintf.cc (fmtresult::type_max_digits): Handle
> 	base == 2.
> 	(tree_digits): Likewise.
> 	(format_integer): Likewise.
> 	(parse_directive): Add cases for %b and %B directives.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wformat-overflow1.c: New test.
Thanks.  I've pushed this to the trunk.

Jeff