diff mbox series

libstdc++-v3: Fix signed-overflow warning for newlib/ctype_base.h, PR116895

Message ID 20240930005639.50DB92043D@pchp3.se.axis.com
State Committed
Commit b1696ffd46872907b324996d4cdf28a2b9df209d
Headers
Series libstdc++-v3: Fix signed-overflow warning for newlib/ctype_base.h, PR116895 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gcc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gcc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 success Test passed

Commit Message

Hans-Peter Nilsson Sept. 30, 2024, 12:56 a.m. UTC 
  FWIW, I see "typedef char mask;" also for bionic and
openbsd.  Tested for cris-elf.

Ok to commit?

-- >8 --
There are 100+ regressions when running the g++ testsuite for newlib
targets (probably excepting ARM-based ones) e.g cris-elf after commit
r15-3859-g63a598deb0c9fc "libstdc++: #ifdef out #pragma GCC
system_header", which effectively no longer silences warnings for
gcc-installed system headers.  Some of these regressions are fixed by
r15-3928.  For the remaining ones, there's in g++.log:

FAIL: g++.old-deja/g++.robertl/eb79.C  -std=c++26 (test for excess errors)
Excess errors:
/gccobj/cris-elf/libstdc++-v3/include/cris-elf/bits/ctype_base.h:50:53: \
 warning: overflow in conversion from 'int' to 'std::ctype_base::mask' \
 {aka 'char'} changes value from '151' to '-105' [-Woverflow]

This is because the _B macro in newlib's ctype.h (from where the
"_<letter>" macros come) is bit 7, the sign-bit of 8-bit types:

#define	_B	0200

Using it in an int-expression that is then truncated to 8 bits will
"change" the value to negative for a default-signed char.  If this
code was created from scratch, it should have been an unsigned type,
however it's not advisable to change the type of mask as this affects
the API.  The least ugly option seems to be to silence the warning by
explict casts in the initializer, and for consistency, doing it for
all members.

	PR libstdc++/116895
	* config/os/newlib/ctype_base.h: Avoid signed-overflow warnings by
	explicitly casting initializer expressions to mask.
---
 libstdc++-v3/config/os/newlib/ctype_base.h | 24 +++++++++++-----------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Jonathan Wakely Sept. 30, 2024, 7:12 a.m. UTC | #1 
On Mon, 30 Sept 2024, 01:58 Hans-Peter Nilsson, <hp@axis.com> wrote:

> FWIW, I see "typedef char mask;" also for bionic and
> openbsd.  Tested for cris-elf.
>
> Ok to commit?
>

OK thanks



> -- >8 --
> There are 100+ regressions when running the g++ testsuite for newlib
> targets (probably excepting ARM-based ones) e.g cris-elf after commit
> r15-3859-g63a598deb0c9fc "libstdc++: #ifdef out #pragma GCC
> system_header", which effectively no longer silences warnings for
> gcc-installed system headers.  Some of these regressions are fixed by
> r15-3928.  For the remaining ones, there's in g++.log:
>
> FAIL: g++.old-deja/g++.robertl/eb79.C  -std=c++26 (test for excess errors)
> Excess errors:
> /gccobj/cris-elf/libstdc++-v3/include/cris-elf/bits/ctype_base.h:50:53: \
>  warning: overflow in conversion from 'int' to 'std::ctype_base::mask' \
>  {aka 'char'} changes value from '151' to '-105' [-Woverflow]
>
> This is because the _B macro in newlib's ctype.h (from where the
> "_<letter>" macros come) is bit 7, the sign-bit of 8-bit types:
>
> #define _B      0200
>
> Using it in an int-expression that is then truncated to 8 bits will
> "change" the value to negative for a default-signed char.  If this
> code was created from scratch, it should have been an unsigned type,
> however it's not advisable to change the type of mask as this affects
> the API.  The least ugly option seems to be to silence the warning by
> explict casts in the initializer, and for consistency, doing it for
> all members.
>
>         PR libstdc++/116895
>         * config/os/newlib/ctype_base.h: Avoid signed-overflow warnings by
>         explicitly casting initializer expressions to mask.
> ---
>  libstdc++-v3/config/os/newlib/ctype_base.h | 24 +++++++++++-----------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/libstdc++-v3/config/os/newlib/ctype_base.h
> b/libstdc++-v3/config/os/newlib/ctype_base.h
> index 309fdeea7731..5ec43a0c6803 100644
> --- a/libstdc++-v3/config/os/newlib/ctype_base.h
> +++ b/libstdc++-v3/config/os/newlib/ctype_base.h
> @@ -41,19 +41,19 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      // NB: Offsets into ctype<char>::_M_table force a particular size
>      // on the mask type. Because of this, we don't use an enum.
>      typedef char               mask;
> -    static const mask upper            = _U;
> -    static const mask lower    = _L;
> -    static const mask alpha    = _U | _L;
> -    static const mask digit    = _N;
> -    static const mask xdigit   = _X | _N;
> -    static const mask space    = _S;
> -    static const mask print    = _P | _U | _L | _N | _B;
> -    static const mask graph    = _P | _U | _L | _N;
> -    static const mask cntrl    = _C;
> -    static const mask punct    = _P;
> -    static const mask alnum    = _U | _L | _N;
> +    static const mask upper            = mask (_U);
> +    static const mask lower    = mask (_L);
> +    static const mask alpha    = mask (_U | _L);
> +    static const mask digit    = mask (_N);
> +    static const mask xdigit   = mask (_X | _N);
> +    static const mask space    = mask (_S);
> +    static const mask print    = mask (_P | _U | _L | _N | _B);
> +    static const mask graph    = mask (_P | _U | _L | _N);
> +    static const mask cntrl    = mask (_C);
> +    static const mask punct    = mask (_P);
> +    static const mask alnum    = mask (_U | _L | _N);
>  #if __cplusplus >= 201103L
> -    static const mask blank    = space;
> +    static const mask blank    = mask (space);
>  #endif
>    };
>
> --
> 2.30.2
>
>
diff mbox series

Patch

diff --git a/libstdc++-v3/config/os/newlib/ctype_base.h b/libstdc++-v3/config/os/newlib/ctype_base.h
index 309fdeea7731..5ec43a0c6803 100644
--- a/libstdc++-v3/config/os/newlib/ctype_base.h
+++ b/libstdc++-v3/config/os/newlib/ctype_base.h
@@ -41,19 +41,19 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
     // NB: Offsets into ctype<char>::_M_table force a particular size
     // on the mask type. Because of this, we don't use an enum.
     typedef char 		mask;
-    static const mask upper    	= _U;
-    static const mask lower 	= _L;
-    static const mask alpha 	= _U | _L;
-    static const mask digit 	= _N;
-    static const mask xdigit 	= _X | _N;
-    static const mask space 	= _S;
-    static const mask print 	= _P | _U | _L | _N | _B;
-    static const mask graph 	= _P | _U | _L | _N;
-    static const mask cntrl 	= _C;
-    static const mask punct 	= _P;
-    static const mask alnum 	= _U | _L | _N;
+    static const mask upper    	= mask (_U);
+    static const mask lower 	= mask (_L);
+    static const mask alpha 	= mask (_U | _L);
+    static const mask digit 	= mask (_N);
+    static const mask xdigit 	= mask (_X | _N);
+    static const mask space 	= mask (_S);
+    static const mask print 	= mask (_P | _U | _L | _N | _B);
+    static const mask graph 	= mask (_P | _U | _L | _N);
+    static const mask cntrl 	= mask (_C);
+    static const mask punct 	= mask (_P);
+    static const mask alnum 	= mask (_U | _L | _N);
 #if __cplusplus >= 201103L
-    static const mask blank 	= space;
+    static const mask blank 	= mask (space);
 #endif
   };