Apple Clang 6.0 and numeric_limits<word128>::max() returns 0 #364

noloader · 2017-01-15T20:19:36Z

Testing on OS X 10.9 using Apple Clang 6.0. cryptest.sh reveals TestRounding() is failing one test case:

try
{
    const word128 h = ((word128)W64LIT(0xffffffffffffffff)) << 64U;
    const word128 v = h | (word128)W64LIT(0xfffffffffffffff9), b=0x08;
    word128 r=RoundUpToMultipleOf(v, b);
    fail = true;
}
catch(const Exception&)
{
    fail = false;
}

Here's the debug session with const word128 m = std::numeric_limits<word128>::max(); added for instrumentation.

Breakpoint 1, TestRounding () at validat0.cpp:456
456                     const word128 h = ((word128)W64LIT(0xffffffffffffffff)) << 64U;
(gdb) n
457                     const word128 v = h | (word128)W64LIT(0xfffffffffffffff9), b=0x08;
(gdb)
458                     const word128 m = std::numeric_limits<word128>::max();
(gdb)
459                     word128 r=RoundUpToMultipleOf(v, b);
(gdb) p v
$1 = 0xfffffffffffffffffffffffffffffff9
(gdb) p m
$2 = 0x00000000000000000000000000000000

The tests were put in place after Issue 360.

The text was updated successfully, but these errors were encountered:

anonimal · 2017-01-15T20:45:43Z

Testing on OS X 10.9

10.9 reached end of life last year. Will cryptopp continue 10.9 support? Does cryptopp have any 10.10+ machine(s) available to do testing on?

noloader · 2017-01-15T20:55:52Z

@anonimal, @mouse07410

10.9 reached end of life last year. Will cryptopp continue 10.9 support?

Yes, we support as far back as we reasonably can. OS X is a great example of why. Apple abandoned the platform, so the only way for users to stay up to date is to use up-to-date libraries.

Projects like Brew, Fink and MacPorts are a great help. But they depend on projects like Botan, Crypto++ and OpenSSL maintaining libraries for them to use.

Does cryptopp have any 10.10+ machine(s) available to do testing on?

I do not. I have OS X 10.5, 10.8 and 10.9. I believe Uri tests on OS X 10.10 or 10.11.

We also have gaps in testing on iPhones and iPads. I have 5 iPads, 1 iPhone and 1 iPod. All of them are older and abandoned by Apple. I think the latest OS I can test is iOS 8 or iOS 9.

I have no devices for testing Apple Watch and Apple TV.

I'm not sure what others have, but testing is welcomed on all of them.

anonimal · 2017-01-15T21:28:16Z

Ok. I have 10.12 to test on (also 10.10 and 10.11) but not the rest. I'm watching the repo and keep up when I can. If any OSX 10.10+ issues catch my eye then I'll see what I can do. If you happen to remember to ping me for 10.12 testing then I'll keep an eye out for that too.

mouse07410 · 2017-01-15T21:48:30Z

I was testing on Mac OS X 10.9, 10.10, and 10.11. Now I'm testing on 10.12, and am about to lose my only remaining 10.11 machine soon. But I still think it is beneficial to maintain compatibility with the older OS (and to a lesser extent - older building systems).

…

Sent from my iPad On Jan 15, 2017, at 16:04, Jeffrey Walton <notifications@github.com<mailto:notifications@github.com>> wrote: @anonimal<https://github.com/anonimal>, @mouse07410<https://github.com/mouse07410> 10.9 reached end of life last year. Will cryptopp continue 10.9 support? Yes, we support as far back as we reasonably can. OS X is a great example of why. Apple abandoned the platform, so the only way for users to stay up to date is to use up-to-date libraries. Projects like Brew, Fink and MacPorts are a great help. But they depend on projects like Botan, Crypto++ and OpenSSL maintaining libraries for them to use. Does cryptopp have any 10.10+ machine(s) available to do testing on? I do not. I have OS X 10.5, 10.8 and 10.9. I believe Uri tests on OS X 10.10 or 10.11. We also have gaps in testing on iPhones and iPads. I have 5 iPads, 1 iPhone and 1 iPod. All of them are older and abandoned by Apple. I think the latest OS I can test is iOS 8 or iOS 9. I have no devices for testing Apple watch and Apple TV. I'm not sure what other users have, but testing is welcomed on all of them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#364 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AFpi-fXI92KIzGwgLLfImGvzf-DZmDS3ks5rSonhgaJpZM4LkBQG>.

noloader · 2017-01-15T22:33:52Z

@anonimal , @mouse07410,

The issue appears to be bound to Apple Clang. Testing LLVM Clang is OK.

How does the program perform on some of the later OS X's? I'm also interested in versions of Apple Clang which fail. You can compile it with c++ -Wall test.cxx -o test.exe.

$ cat test.cxx
#include <iostream>
#include <limits>

int main(int argc, char* argv[])
{
#if (__SIZEOF_INT128__ >= 16)
    std::cout << "__int128 is available" << std::endl;
#else
    std::cout << "__int128 is not available" << std::endl;
#endif

    unsigned __int128 m = std::numeric_limits<unsigned __int128>::max();
    if (m == 0)
        std::cout << "numeric_limits<unsigned __int128>::max() is 0" << std::endl;
    else
        std::cout << "numeric_limits<unsigned __int128>::max() is not 0" << std::endl;

    return 0;
}

noloader · 2017-01-16T00:12:38Z

It looks like we need the following patch. It handles missing numeric_limits for Clang without interfering with the std namespace. It also adds documentation on the use of signed and unsigned types, and asserts in debug builds.

$ cat misc.diff
diff --git a/misc.h b/misc.h
index 6b8a2fa..fde92b5 100644
--- a/misc.h
+++ b/misc.h
@@ -116,6 +116,35 @@

 #endif // CRYPTOPP_DOXYGEN_PROCESSING

+// NumericLimitsMin and NumericLimitsMax added for word128 types,
+//   see http://github.com/weidai11/cryptopp/issues/364
+ANONYMOUS_NAMESPACE_BEGIN
+template<class T>
+T NumericLimitsMin()
+{
+       CRYPTOPP_ASSERT(std::numeric_limits<T>::is_specialized);
+       return std::numeric_limits<T>::min();
+};
+template<class T>
+T NumericLimitsMax()
+{
+       CRYPTOPP_ASSERT(std::numeric_limits<T>::is_specialized);
+       return std::numeric_limits<T>::max();
+};
+#if defined(CRYPTOPP_WORD128_AVAILABLE)
+template<>
+CryptoPP::word128 NumericLimitsMin()
+{
+       return 0;
+}
+template<>
+CryptoPP::word128 NumericLimitsMax()
+{
+       return (((CryptoPP::word128)W64LIT(0xffffffffffffffff)) << 64U) | (CryptoPP::word128)W64LIT(0xffffffffffffffff);
+}
+#endif
+ANONYMOUS_NAMESPACE_END
+
 NAMESPACE_BEGIN(CryptoPP)

 // Forward declaration for IntToString specialization
@@ -885,9 +914,21 @@ inline T2 ModPowerOf2(const T1 &a, const T2 &b)
 //! \returns the possibly unmodified value \n
 //! \details RoundDownToMultipleOf is effectively a floor function based on m. The function returns
 //!   the value <tt>n - n\%m</tt>. If n is a multiple of m, then the original value is returned.
+//! \note <tt>T1</tt> and <tt>T2</tt> should be usigned arithmetic types. If <tt>T1</tt> or
+//!   <tt>T2</tt> is signed, then the value should be non-negative. The library asserts in
+//!   debug builds when practical, but allows you to perform the operation in release builds.
 template <class T1, class T2>
 inline T1 RoundDownToMultipleOf(const T1 &n, const T2 &m)
 {
+       // http://github.com/weidai11/cryptopp/issues/364
+#if !defined(CRYPTOPP_APPLE_CLANG_VERSION) || (CRYPTOPP_APPLE_CLANG_VERSION >= 80000)
+       CRYPTOPP_ASSERT(std::numeric_limits<T1>::is_integer);
+       CRYPTOPP_ASSERT(std::numeric_limits<T2>::is_integer);
+#endif
+
+       CRYPTOPP_ASSERT(!std::numeric_limits<T1>::is_signed || n > 0);
+       CRYPTOPP_ASSERT(!std::numeric_limits<T2>::is_signed || m > 0);
+
        if (IsPowerOf2(m))
                return n - ModPowerOf2(n, m);
        else
@@ -901,10 +942,22 @@ inline T1 RoundDownToMultipleOf(const T1 &n, const T2 &m)
 //! \details RoundUpToMultipleOf is effectively a ceiling function based on m. The function
 //!   returns the value <tt>n + n\%m</tt>. If n is a multiple of m, then the original value is
 //!   returned. If the value n would overflow, then an InvalidArgument exception is thrown.
+//! \note <tt>T1</tt> and <tt>T2</tt> should be usigned arithmetic types. If <tt>T1</tt> or
+//!   <tt>T2</tt> is signed, then the value should be non-negative. The library asserts in
+//!   debug builds when practical, but allows you to perform the operation in release builds.
 template <class T1, class T2>
 inline T1 RoundUpToMultipleOf(const T1 &n, const T2 &m)
 {
-       if (std::numeric_limits<T1>::max() - m + 1 < n)
+       // http://github.com/weidai11/cryptopp/issues/364
+#if !defined(CRYPTOPP_APPLE_CLANG_VERSION) || (CRYPTOPP_APPLE_CLANG_VERSION >= 80000)
+       CRYPTOPP_ASSERT(std::numeric_limits<T1>::is_integer);
+       CRYPTOPP_ASSERT(std::numeric_limits<T2>::is_integer);
+#endif
+
+       CRYPTOPP_ASSERT(!std::numeric_limits<T1>::is_signed || n > 0);
+       CRYPTOPP_ASSERT(!std::numeric_limits<T2>::is_signed || m > 0);
+
+       if (NumericLimitsMax<T1>() - m + 1 < n)
                throw InvalidArgument("RoundUpToMultipleOf: integer overflow");
        return RoundDownToMultipleOf(T1(n+m-1), m);
 }

Also see Issue 360 and http://stackoverflow.com/q/41666815

noloader · 2017-01-16T00:43:15Z

I believe this was cleared at Commit b274f062022eeb86 (Clang and GCC) and Commit 1d391c190d8c4869 (MSVC).

RoundUpToMultipleOf and RoundDownToMultipleOf are tricky functions. We want them to be minimal so they are fast. The library also uses unsigned types or non-negative values, so the library's use is safe. However, that does not mean users are doing the same.

We added more documentation and some asserts to help folks use the functions as intended, and not operate outside the bounds.

Apple Clang has the problem. Limit the work around to just Apple Clang.

noloader added the Bug label Jan 15, 2017

noloader referenced this issue Jan 16, 2017

Fix RoundUpToMultipleOf for Apple Clang 6.0 (Issue 364)

b274f06

Also see Issue 360 and http://stackoverflow.com/q/41666815

noloader closed this as completed Jan 16, 2017

noloader referenced this issue Jan 16, 2017

Make NumericLimitsMin and NumericLimitsMax available to MSVC (Issue 364)

1d391c1

noloader mentioned this issue Jan 25, 2017

Support for AES-256-CBC with W3C padding scheme ( http://www.w3.org/2001/04/xmlenc#aes256-cbc ) #368

Merged

danielweck mentioned this issue Jan 25, 2017

Future concern? Crypto++ CryptoPP on iOS / OSX readium/readium-lcp-client#27

Open

noloader referenced this issue Jun 23, 2023

Guard use of std::numeric_limits<word128>::max()

3b1cf30

Apple Clang has the problem. Limit the work around to just Apple Clang.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apple Clang 6.0 and numeric_limits<word128>::max() returns 0 #364

Apple Clang 6.0 and numeric_limits<word128>::max() returns 0 #364

noloader commented Jan 15, 2017 •

edited

anonimal commented Jan 15, 2017

noloader commented Jan 15, 2017 •

edited

anonimal commented Jan 15, 2017

mouse07410 commented Jan 15, 2017 via email

noloader commented Jan 15, 2017 •

edited

noloader commented Jan 16, 2017 •

edited

noloader commented Jan 16, 2017

Apple Clang 6.0 and numeric_limits<word128>::max() returns 0 #364

Apple Clang 6.0 and numeric_limits<word128>::max() returns 0 #364

Comments

noloader commented Jan 15, 2017 • edited

anonimal commented Jan 15, 2017

noloader commented Jan 15, 2017 • edited

anonimal commented Jan 15, 2017

mouse07410 commented Jan 15, 2017 via email

noloader commented Jan 15, 2017 • edited

noloader commented Jan 16, 2017 • edited

noloader commented Jan 16, 2017

noloader commented Jan 15, 2017 •

edited

noloader commented Jan 15, 2017 •

edited

noloader commented Jan 15, 2017 •

edited

noloader commented Jan 16, 2017 •

edited