Re: [PATCH] Avoid signed overflow in num_get::_M_extract_int (PR libstdc++/67214)

[Jonathan Wakely <jwakely at redhat dot com>]

On 20/05/17 15:10 +0800, Xi Ruoyao wrote:
> On 2017-05-19 15:38 +0100, Jonathan Wakely wrote:
> > On 18/05/17 19:10 +0800, Xi Ruoyao wrote:
> > > This UB has been hiding so long...
> >
> > Indeed! Thanks for the patch.
> >
> > > 2017-03-11  Xi Ruoyao  <ryxi@stu.xidian.edu.cn>
> > >
> > > 	PR libstdc++/67214
> > > 	* include/bits/locale_facets.tcc (_M_extract_int):
> > > 	  Add explicit conversion to avoid signed overflow.
> > > ---
> > >  libstdc++-v3/include/bits/locale_facets.tcc | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/libstdc++-v3/include/bits/locale_facets.tcc b/libstdc++-v3/include/bits/locale_facets.tcc
> > > index 351190c..5f85d15 100644
> > > --- a/libstdc++-v3/include/bits/locale_facets.tcc
> > > +++ b/libstdc++-v3/include/bits/locale_facets.tcc
> > > @@ -470,7 +470,8 @@ _GLIBCXX_BEGIN_NAMESPACE_LDBL
> > >  	bool __testoverflow = false;
> > >  	const __unsigned_type __max =
> > >  	  (__negative && __gnu_cxx::__numeric_traits<_ValueT>::__is_signed)
> > > -	  ? -__gnu_cxx::__numeric_traits<_ValueT>::__min
> > > +	  ? -static_cast<__unsigned_type>(__gnu_cxx::
> > > +	                 __numeric_traits<_ValueT>::__min)
> >
> > Do we need to keep the negation, or can we just cast to
> > __unsigned_type?
>
> For 2's complement we can just cast to __unsigned_type.  But for
> clarity and other strange architectures I think we should keep
> the negation.

https://gcc.gnu.org/onlinedocs/gcc/Integers-implementation.html
"GCC only supports two's complement integer types"

I doubt that's ever going to change, but keeping the negation also
doesn't do any harm. I'll test and commit this, thanks.