[committed] libstdc++: Fix __numeric_traits_integer<__int20> [PR 97798]

[committed] libstdc++: Fix __numeric_traits_integer<__int20> [PR 97798]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

The expression used to calculate the maximum value for an integer type
assumes that the number of bits in the value representation is always
sizeof(T) * CHAR_BIT. This is not true for the __int20 type on msp430,
which has only 20 bits in the value representation but 32 bits in the
object representation. This causes an integer overflow in a constant
expression, which is ill-formed.

This problem was already solved by DJ for std::numeric_limits<__int20>
by generalizing the helper macros to use a specified number of bits
instead of assuming sizeof(T) * CHAR_BIT. Then the INT_N_n types can
specify the number of bits using the __GLIBCXX_BITSIZE_INT_N_n macros
that the compiler defines.

I'm using a slightly different approach here. I've replaced the helper
macros entirely, and just expanded the calculations in the initializers
for the static data members. By reordering the data members we can reuse
__is_signed and __digits in the other initializers. This removes the
repetition of expanding __glibcxx_signed(T) and __glibcxx_digits(T)
multiple times in each initializer.

The __is_integer_nonstrict trait now defines a new constant, __width,
which is sizeof(T) * CHAR_BIT by default (defined as an enumerator so
that no storage is needed for a static data member). By specializing
__is_integer_nonstrict for the INT_N types that have padding bits, we
can provide the correct width via the __GLIBCXX_BITSIZE_INT_N_n macros.

libstdc++-v3/ChangeLog:

	PR libstdc++/97798
	* include/ext/numeric_traits.h (__glibcxx_signed)
	(__glibcxx_digits, __glibcxx_min, __glibcxx_max): Remove
	macros.
	(__is_integer_nonstrict::__width): Define new constant.
	(__numeric_traits_integer): Define constants in terms of each
	other and __is_integer_nonstrict::__width, rather than the
	removed macros.
	(_GLIBCXX_INT_N_TRAITS): Macro to define explicit
	specializations for non-standard integer types.

Bootstrapped a msp430-elf cross-compiler. Tested on powerpc64le-linux.

Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 8519 bytes --]

commit 7f851c33411fc39982c62a91fa93ec02981fd956
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 12 10:29:21 2020

    libstdc++: Fix __numeric_traits_integer<__int20> [PR 97798]
    
    The expression used to calculate the maximum value for an integer type
    assumes that the number of bits in the value representation is always
    sizeof(T) * CHAR_BIT. This is not true for the __int20 type on msp430,
    which has only 20 bits in the value representation but 32 bits in the
    object representation. This causes an integer overflow in a constant
    expression, which is ill-formed.
    
    This problem was already solved by DJ for std::numeric_limits<__int20>
    by generalizing the helper macros to use a specified number of bits
    instead of assuming sizeof(T) * CHAR_BIT. Then the INT_N_n types can
    specify the number of bits using the __GLIBCXX_BITSIZE_INT_N_n macros
    that the compiler defines.
    
    I'm using a slightly different approach here. I've replaced the helper
    macros entirely, and just expanded the calculations in the initializers
    for the static data members. By reordering the data members we can reuse
    __is_signed and __digits in the other initializers. This removes the
    repetition of expanding __glibcxx_signed(T) and __glibcxx_digits(T)
    multiple times in each initializer.
    
    The __is_integer_nonstrict trait now defines a new constant, __width,
    which is sizeof(T) * CHAR_BIT by default (defined as an enumerator so
    that no storage is needed for a static data member). By specializing
    __is_integer_nonstrict for the INT_N types that have padding bits, we
    can provide the correct width via the __GLIBCXX_BITSIZE_INT_N_n macros.
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/97798
            * include/ext/numeric_traits.h (__glibcxx_signed)
            (__glibcxx_digits, __glibcxx_min, __glibcxx_max): Remove
            macros.
            (__is_integer_nonstrict::__width): Define new constant.
            (__numeric_traits_integer): Define constants in terms of each
            other and __is_integer_nonstrict::__width, rather than the
            removed macros.
            (_GLIBCXX_INT_N_TRAITS): Macro to define explicit
            specializations for non-standard integer types.

diff --git a/libstdc++-v3/include/ext/numeric_traits.h b/libstdc++-v3/include/ext/numeric_traits.h
index 585ecc0ba9f5..c29f9f21d1aa 100644
--- a/libstdc++-v3/include/ext/numeric_traits.h
+++ b/libstdc++-v3/include/ext/numeric_traits.h
@@ -39,31 +39,23 @@ namespace __gnu_cxx _GLIBCXX_VISIBILITY(default)
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   // Compile time constants for builtin types.
-  // In C++98 std::numeric_limits member functions cannot be used for this.
-#define __glibcxx_signed(_Tp) ((_Tp)(-1) < 0)
-#define __glibcxx_digits(_Tp) \
-  (sizeof(_Tp) * __CHAR_BIT__ - __glibcxx_signed(_Tp))
-
-#define __glibcxx_min(_Tp) \
-  (__glibcxx_signed(_Tp) ? -__glibcxx_max(_Tp) - 1 : (_Tp)0)
-
-#define __glibcxx_max(_Tp) \
-  (__glibcxx_signed(_Tp) ? \
-   (((((_Tp)1 << (__glibcxx_digits(_Tp) - 1)) - 1) << 1) + 1) : ~(_Tp)0)
+  // In C++98 std::numeric_limits member functions are not constant expressions
+  // (that changed in C++11 with the addition of 'constexpr').
+  // Even for C++11, this header is smaller than <limits> and can be used
+  // when only is_signed, digits, min, or max values are needed for integers,
+  // or is_signed, digits10, max_digits10, or max_exponent10 for floats.
 
+  // Unlike __is_integer (and std::is_integral) this trait is true for
+  // non-standard built-in integer types such as __int128 and __int20.
   template<typename _Tp>
     struct __is_integer_nonstrict
     : public std::__is_integer<_Tp>
-    { };
+    {
+      using std::__is_integer<_Tp>::__value;
 
-#if defined __STRICT_ANSI__ && defined __SIZEOF_INT128__
-  // __is_integer<__int128> is false, but we still want to allow it here.
-  template<> struct __is_integer_nonstrict<__int128>
-  { enum { __value = 1 }; typedef std::__true_type __type; };
-
-  template<> struct __is_integer_nonstrict<unsigned __int128>
-  { enum { __value = 1 }; typedef std::__true_type __type; };
-#endif
+      // The number of bits in the value representation.
+      enum { __width = __value ? sizeof(_Tp) * __CHAR_BIT__ : 0 };
+    };
 
   template<typename _Value>
     struct __numeric_traits_integer
@@ -73,14 +65,17 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 		    "invalid specialization");
 #endif
 
-      // Only integers for initialization of member constant.
-      static const _Value __min = __glibcxx_min(_Value);
-      static const _Value __max = __glibcxx_max(_Value);
+      // NB: these two are also available in std::numeric_limits as compile
+      // time constants, but <limits> is big and we can avoid including it.
+      static const bool __is_signed = (_Value)(-1) < 0;
+      static const int __digits
+	= __is_integer_nonstrict<_Value>::__width - __is_signed;
 
-      // NB: these two also available in std::numeric_limits as compile
-      // time constants, but <limits> is big and we avoid including it.
-      static const bool __is_signed = __glibcxx_signed(_Value);
-      static const int __digits = __glibcxx_digits(_Value);      
+      // The initializers must be constants so that __max and __min are too.
+      static const _Value __max = __is_signed
+	? (((((_Value)1 << (__digits - 1)) - 1) << 1) + 1)
+	: ~(_Value)0;
+      static const _Value __min = __is_signed ? -__max - 1 : (_Value)0;
     };
 
   template<typename _Value>
@@ -95,16 +90,52 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Value>
     const int __numeric_traits_integer<_Value>::__digits;
 
+  // Enable __numeric_traits_integer for types where the __is_integer_nonstrict
+  // primary template doesn't give the right answer.
+#define _GLIBCXX_INT_N_TRAITS(T, WIDTH)			\
+  template<> struct __is_integer_nonstrict<T>		\
+  {							\
+    enum { __value = 1 };				\
+    typedef std::__true_type __type;			\
+    enum { __width = WIDTH };				\
+  };							\
+  template<> struct __is_integer_nonstrict<unsigned T>	\
+  {							\
+    enum { __value = 1 };				\
+    typedef std::__true_type __type;			\
+    enum { __width = WIDTH };				\
+  };
+
+  // We need to specify the width for some __intNN types because they
+  // have padding bits, e.g. the object representation of __int20 has 32 bits,
+  // but its width (number of bits in the value representation) is only 20.
+#if defined __GLIBCXX_TYPE_INT_N_0 && __GLIBCXX_BITSIZE_INT_N_0 % __CHAR_BIT__
+  _GLIBCXX_INT_N_TRAITS(__GLIBCXX_TYPE_INT_N_0, __GLIBCXX_BITSIZE_INT_N_0)
+#endif
+#if defined __GLIBCXX_TYPE_INT_N_1 && __GLIBCXX_BITSIZE_INT_N_1 % __CHAR_BIT__
+  _GLIBCXX_INT_N_TRAITS(__GLIBCXX_TYPE_INT_N_1, __GLIBCXX_BITSIZE_INT_N_1)
+#endif
+#if defined __GLIBCXX_TYPE_INT_N_2 && __GLIBCXX_BITSIZE_INT_N_2 % __CHAR_BIT__
+  _GLIBCXX_INT_N_TRAITS(__GLIBCXX_TYPE_INT_N_2, __GLIBCXX_BITSIZE_INT_N_2)
+#endif
+#if defined __GLIBCXX_TYPE_INT_N_3 && __GLIBCXX_BITSIZE_INT_N_3 % __CHAR_BIT__
+  _GLIBCXX_INT_N_TRAITS(__GLIBCXX_TYPE_INT_N_3, __GLIBCXX_BITSIZE_INT_N_3)
+#endif
+
+#if defined __STRICT_ANSI__ && defined __SIZEOF_INT128__
+  // In strict modes __is_integer<__int128> is false,
+  // but we still want to define __numeric_traits_integer<__int128>.
+  _GLIBCXX_INT_N_TRAITS(__int128, 128)
+#endif
+
+#undef _GLIBCXX_INT_N_TRAITS
+
 #if __cplusplus >= 201103L
+  /// Convenience alias for __numeric_traits<integer-type>.
   template<typename _Tp>
     using __int_traits = __numeric_traits_integer<_Tp>;
 #endif
 
-#undef __glibcxx_signed
-#undef __glibcxx_digits
-#undef __glibcxx_min
-#undef __glibcxx_max
-
 #define __glibcxx_floating(_Tp, _Fval, _Dval, _LDval) \
   (std::__are_same<_Tp, float>::__value ? _Fval \
    : std::__are_same<_Tp, double>::__value ? _Dval : _LDval)
@@ -120,10 +151,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   __glibcxx_floating(_Tp, __FLT_MAX_10_EXP__, __DBL_MAX_10_EXP__, \
 		     __LDBL_MAX_10_EXP__)
 
+  // N.B. this only supports float, double and long double (no __float128 etc.)
   template<typename _Value>
     struct __numeric_traits_floating
     {
-      // Only floating point types. See N1822. 
+      // Only floating point types. See N1822.
       static const int __max_digits10 = __glibcxx_max_digits10(_Value);
 
       // See above comment...
@@ -159,4 +191,4 @@ _GLIBCXX_END_NAMESPACE_VERSION
 #undef __glibcxx_digits10
 #undef __glibcxx_max_exponent10
 
-#endif 
+#endif

Re: [committed] libstdc++: Fix __numeric_traits_integer<__int20> [PR 97798]

[Jonathan Wakely]

[-- Attachment #1: Type: text/plain, Size: 139 bytes --]

Here's a small tweak to __numeric_traits that I decided to do after
the previous patch.

Tested on powerpc64le-linux. Committed to trunk.


[-- Attachment #2: patch.txt --]
[-- Type: text/x-patch, Size: 2571 bytes --]

commit d21776ef90361e66401cd99c8ff0d98b46d3b0d6
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Thu Nov 12 13:31:02 2020

    libstdc++: Simplify __numeric_traits definition
    
    This changes the __numeric_traits primary template to assume its
    argument is an integer type. For the three floating point types that are
    supported by __numeric_traits_floating an explicit specialization of
    __numeric_traits chooses the right base class.
    
    This improves the failure mode for using __numeric_traits with an
    unsupported type. Previously it would use __numeric_traits_floating as
    the base class, and give somewhat obscure errors for trying to access
    the static data members. Now it will use __numeric_traits_integer which
    has a static_assert to check for supported types.
    
    As a side effect of this change there is no need to instantiate
    __conditional_type to decide which base class to use.
    
    libstdc++-v3/ChangeLog:
    
            * include/ext/numeric_traits.h (__numeric_traits): Change
            primary template to always derive from __numeric_traits_integer.
            (__numeric_traits<float>, __numeric_traits<double>)
            (__numeric_traits<long double>): Add explicit specializations.

diff --git a/libstdc++-v3/include/ext/numeric_traits.h b/libstdc++-v3/include/ext/numeric_traits.h
index c29f9f21d1aa..2cac7f1d1edc 100644
--- a/libstdc++-v3/include/ext/numeric_traits.h
+++ b/libstdc++-v3/include/ext/numeric_traits.h
@@ -176,19 +176,32 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
   template<typename _Value>
     const int __numeric_traits_floating<_Value>::__max_exponent10;
 
-  template<typename _Value>
-    struct __numeric_traits
-    : public __conditional_type<__is_integer_nonstrict<_Value>::__value,
-				__numeric_traits_integer<_Value>,
-				__numeric_traits_floating<_Value> >::__type
-    { };
-
-_GLIBCXX_END_NAMESPACE_VERSION
-} // namespace
-
 #undef __glibcxx_floating
 #undef __glibcxx_max_digits10
 #undef __glibcxx_digits10
 #undef __glibcxx_max_exponent10
 
+  template<typename _Value>
+    struct __numeric_traits
+    : public __numeric_traits_integer<_Value>
+    { };
+
+  template<>
+    struct __numeric_traits<float>
+    : public __numeric_traits_floating<float>
+    { };
+
+  template<>
+    struct __numeric_traits<double>
+    : public __numeric_traits_floating<double>
+    { };
+
+  template<>
+    struct __numeric_traits<long double>
+    : public __numeric_traits_floating<long double>
+    { };
+
+_GLIBCXX_END_NAMESPACE_VERSION
+} // namespace
+
 #endif