diff options
author | Gavin Andresen <gavinandresen@gmail.com> | 2012-06-18 10:48:40 -0400 |
---|---|---|
committer | Gavin Andresen <gavinandresen@gmail.com> | 2012-06-18 10:48:40 -0400 |
commit | 550c73f4c87825e8524f7f164e0bc3e80c6822b9 (patch) | |
tree | a674350fd0b5b4dd017ce5c553a268d960cbf287 /src | |
parent | 5204ca87e23fb94f31e733c63efcb46980ad42ad (diff) | |
parent | 31ac53fbdc2346876da201b9e1495565b38b46ba (diff) | |
download | bitcoin-550c73f4c87825e8524f7f164e0bc3e80c6822b9.tar.xz |
Merge branch 'signbugs' of https://github.com/wizeman/bitcoin
Resolved minor conflict in main.cpp
Diffstat (limited to 'src')
-rw-r--r-- | src/bignum.h | 22 | ||||
-rw-r--r-- | src/main.cpp | 2 | ||||
-rw-r--r-- | src/netbase.cpp | 4 | ||||
-rw-r--r-- | src/netbase.h | 2 | ||||
-rw-r--r-- | src/test/bignum_tests.cpp | 125 | ||||
-rw-r--r-- | src/util.h | 2 |
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; } |