Mercurial > hg > bitcoin
changeset 2794:7b0cba675f5f draft
Merge branch 'signbugs' of https://github.com/wizeman/bitcoin
Resolved minor conflict in main.cpp
| author | Gavin Andresen <gavinandresen@gmail.com> |
|---|---|
| date | Mon, 18 Jun 2012 10:48:40 -0400 |
| parents | 402e92d96ee2 (current diff) c3e5b0c0ee7e (diff) |
| children | 8ed4bc27e882 |
| files | src/bignum.h src/main.cpp src/netbase.cpp src/netbase.h src/util.h |
| diffstat | 6 files changed, 148 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/src/bignum.h +++ b/src/bignum.h @@ -122,16 +122,30 @@ 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++) {
--- a/src/main.cpp +++ b/src/main.cpp @@ -2475,7 +2475,7 @@ 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;
--- a/src/netbase.cpp +++ b/src/netbase.cpp @@ -867,10 +867,10 @@ 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; }
--- a/src/netbase.h +++ b/src/netbase.h @@ -66,7 +66,7 @@ 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;
new file mode 100644 --- /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()