Merge branch 'signbugs' of https://github.com/wizeman/bitcoin

Resolved minor conflict in main.cpp

6 files changed, 148 insertions, 9 deletions

diff --git a/src/bignum.h b/src/bignum.h
index 307017b0ab..9af934051a 100644
--- a/src/bignum.h
+++ b/src/bignum.h
@@ -122,16 +122,30 @@ public:
             return (n > (unsigned long)std::numeric_limits<int>::max() ? std::numeric_limits<int>::min() : -(int)n);
     }
 
-    void setint64(int64 n)
+    void setint64(int64 sn)
     {
-        unsigned char pch[sizeof(n) + 6];
+        unsigned char pch[sizeof(sn) + 6];
         unsigned char* p = pch + 4;
-        bool fNegative = false;
-        if (n < (int64)0)
+        bool fNegative;
+        uint64 n;
+
+        if (sn < (int64)0)
         {
+            // We negate in 2 steps to avoid signed subtraction overflow,
+            // i.e. -(-2^63), which is an undefined operation and causes SIGILL
+            // when compiled with -ftrapv.
+            //
+            // Note that uint64_t n = sn, when sn is an int64_t, is a
+            // well-defined operation and n will be equal to sn + 2^64 when sn
+            // is negative.
+            n = sn;
             n = -n;
             fNegative = true;
+        } else {
+            n = sn;
+            fNegative = false;
         }
+
         bool fLeadingZeroes = true;
         for (int i = 0; i < 8; i++)
         {
diff --git a/src/main.cpp b/src/main.cpp
index be2733192e..0654ff8897 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -2475,7 +2475,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv)
                     static uint256 hashSalt;
                     if (hashSalt == 0)
                         hashSalt = GetRandHash();
-                    int64 hashAddr = addr.GetHash();
+                    uint64 hashAddr = addr.GetHash();
                     uint256 hashRand = hashSalt ^ (hashAddr<<32) ^ ((GetTime()+hashAddr)/(24*60*60));
                     hashRand = Hash(BEGIN(hashRand), END(hashRand));
                     multimap<uint256, CNode*> mapMix;
diff --git a/src/netbase.cpp b/src/netbase.cpp
index d1ead79ebb..2b918b2ff3 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -867,10 +867,10 @@ std::vector<unsigned char> CNetAddr::GetGroup() const
     return vchRet;
 }
 
-int64 CNetAddr::GetHash() const
+uint64 CNetAddr::GetHash() const
 {
     uint256 hash = Hash(&ip[0], &ip[16]);
-    int64 nRet;
+    uint64 nRet;
     memcpy(&nRet, &hash, sizeof(nRet));
     return nRet;
 }
diff --git a/src/netbase.h b/src/netbase.h
index 0f6fc9b499..a66d3e7f61 100644
--- a/src/netbase.h
+++ b/src/netbase.h
@@ -66,7 +66,7 @@ class CNetAddr
         std::string ToString() const;
         std::string ToStringIP() const;
         int GetByte(int n) const;
-        int64 GetHash() const;
+        uint64 GetHash() const;
         bool GetInAddr(struct in_addr* pipv4Addr) const;
         std::vector<unsigned char> GetGroup() const;
         int GetReachabilityFrom(const CNetAddr *paddrPartner = NULL) const;
diff --git a/src/test/bignum_tests.cpp b/src/test/bignum_tests.cpp
new file mode 100644
index 0000000000..8620f81f17
--- /dev/null
+++ b/src/test/bignum_tests.cpp
@@ -0,0 +1,125 @@
+#include <boost/test/unit_test.hpp>
+#include <limits>
+
+#include "bignum.h"
+#include "util.h"
+
+BOOST_AUTO_TEST_SUITE(bignum_tests)
+
+// Unfortunately there's no standard way of preventing a function from being
+// inlined, so we define a macro for it.
+//
+// You should use it like this:
+//   NOINLINE void function() {...}
+#if defined(__GNUC__)
+// This also works and will be defined for any compiler implementing gcc
+// extensions, such as clang and icc.
+#define NOINLINE __attribute__((noinline))
+#elif defined(_MSC_VER)
+#define NOINLINE __declspec(noinline)
+#else
+// We give out a warning because it impacts the correctness of one bignum test.
+#warning You should define NOINLINE for your compiler.
+#define NOINLINE
+#endif
+
+// For the following test case, it is useful to use additional tools.
+//
+// The simplest one to use is the compiler flag -ftrapv, which detects integer
+// overflows and similar errors. However, due to optimizations and compilers
+// taking advantage of undefined behavior sometimes it may not actually detect
+// anything.
+//
+// You can also use compiler-based stack protection to possibly detect possible
+// stack buffer overruns.
+//
+// For more accurate diagnostics, you can use an undefined arithmetic operation
+// detector such as the clang-based tool:
+//
+// "IOC: An Integer Overflow Checker for C/C++"
+//
+// Available at: http://embed.cs.utah.edu/ioc/
+//
+// It might also be useful to use Google's AddressSanitizer to detect
+// stack buffer overruns, which valgrind can't currently detect.
+
+// Let's force this code not to be inlined, in order to actually
+// test a generic version of the function. This increases the chance
+// that -ftrapv will detect overflows.
+NOINLINE void mysetint64(CBigNum& num, int64 n)
+{
+	num.setint64(n);
+}
+
+// For each number, we do 2 tests: one with inline code, then we reset the
+// value to 0, then the second one with a non-inlined function.
+BOOST_AUTO_TEST_CASE(bignum_setint64)
+{
+    int64 n;
+
+    {
+        n = 0;
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "0");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "0");
+    }
+    {
+        n = 1;
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "1");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "1");
+    }
+    {
+        n = -1;
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "-1");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "-1");
+    }
+    {
+        n = 5;
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "5");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "5");
+    }
+    {
+        n = -5;
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "-5");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "-5");
+    }
+    {
+        n = std::numeric_limits<int64>::min();
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "-9223372036854775808");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "-9223372036854775808");
+    }
+    {
+        n = std::numeric_limits<int64>::max();
+        CBigNum num(n);
+        BOOST_CHECK(num.ToString() == "9223372036854775807");
+        num.setulong(0);
+        BOOST_CHECK(num.ToString() == "0");
+        mysetint64(num, n);
+        BOOST_CHECK(num.ToString() == "9223372036854775807");
+    }
+}
+
+BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/util.h b/src/util.h
index c1c91bdbc9..c9b2b2f17c 100644
--- a/src/util.h
+++ b/src/util.h
@@ -272,7 +272,7 @@ inline int64 GetPerformanceCounter()
 #else
     timeval t;
     gettimeofday(&t, NULL);
-    nCounter = t.tv_sec * 1000000 + t.tv_usec;
+    nCounter = (int64) t.tv_sec * 1000000 + t.tv_usec;
 #endif
     return nCounter;
 }