From 3bd1d6645ee39bb6a6e7b016d4cbfc15188b1a00 Mon Sep 17 00:00:00 2001 From: "Ricardo M. Correia" Date: Mon, 14 May 2012 21:17:24 +0200 Subject: [PATCH 1/4] Fix signed subtraction overflow in CBigNum::setint64(). As noticed by sipa (Pieter Wuille), this can happen when CBigNum::setint64() is called with an integer value of INT64_MIN (-2^63). When compiled with -ftrapv, the program would crash. Otherwise, it would execute an undefined operation (although in practice, usually the correct one). --- src/bignum.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bignum.h b/src/bignum.h index e203b26a054..1c1c1fc10f8 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -130,7 +130,15 @@ public: if (sn < (int64)0) { - n = -sn; + // 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; From 7a161e48470ad3bfe1a4f497a7f0a1bed2f2b8a1 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 18 Jun 2012 20:35:10 +0000 Subject: [PATCH 2/4] CBigNum: Convert negative int64 values in a more well-defined way Since the minimum signed integer cannot be represented as positive so long as its type is signed, and it's not well-defined what happens if you make it unsigned before negating it, we instead increment the negative integer by 1, convert it, then increment the (now positive) unsigned integer by 1 to compensate --- src/bignum.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/bignum.h b/src/bignum.h index 1c1c1fc10f8..ef8423af9e1 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -130,15 +130,9 @@ public: 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; + // Since the minimum signed integer cannot be represented as positive so long as its type is signed, and it's not well-defined what happens if you make it unsigned before negating it, we instead increment the negative integer by 1, convert it, then increment the (now positive) unsigned integer by 1 to compensate + n = -(sn + 1); + ++n; fNegative = true; } else { n = sn; From 63f319353c6716e51f965043923779aee5251637 Mon Sep 17 00:00:00 2001 From: "Rune K. Svendsen" Date: Wed, 18 Jul 2012 09:37:05 +0200 Subject: [PATCH 3/4] Let the comment in GetBlockValue() reflect the uncertainty about the time interval between subsidy reductions --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index 7ce7c92e5d2..453b0f82579 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -690,7 +690,7 @@ int64 static GetBlockValue(int nHeight, int64 nFees) { int64 nSubsidy = 50 * COIN; - // Subsidy is cut in half every 4 years + // Subsidy is cut in half every 210000 blocks, which will occur approximately every 4 years nSubsidy >>= (nHeight / 210000); return nSubsidy + nFees; From ec9a3c04edc5180e20cb25ecd16558f449fa52e0 Mon Sep 17 00:00:00 2001 From: Philip Kaufmann Date: Sat, 21 Jul 2012 12:44:54 +0200 Subject: [PATCH 4/4] fix OpenSSL not written as proper noun in some comments --- src/bignum.h | 2 +- src/util.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bignum.h b/src/bignum.h index ef8423af9e1..7db89b30c4d 100644 --- a/src/bignum.h +++ b/src/bignum.h @@ -415,7 +415,7 @@ public: CBigNum& operator>>=(unsigned int shift) { // Note: BN_rshift segfaults on 64-bit if 2^shift is greater than the number - // if built on ubuntu 9.04 or 9.10, probably depends on version of openssl + // if built on ubuntu 9.04 or 9.10, probably depends on version of OpenSSL CBigNum a = 1; a <<= shift; if (BN_cmp(&a, this) > 0) diff --git a/src/util.cpp b/src/util.cpp index 6e31540f2ad..fb3bc64cc89 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -54,7 +54,7 @@ extern "C" void tss_cleanup_implemented() { } -// Init openssl library multithreading support +// Init OpenSSL library multithreading support static boost::interprocess::interprocess_mutex** ppmutexOpenSSL; void locking_callback(int mode, int i, const char* file, int line) { @@ -70,7 +70,7 @@ class CInit public: CInit() { - // Init openssl library multithreading support + // Init OpenSSL library multithreading support ppmutexOpenSSL = (boost::interprocess::interprocess_mutex**)OPENSSL_malloc(CRYPTO_num_locks() * sizeof(boost::interprocess::interprocess_mutex*)); for (int i = 0; i < CRYPTO_num_locks(); i++) ppmutexOpenSSL[i] = new boost::interprocess::interprocess_mutex(); @@ -86,7 +86,7 @@ public: } ~CInit() { - // Shutdown openssl library multithreading support + // Shutdown OpenSSL library multithreading support CRYPTO_set_locking_callback(NULL); for (int i = 0; i < CRYPTO_num_locks(); i++) delete ppmutexOpenSSL[i];