Re: -Wformat-overflow handling for %b and %B directives in C2X standard
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 Date: 9/1/22, 03:41 To: Marek Polacek CC: "gcc-patches@gcc.gnu.org" From eb9e8241d99145020ec5c050c918c1ad3abc2701 Mon Sep 17 00:00:00 2001 From: Frolov Daniil 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
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
From eb9e8241d99145020ec5c050c918c1ad3abc2701 Mon Sep 17 00:00:00 2001
From: Frolov Daniil
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.
---
gcc/gimple-ssa-sprintf.cc| 41 +++-
gcc/testsuite/gcc.dg/Wformat-overflow1.c | 28
2 files changed, 53 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/Wformat-overflow1.c
diff --git a/gcc/gimple-ssa-sprintf.cc b/gcc/gimple-ssa-sprintf.cc
index a888b5ac7d5..1dd9b0dc46b 100644
--- a/gcc/gimple-ssa-sprintf.cc
+++ b/gcc/gimple-ssa-sprintf.cc
@@ -535,6 +535,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:
@@ -804,9 +806,9 @@ ilog (unsigned HOST_WIDE_INT x, int base)
/* Return the number of bytes resulting from converting into a string
the INTEGER_CST tree node X in BASE with a minimum of PREC digits.
PLUS indicates whether 1 for a plus sign should be added for positive
- numbers, and PREFIX whether the length of an octal ('O') or hexadecimal
- ('0x') prefix should be added for nonzero numbers. Return -1 if X cannot
- be represented. */
+ numbers, and PREFIX whether the length of an octal ('0') or hexadecimal
+ ('0x') or binary ('0b') prefix should be added for nonzero numbers.
+ Return -1 if X cannot be represented. */
static HOST_WIDE_INT
tree_digits (tree x, int base, HOST_WIDE_INT prec, bool plus, bool prefix)
@@ -857,11 +859,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)
{
- if (base == 8)
+ if (base == 8 && prec <= ndigs)
res += 1;
- else if (base == 16)
+ else if (base == 16 || base == 2) /* 0x...(0X...) or 0b...(0B...). */
res += 2;
}
@@ -1209,7 +1211,7 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
/* True when a conversion is preceded by a prefix indicating the base
of the argument (octal or hexadecimal). */
- bool maybebase = dir.get_flag ('#');
+ const bool maybebase = dir.get_flag ('#');
/* True when a signed conversion is preceded by a sign or space. */
bool maybesign = false;
@@ -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;
@@ -1240,6 +1246,8 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
gcc_unreachable ();
}
+ const unsigned adj = (sign | maybebase) + (base == 2 || base == 16);
+
/* The type of the "formal" argument expected by the directive. */
tree dirtype = NULL_TREE;
@@ -1350,11 +1358,9 @@ format_integer (const directive &dir, tree arg, pointer_query &ptr_qry)
res.range.unlikely = res.range.max;
/* 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 +1509,15 @@ 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;
}
}
res.range.unlikely = res.range.max;
- res.adjust_for_width_or_precision (dir.width, dirtype, base,
- (sign | maybebase) + (base == 16));
- res.adjust_for_width_or_precision (dir.prec, dirtype, base,
- (sign | maybebase) + (base == 16));
+ res.adjust_for_width_or_precision (dir.width, dirtype, base, adj);
+ res.adjust_for_width_or_precision (dir.prec, dirtype, base, adj);
return res;
}
@@ -3725,6 +3729,11 @@ parse_directive (call_info &info,
dir.fmtfunc = format_integer;
break;
+case 'b':
+case 'B':
+ dir.fmtfunc = format_integer;
+ break;
+
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
On Mon, Aug 15, 2022 at 07:42:39PM +0300, Frolov Daniil wrote: > вт, 12 апр. 2022 г. в 00:56, Marek Polacek : > > > > > 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
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
вт, 12 апр. 2022 г. в 00:56, Marek Polacek :
>
> 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 :
> >
> > > 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
> > > > 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 (pr
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
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 :
>
> > 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
> > > 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, t
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
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 :
> 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
> > 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
> ||
Re: -Wformat-overflow handling for %b and %B directives in C2X standard
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
> 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 (re
