From c4fbbd0aa116a0477ff4d1c93c722f37b9528c0d Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Mon, 13 Oct 2014 23:43:37 +0300 Subject: [PATCH 1/4] New assertion styles. --- libdevcore/Common.h | 48 +++++++++++++++ libdevcore/debugbreak.h | 121 +++++++++++++++++++++++++++++++++++++ libdevcrypto/Common.cpp | 3 +- libethereum/BlockChain.cpp | 12 +++- 4 files changed, 180 insertions(+), 4 deletions(-) create mode 100644 libdevcore/debugbreak.h diff --git a/libdevcore/Common.h b/libdevcore/Common.h index 332081ec8..cbac82b40 100644 --- a/libdevcore/Common.h +++ b/libdevcore/Common.h @@ -38,6 +38,7 @@ #include #include #include "vector_ref.h" +#include "debugbreak.h" // CryptoPP defines byte in the global namespace, so must we. using byte = uint8_t; @@ -103,4 +104,51 @@ inline unsigned int toLog2(u256 _x) return ret; } +// Assertions... + +#if defined(_MSC_VER) +#define ETH_FUNC __FUNCSIG__ +#elif defined(__GNUC__) +#define ETH_FUNC __PRETTY_FUNCTION__ +#else +#define ETH_FUNC __func__ +#endif + +#define asserts(A) ::dev::assertAux(A, #A, __LINE__, __FILE__, ETH_FUNC) +#define assertsEqual(A, B) ::dev::assertEqualAux(A, B, #A, #B, __LINE__, __FILE__, ETH_FUNC) + +#if defined(__GNUC__) +__attribute__((gnu_inline, always_inline)) +#endif +inline bool assertAux(bool _a, char const* _aStr, unsigned _line, char const* _file, char const* _func) +{ + bool ret = _a; + if (!ret) + { + std::cerr << "Assertion failed:" << _aStr << " [func=" << _func << ", line=" << _line << ", file=" << _file << "]" << std::endl; +#if ETH_DEBUG + debug_break(); +#endif + } + return !ret; +} + +template +#if defined(__GNUC__) +__attribute__((gnu_inline, always_inline)) +#endif +inline bool assertEqualAux(A const& _a, B const& _b, char const* _aStr, char const* _bStr, unsigned _line, char const* _file, char const* _func) +{ + bool ret = _a == _b; + if (!ret) + { + std::cerr << "Assertion failed: " << _aStr << " == " << _bStr << " [func=" << _func << ", line=" << _line << ", file=" << _file << "]" << std::endl; + std::cerr << " Fail equality: " << _a << "==" << _b << std::endl; +#if ETH_DEBUG + debug_break(); +#endif + } + return !ret; +} + } diff --git a/libdevcore/debugbreak.h b/libdevcore/debugbreak.h new file mode 100644 index 000000000..57d9d8cde --- /dev/null +++ b/libdevcore/debugbreak.h @@ -0,0 +1,121 @@ +/* Copyright (c) 2013, Scott Tsai + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * + * 1. Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright notice, + * this list of conditions and the following disclaimer in the documentation + * and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + */ + +#ifndef DEBUG_BREAK_H +#define DEBUG_BREAK_H + +#ifdef _MSC_VER + +#define debug_break __debugbreak + +#else + +#include +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +enum { + /* gcc optimizers consider code after __builtin_trap() dead. + * Making __builtin_trap() unsuitable for breaking into the debugger */ + DEBUG_BREAK_PREFER_BUILTIN_TRAP_TO_SIGTRAP = 0, +}; + +#if defined(__i386__) || defined(__x86_64__) +enum { HAVE_TRAP_INSTRUCTION = 1, }; +__attribute__((gnu_inline, always_inline)) +static void __inline__ trap_instruction(void) +{ + __asm__ volatile("int $0x03"); +} +#elif defined(__thumb__) +enum { HAVE_TRAP_INSTRUCTION = 1, }; +/* FIXME: handle __THUMB_INTERWORK__ */ +__attribute__((gnu_inline, always_inline)) +static void __inline__ trap_instruction(void) +{ + /* See 'arm-linux-tdep.c' in GDB source. + * Both instruction sequences below works. */ +#if 1 + /* 'eabi_linux_thumb_le_breakpoint' */ + __asm__ volatile(".inst 0xde01"); +#else + /* 'eabi_linux_thumb2_le_breakpoint' */ + __asm__ volatile(".inst.w 0xf7f0a000"); +#endif + + /* Known problem: + * After a breakpoint hit, can't stepi, step, or continue in GDB. + * 'step' stuck on the same instruction. + * + * Workaround: a new GDB command, + * 'debugbreak-step' is defined in debugbreak-gdb.py + * that does: + * (gdb) set $instruction_len = 2 + * (gdb) tbreak *($pc + $instruction_len) + * (gdb) jump *($pc + $instruction_len) + */ +} +#elif defined(__arm__) && !defined(__thumb__) +enum { HAVE_TRAP_INSTRUCTION = 1, }; +__attribute__((gnu_inline, always_inline)) +static void __inline__ trap_instruction(void) +{ + /* See 'arm-linux-tdep.c' in GDB source, + * 'eabi_linux_arm_le_breakpoint' */ + __asm__ volatile(".inst 0xe7f001f0"); + /* Has same known problem and workaround + * as Thumb mode */ +} +#else +enum { HAVE_TRAP_INSTRUCTION = 0, }; +#endif + +__attribute__((gnu_inline, always_inline)) +static void __inline__ debug_break(void) +{ + if (HAVE_TRAP_INSTRUCTION) { + trap_instruction(); + } else if (DEBUG_BREAK_PREFER_BUILTIN_TRAP_TO_SIGTRAP) { + /* raises SIGILL on Linux x86{,-64}, to continue in gdb: + * (gdb) handle SIGILL stop nopass + * */ + __builtin_trap(); + } else { + raise(SIGTRAP); + } +} + +#ifdef __cplusplus +} +#endif + +#endif + +#endif diff --git a/libdevcrypto/Common.cpp b/libdevcrypto/Common.cpp index dd4d6961c..8863352a4 100644 --- a/libdevcrypto/Common.cpp +++ b/libdevcrypto/Common.cpp @@ -39,7 +39,8 @@ Address dev::toAddress(Secret _private) if (!ok) return Address(); ok = secp256k1_ecdsa_pubkey_create(pubkey, &pubkeylen, _private.data(), 0); - assert(pubkeylen == 65); + if (asserts(pubkeylen == 65)) + return Address(); if (!ok) return Address(); ok = secp256k1_ecdsa_pubkey_verify(pubkey, 65); diff --git a/libethereum/BlockChain.cpp b/libethereum/BlockChain.cpp index 6f0af911c..2d904ed7f 100644 --- a/libethereum/BlockChain.cpp +++ b/libethereum/BlockChain.cpp @@ -439,11 +439,17 @@ void BlockChain::checkConsistency() h256 h((byte const*)it->key().data(), h256::ConstructFromPointer); auto dh = details(h); auto p = dh.parent; - if (p != h256()) + if (p != h256() && p != m_genesisHash) { auto dp = details(p); -// assert(contains(dp.children, h)); // WTF? - assert(dp.number == dh.number - 1); + if (asserts(contains(dp.children, h))) + { + cnote << "Apparently the database is corrupt. Not much we can do at this stage..."; + } + if (assertsEqual(dp.number, dh.number - 1)) + { + cnote << "Apparently the database is corrupt. Not much we can do at this stage..."; + } } } delete it; From 1faaed33955df904ebf087bde745833839bbd576 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 14 Oct 2014 00:02:37 +0300 Subject: [PATCH 2/4] Make bad protocol disconnects a bit more obvious. --- libethereum/BlockChain.cpp | 2 +- libethereum/EthereumPeer.cpp | 8 ++++++++ libp2p/Session.cpp | 4 +++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/libethereum/BlockChain.cpp b/libethereum/BlockChain.cpp index 2d904ed7f..d07bb71f3 100644 --- a/libethereum/BlockChain.cpp +++ b/libethereum/BlockChain.cpp @@ -439,7 +439,7 @@ void BlockChain::checkConsistency() h256 h((byte const*)it->key().data(), h256::ConstructFromPointer); auto dh = details(h); auto p = dh.parent; - if (p != h256() && p != m_genesisHash) + if (p != h256() && p != m_genesisHash) // TODO: for some reason the genesis details with the children get squished. not sure why. { auto dp = details(p); if (asserts(contains(dp.children, h))) diff --git a/libethereum/EthereumPeer.cpp b/libethereum/EthereumPeer.cpp index 5817aa434..6c10524ca 100644 --- a/libethereum/EthereumPeer.cpp +++ b/libethereum/EthereumPeer.cpp @@ -288,6 +288,8 @@ void EthereumPeer::attemptSync() bool EthereumPeer::interpret(unsigned _id, RLP const& _r) { + try + { switch (_id) { case StatusPacket: @@ -509,5 +511,11 @@ bool EthereumPeer::interpret(unsigned _id, RLP const& _r) default: return false; } + } + catch (std::exception const& _e) + { + clogS(NetWarn) << "Peer causing an exception:" << _e.what() << _r; + } + return true; } diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index 1ab4ca123..327233820 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -297,6 +297,7 @@ bool Session::interpret(RLP const& _r) peerAddress = bi::address_v4(_r[i][0].toHash>().asArray()); else { + cwarn << "Received bad peer packet:" << _r; disconnect(BadProtocol); return true; } @@ -372,7 +373,7 @@ bool Session::interpret(RLP const& _r) } catch (std::exception const& _e) { - clogS(NetWarn) << "Peer causing an exception:" << _e.what(); + clogS(NetWarn) << "Peer causing an exception:" << _e.what() << _r; disconnect(BadProtocol); return true; } @@ -579,6 +580,7 @@ void Session::doRead() if (!interpret(r)) { // error - bad protocol + clogS(NetWarn) << "Couldn't interpret packet." << RLP(r); disconnect(BadProtocol); return; } From 9600da565e6da158a49633de7f8c0d4e0983611f Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 14 Oct 2014 00:12:55 +0300 Subject: [PATCH 3/4] Don't be quite so punitive when peer sontinues sending packets for a protocol they signed up to. --- libp2p/Session.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index 327233820..5a66064f8 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -581,7 +581,7 @@ void Session::doRead() { // error - bad protocol clogS(NetWarn) << "Couldn't interpret packet." << RLP(r); - disconnect(BadProtocol); + // Just wasting our bandwidth - perhaps reduce rating? return; } } From 8447f356087941282e0ad9e567eddf261a800ca8 Mon Sep 17 00:00:00 2001 From: subtly Date: Tue, 14 Oct 2014 01:09:06 +0200 Subject: [PATCH 4/4] "Fix" for duplicate symbols w libdevcore/Common* --- libdevcore/Common.cpp | 16 ++++++++++++++++ libdevcore/Common.h | 15 ++------------- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/libdevcore/Common.cpp b/libdevcore/Common.cpp index 41b75f5ec..a463cf4b1 100644 --- a/libdevcore/Common.cpp +++ b/libdevcore/Common.cpp @@ -29,5 +29,21 @@ namespace dev char const* Version = "0.7.3"; +#if defined(__GNUC__) +__attribute__((gnu_inline, always_inline)) +#endif +inline bool assertAux(bool _a, char const* _aStr, unsigned _line, char const* _file, char const* _func) +{ + bool ret = _a; + if (!ret) + { + std::cerr << "Assertion failed:" << _aStr << " [func=" << _func << ", line=" << _line << ", file=" << _file << "]" << std::endl; +#if ETH_DEBUG + debug_break(); +#endif + } + return !ret; +} + } diff --git a/libdevcore/Common.h b/libdevcore/Common.h index cbac82b40..f0f2c192b 100644 --- a/libdevcore/Common.h +++ b/libdevcore/Common.h @@ -120,19 +120,8 @@ inline unsigned int toLog2(u256 _x) #if defined(__GNUC__) __attribute__((gnu_inline, always_inline)) #endif -inline bool assertAux(bool _a, char const* _aStr, unsigned _line, char const* _file, char const* _func) -{ - bool ret = _a; - if (!ret) - { - std::cerr << "Assertion failed:" << _aStr << " [func=" << _func << ", line=" << _line << ", file=" << _file << "]" << std::endl; -#if ETH_DEBUG - debug_break(); -#endif - } - return !ret; -} - +inline bool assertAux(bool _a, char const* _aStr, unsigned _line, char const* _file, char const* _func); + template #if defined(__GNUC__) __attribute__((gnu_inline, always_inline))