| Message ID | 1495105840.1663.4.camel@stu.xidian.edu.cn |
|---|---|
| State | New |
| Headers | show |
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?
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.
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.
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) : __gnu_cxx::__numeric_traits<_ValueT>::__max; const __unsigned_type __smax = __max / __base; __unsigned_type __result = 0;