bitcoind minor bug in wallet and possible fix
3 messages, 30 May 2014. Started by Toshi Morita.
Toshi Morita (2), Mark Friedenbach
I ran bitcoind under valgrind and found a place where it references an uninitialized variable in some cases:
tm@tm-VirtualBox:~/bitcoind/bitcoin/src$ valgrind ./bitcoind ==2337== Memcheck, a memory error detector ==2337== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. ==2337== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info ==2337== Command: ./bitcoind ==2337== ==2337== Conditional jump or move depends on uninitialised value(s) ==2337== at 0x319176: CWallet::LoadKeyMetadata(CPubKey const&, CKeyMetadata const&) (wallet.cpp:110) ==2337== by 0x33645A: ReadKeyValue(CWallet*, CDataStream&, CDataStream&, CWalletScanState&, std::string&, std::string&) (walletdb.cpp:509) ==2337== by 0x3374F0: CWalletDB::LoadWallet(CWallet*) (walletdb.cpp:623) ==2337== by 0x3218FD: CWallet::LoadWallet(bool&) (wallet.cpp:1485) ==2337== by 0x157F16: AppInit2(boost::thread_group&) (init.cpp:958) ==2337== by 0x140142: AppInit(int, char**) (bitcoind.cpp:143) ==2337== by 0x13649E: main (bitcoind.cpp:180) ==2337==
The bug occurs here because nTimeFirstKey is not initialized when the wallet is instantiated:
wallet.cpp:63
if (!nTimeFirstKey || nCreationTime < nTimeFirstKey)
nTimeFirstKey = nCreationTime;
I fixed it in my fork:
diff --git a/src/wallet.h b/src/wallet.h
index 9607415..b78045f 100644
--- a/src/wallet.h
+++ b/src/wallet.h
@@ -163,6 +163,7 @@ public:
nOrderPosNext = 0;
nNextResend = 0;
nLastResend = 0;
+ nTimeFirstKey = 0;
}
If this fix is ok please pull from my GitHub fork; username on GitHub is tm314159.
Toshi
Please make a pull request on github. It'll likely get merged quickly. On May 29, 2014 5:04 PM, "Toshi Morita" <toshi@peernova•com> wrote:
Quoted text (60 lines)
> I ran bitcoind under valgrind and found a place where it references an > uninitialized variable in some cases: > > tm@tm-VirtualBox:~/bitcoind/bitcoin/src$ valgrind ./bitcoind > ==2337== Memcheck, a memory error detector > ==2337== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. > ==2337== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info > ==2337== Command: ./bitcoind > ==2337== > ==2337== Conditional jump or move depends on uninitialised value(s) > ==2337== at 0x319176: CWallet::LoadKeyMetadata(CPubKey const&, > CKeyMetadata const&) (wallet.cpp:110) > ==2337== by 0x33645A: ReadKeyValue(CWallet*, CDataStream&, > CDataStream&, CWalletScanState&, std::string&, std::string&) > (walletdb.cpp:509) > ==2337== by 0x3374F0: CWalletDB::LoadWallet(CWallet*) (walletdb.cpp:623) > ==2337== by 0x3218FD: CWallet::LoadWallet(bool&) (wallet.cpp:1485) > ==2337== by 0x157F16: AppInit2(boost::thread_group&) (init.cpp:958) > ==2337== by 0x140142: AppInit(int, char**) (bitcoind.cpp:143) > ==2337== by 0x13649E: main (bitcoind.cpp:180) > ==2337== > > The bug occurs here because nTimeFirstKey is not initialized when the > wallet is instantiated: > > wallet.cpp:63 > if (!nTimeFirstKey || nCreationTime < nTimeFirstKey) > nTimeFirstKey = nCreationTime; > > > I fixed it in my fork: > > diff --git a/src/wallet.h b/src/wallet.h > index 9607415..b78045f 100644 > --- a/src/wallet.h > +++ b/src/wallet.h > @@ -163,6 +163,7 @@ public: > nOrderPosNext = 0; > nNextResend = 0; > nLastResend = 0; > + nTimeFirstKey = 0; > } > > If this fix is ok please pull from my GitHub fork; username on GitHub is > tm314159. > > Toshi > > > > ------------------------------------------------------------------------------ > Time is money. Stop wasting it! Get your web API in 5 minutes. > www.restlet.com/download > http://p.sf.net/sfu/restlet > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge•net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > >
Ok. Done.
Toshi
On Thu, May 29, 2014 at 5:06 PM, Mark Friedenbach <mark@monetize•io> wrote:
Quoted text (64 lines)
> Please make a pull request on github. It'll likely get merged quickly. > On May 29, 2014 5:04 PM, "Toshi Morita" <toshi@peernova•com> wrote: > >> I ran bitcoind under valgrind and found a place where it references an >> uninitialized variable in some cases: >> >> tm@tm-VirtualBox:~/bitcoind/bitcoin/src$ valgrind ./bitcoind >> ==2337== Memcheck, a memory error detector >> ==2337== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al. >> ==2337== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info >> ==2337== Command: ./bitcoind >> ==2337== >> ==2337== Conditional jump or move depends on uninitialised value(s) >> ==2337== at 0x319176: CWallet::LoadKeyMetadata(CPubKey const&, >> CKeyMetadata const&) (wallet.cpp:110) >> ==2337== by 0x33645A: ReadKeyValue(CWallet*, CDataStream&, >> CDataStream&, CWalletScanState&, std::string&, std::string&) >> (walletdb.cpp:509) >> ==2337== by 0x3374F0: CWalletDB::LoadWallet(CWallet*) >> (walletdb.cpp:623) >> ==2337== by 0x3218FD: CWallet::LoadWallet(bool&) (wallet.cpp:1485) >> ==2337== by 0x157F16: AppInit2(boost::thread_group&) (init.cpp:958) >> ==2337== by 0x140142: AppInit(int, char**) (bitcoind.cpp:143) >> ==2337== by 0x13649E: main (bitcoind.cpp:180) >> ==2337== >> >> The bug occurs here because nTimeFirstKey is not initialized when the >> wallet is instantiated: >> >> wallet.cpp:63 >> if (!nTimeFirstKey || nCreationTime < nTimeFirstKey) >> nTimeFirstKey = nCreationTime; >> >> >> I fixed it in my fork: >> >> diff --git a/src/wallet.h b/src/wallet.h >> index 9607415..b78045f 100644 >> --- a/src/wallet.h >> +++ b/src/wallet.h >> @@ -163,6 +163,7 @@ public: >> nOrderPosNext = 0; >> nNextResend = 0; >> nLastResend = 0; >> + nTimeFirstKey = 0; >> } >> >> If this fix is ok please pull from my GitHub fork; username on GitHub is >> tm314159. >> >> Toshi >> >> >> >> ------------------------------------------------------------------------------ >> Time is money. Stop wasting it! Get your web API in 5 minutes. >> www.restlet.com/download >> http://p.sf.net/sfu/restlet >> _______________________________________________ >> Bitcoin-development mailing list >> Bitcoin-development@lists•sourceforge•net >> https://lists.sourceforge.net/lists/listinfo/bitcoin-development >> >>