* [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT @ 2014-06-03 19:07 Ron 2014-06-04 9:51 ` Mike Hearn 0 siblings, 1 reply; 14+ messages in thread From: Ron @ 2014-06-03 19:07 UTC (permalink / raw) To: bitcoin-development [-- Attachment #1: Type: text/plain, Size: 1800 bytes --] Hello What is the issue with the Bitcoin code for 0.9.x with regard to assertions that isn't in 0.8.6 or previous releases? on April 18th, I offered https://github.com/bc4-old-c-coder/bitcoin/commit/f0d221e56a12947b67b9c8f43cc5832b665052c8 this commit and code with all side effects removed from the assertions. Then on the 28th, https://github.com/bc4-old-c-coder/bitcoin/tree/0.8.6 this code with unit tests working. And if that isn't enough, I did a video series on building Bitcoind.exe and the static libraries (on MSVC++) all in NDEBUG (release) mode. See https://www.youtube.com/playlist?list=PLFnWb0ttBBMLyUuniLp3PJ5Mn4tVUlliZ Notice that the NDEBUG release mode is featured, and I even run it! Lastly what does that say about building Bitcoin-qt in release mode? Should one or not?? There is also a video on building an alternate coin-qt.exe in release mode (gcc version) and running it! See https://www.youtube.com/watch?v=C8GvHpjbAnM assert() should have no side effects, that is the problem. See http://books.google.com/books?id=L5ZbzVnpkXAC&pg=PA72&lpg=PA72&dq=Gotcha+%2328+Side+Effects&source=bl&ots=Rn15TlPmje&sig=tymHqta0aSANwaM2GaXC-1Di_tk&hl=en&sa=X&ei=uVKNU47fCcvTsAT6goHIBA&ved=0CCAQ6AEwAA#v=onepage&q=Gotcha%20%2328%20Side%20Effects&f=false a great book, BTW. Everyone who thinks they know what they are doing when they write C++ should read this book! They will realize that they don't know Jack Why weren't these and all the other examples of amateur, i.e., non-professional, software fixed way back in version 0.3.0 in 2010, before any more releases were done? And why were these and other sub-standard coding practices continued and expanded in later releases, right up until the present? Ron [-- Attachment #2: Type: text/html, Size: 10190 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-03 19:07 [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT Ron @ 2014-06-04 9:51 ` Mike Hearn 2014-06-04 10:12 ` Wladimir 2014-06-04 10:15 ` Gregory Maxwell 0 siblings, 2 replies; 14+ messages in thread From: Mike Hearn @ 2014-06-04 9:51 UTC (permalink / raw) To: Ron; +Cc: bitcoin-development [-- Attachment #1: Type: text/plain, Size: 1820 bytes --] Hi Ron, FYI your mail is being spamfoldered due to Yahoo's DMARC policy and the brokenness of the SF.net mailing list software. I would not expect to get replies reliably whilst this is the case. I think we should move away from SF.net for hosting mailing lists personally, because it's this list that's at fault not Yahoo, but until then you may wish to send to the list with a different email address. As to your question, assert() should have *no* side effects, that is the problem. > > See > > http://books.google.com/books?id=L5ZbzVnpkXAC&pg=PA72&lpg=PA72&dq=Gotcha+%2328+Side+Effects&source=bl&ots=Rn15TlPmje&sig=tymHqta0aSANwaM2GaXC-1Di_tk&hl=en&sa=X&ei=uVKNU47fCcvTsAT6goHIBA&ved=0CCAQ6AEwAA#v=onepage&q=Gotcha%20%2328%20Side%20Effects&f=false > > a great book, BTW. Everyone who thinks they know what they are doing when > they write C++ should read this book! They will realize that they don't > know Jack [image: Roll Eyes] > > Why weren't these and all the other examples of amateur, i.e., > non-professional, software fixed way back in version 0.3.0 in 2010, before > any more releases were done? And why were these and other sub-standard > coding practices continued and expanded in later releases, right up until > the present? > Back in 2010 most code was still being written by Satoshi so perhaps you should ask him. Regardless, it's very common for professional codebases to require assertions be enabled. For example the entire Google C++ codebase uses always-on assertions that have side effects liberally: it's convenient and safe, when you have the guarantee the code will always run, and the performance benefits of compiling out assertions are usually non-existent. So for this reason I think Bitcoin Core currently will fail to build if assertions are disabled, and that seems OK to me. [-- Attachment #2: Type: text/html, Size: 3468 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 9:51 ` Mike Hearn @ 2014-06-04 10:12 ` Wladimir 2014-06-04 10:15 ` Gregory Maxwell 1 sibling, 0 replies; 14+ messages in thread From: Wladimir @ 2014-06-04 10:12 UTC (permalink / raw) To: Mike Hearn; +Cc: bitcoin-development, Ron [-- Attachment #1: Type: text/plain, Size: 337 bytes --] > > > assert() should have *no* side effects, that is the problem. >> > I'm pretty sure that all the side effects of assertions have been removed before 0.9.0. However, the assertion checks are extremely important to the proper sanity of the client and network, so IMHO it's fair to still require building with them enabled. Wladimir [-- Attachment #2: Type: text/html, Size: 1323 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 9:51 ` Mike Hearn 2014-06-04 10:12 ` Wladimir @ 2014-06-04 10:15 ` Gregory Maxwell 2014-06-04 10:20 ` Mike Hearn 2014-06-04 10:42 ` Jannis Froese 1 sibling, 2 replies; 14+ messages in thread From: Gregory Maxwell @ 2014-06-04 10:15 UTC (permalink / raw) To: Mike Hearn; +Cc: bitcoin-development, Ron [-- Attachment #1: Type: text/plain, Size: 2979 bytes --] On Wed, Jun 4, 2014 at 2:51 AM, Mike Hearn <mike@plan99•net> wrote: > Hi Ron, > > FYI your mail is being spamfoldered due to Yahoo's DMARC policy and the > brokenness of the SF.net mailing list software. I would not expect to get > replies reliably whilst this is the case. I think we should move away from > SF.net for hosting mailing lists personally, because it's this list that's > at fault not Yahoo, but until then you may wish to send to the list with a > different email address. > > As to your question, > > assert() should have *no* side effects, that is the problem. >> >> See >> >> http://books.google.com/books?id=L5ZbzVnpkXAC&pg=PA72&lpg=PA72&dq=Gotcha+%2328+Side+Effects&source=bl&ots=Rn15TlPmje&sig=tymHqta0aSANwaM2GaXC-1Di_tk&hl=en&sa=X&ei=uVKNU47fCcvTsAT6goHIBA&ved=0CCAQ6AEwAA#v=onepage&q=Gotcha%20%2328%20Side%20Effects&f=false >> >> a great book, BTW. Everyone who thinks they know what they are doing >> when they write C++ should read this book! They will realize that they >> don't know Jack [image: Roll Eyes] >> >> Why weren't these and all the other examples of amateur, i.e., >> non-professional, software fixed way back in version 0.3.0 in 2010, before >> any more releases were done? And why were these and other sub-standard >> coding practices continued and expanded in later releases, right up until >> the present? >> > > Back in 2010 most code was still being written by Satoshi so perhaps you > should ask him. Regardless, it's very common for professional codebases to > require assertions be enabled. For example the entire Google C++ codebase > uses always-on assertions that have side effects liberally: it's convenient > and safe, when you have the guarantee the code will always run, and the > performance benefits of compiling out assertions are usually non-existent. > > So for this reason I think Bitcoin Core currently will fail to build if > assertions are disabled, and that seems OK to me. > As a matter of procedure we do not use assertions with side effects— the codebase did at one point, but have cleaned them up. In an abundance of caution we also made it refuse to compile without assertions enabled: A decision who's wisdom was clearly demonstrated when not long after, some additional side-effect having assert was contributed. In the real world errors happen here and there, and making robust software involves defense in depth. Considering the normal criticality of the software it should always be with the assertions. Without them is an untested configuration. It would probably be superior to use our own assertion macros (for one, they can give some better reporting…) that don't have the baggage ordinary assertions have, but as a the codebase is a production thing, making larger changes all at once to satisfy aesthetics would be unwise... simply refusing to compile in that untested, unsupported configuration is prudent, for the time being. [-- Attachment #2: Type: text/html, Size: 4811 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:15 ` Gregory Maxwell @ 2014-06-04 10:20 ` Mike Hearn 2014-06-04 10:31 ` Gregory Maxwell 2014-06-04 12:15 ` Jeff Garzik 2014-06-04 10:42 ` Jannis Froese 1 sibling, 2 replies; 14+ messages in thread From: Mike Hearn @ 2014-06-04 10:20 UTC (permalink / raw) To: Gregory Maxwell; +Cc: bitcoin-development, Ron [-- Attachment #1: Type: text/plain, Size: 830 bytes --] > > As a matter of procedure we do not use assertions with side effects— the > codebase did at one point, but have cleaned them up. In an abundance of > caution we also made it refuse to compile without assertions enabled: A > decision who's wisdom was clearly demonstrated when not long after, some > additional side-effect having assert was contributed. In the real world > errors happen here and there, and making robust software involves defense > in depth. > I think this class of errors could be removed entirely by just saying it's OK for assertions to have side effects and requiring them to be enabled, as is currently done. The glog library: http://google-glog.googlecode.com/svn/trunk/doc/glog.html provides CHECK macros that print stack traces when they fail. Using them would also be good. [-- Attachment #2: Type: text/html, Size: 1409 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:20 ` Mike Hearn @ 2014-06-04 10:31 ` Gregory Maxwell 2014-06-04 12:15 ` Jeff Garzik 1 sibling, 0 replies; 14+ messages in thread From: Gregory Maxwell @ 2014-06-04 10:31 UTC (permalink / raw) To: Mike Hearn; +Cc: bitcoin-development, Ron On Wed, Jun 4, 2014 at 3:20 AM, Mike Hearn <mike@plan99•net> wrote: >> As a matter of procedure we do not use assertions with side effects— the >> codebase did at one point, but have cleaned them up. In an abundance of >> caution we also made it refuse to compile without assertions enabled: A >> decision who's wisdom was clearly demonstrated when not long after, some >> additional side-effect having assert was contributed. In the real world >> errors happen here and there, and making robust software involves defense in >> depth. > > > I think this class of errors could be removed entirely by just saying it's > OK for assertions to have side effects and requiring them to be enabled, as > is currently done. > > The glog library: > > http://google-glog.googlecode.com/svn/trunk/doc/glog.html > > provides CHECK macros that print stack traces when they fail. Using them > would also be good. Yes... it takes only about 10 lines of code to have a nicer assert than the posix one, all my own software does... and with the noreturn attribute on the failure path it behaves the same for most static analysis tools as a regular assert does. I would have just dropped one in, but an IFDEF seemed more prudent at the time. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:20 ` Mike Hearn 2014-06-04 10:31 ` Gregory Maxwell @ 2014-06-04 12:15 ` Jeff Garzik 1 sibling, 0 replies; 14+ messages in thread From: Jeff Garzik @ 2014-06-04 12:15 UTC (permalink / raw) To: Mike Hearn; +Cc: bitcoin-development, Ron Yes, check macros like that can be useful. I like the kernel's policy, which parallels our direction: 1) Enable and use lightweight assertions for most users. 2) No assertions with side effects If you want to compile them out, that's fine, but they should always be present in production software. On Wed, Jun 4, 2014 at 6:20 AM, Mike Hearn <mike@plan99•net> wrote: >> As a matter of procedure we do not use assertions with side effects— the >> codebase did at one point, but have cleaned them up. In an abundance of >> caution we also made it refuse to compile without assertions enabled: A >> decision who's wisdom was clearly demonstrated when not long after, some >> additional side-effect having assert was contributed. In the real world >> errors happen here and there, and making robust software involves defense in >> depth. > > > I think this class of errors could be removed entirely by just saying it's > OK for assertions to have side effects and requiring them to be enabled, as > is currently done. > > The glog library: > > http://google-glog.googlecode.com/svn/trunk/doc/glog.html > > provides CHECK macros that print stack traces when they fail. Using them > would also be good. > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/NeoTech > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > -- Jeff Garzik Bitcoin core developer and open source evangelist BitPay, Inc. https://bitpay.com/ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:15 ` Gregory Maxwell 2014-06-04 10:20 ` Mike Hearn @ 2014-06-04 10:42 ` Jannis Froese 2014-06-04 10:51 ` Mike Hearn ` (2 more replies) 1 sibling, 3 replies; 14+ messages in thread From: Jannis Froese @ 2014-06-04 10:42 UTC (permalink / raw) To: bitcoin-development [-- Attachment #1: Type: text/plain, Size: 4649 bytes --] There are reasons to have assertions enabled by default in software like Bitcoin Core, where incorrect behaviour can be costly. But this comes at a prize: our assertions have to satisfy certain performance requirements. It's no longer possible to do expensive, redundant checks in performance critical code, which is one of the main advantages of asserts. Imho, asserts are not intended for small range checks etc, but are meant for checks that a hash hasn't changed, that a tree structure is still a tree, that data is still sorted, or that data structures are in sync. I think most concerns about the current use of asserts would be resolved if the currently used asserts would be changed to a nicer definition which is independent of NDEBUG, and a second class of debugging asserts would be introduced, which is exclusively for expensive, redundant checks and is disabled by NDEBUG. Am 2014-06-04 12:15, schrieb Gregory Maxwell: > On Wed, Jun 4, 2014 at 2:51 AM, Mike Hearn <mike@plan99•net > <mailto:mike@plan99•net>> wrote: > > Hi Ron, > > FYI your mail is being spamfoldered due to Yahoo's DMARC policy > and the brokenness of the SF.net mailing list software. I would > not expect to get replies reliably whilst this is the case. I > think we should move away from SF.net for hosting mailing lists > personally, because it's this list that's at fault not Yahoo, but > until then you may wish to send to the list with a different email > address. > > As to your question, > > assert() should have *no* side effects, that is the problem. > > See > http://books.google.com/books?id=L5ZbzVnpkXAC&pg=PA72&lpg=PA72&dq=Gotcha+%2328+Side+Effects&source=bl&ots=Rn15TlPmje&sig=tymHqta0aSANwaM2GaXC-1Di_tk&hl=en&sa=X&ei=uVKNU47fCcvTsAT6goHIBA&ved=0CCAQ6AEwAA#v=onepage&q=Gotcha%20%2328%20Side%20Effects&f=false > > a great book, BTW. Everyone who thinks they know what they > are doing when they write C++ should read this book! They > will realize that they don't know Jack Roll Eyes > > Why weren't these and all the other examples of amateur, i.e., > non-professional, software fixed way back in version 0.3.0 in > 2010, before any more releases were done? And why were these > and other sub-standard coding practices continued and expanded > in later releases, right up until the present? > > > Back in 2010 most code was still being written by Satoshi so > perhaps you should ask him. Regardless, it's very common for > professional codebases to require assertions be enabled. For > example the entire Google C++ codebase uses always-on assertions > that have side effects liberally: it's convenient and safe, when > you have the guarantee the code will always run, and the > performance benefits of compiling out assertions are usually > non-existent. > > So for this reason I think Bitcoin Core currently will fail to > build if assertions are disabled, and that seems OK to me. > > > As a matter of procedure we do not use assertions with side effects--- > the codebase did at one point, but have cleaned them up. In an > abundance of caution we also made it refuse to compile without > assertions enabled: A decision who's wisdom was clearly demonstrated > when not long after, some additional side-effect having assert was > contributed. In the real world errors happen here and there, and > making robust software involves defense in depth. > > Considering the normal criticality of the software it should always be > with the assertions. Without them is an untested configuration. It > would probably be superior to use our own assertion macros (for one, > they can give some better reporting...) that don't have the baggage > ordinary assertions have, but as a the codebase is a production thing, > making larger changes all at once to satisfy aesthetics would be > unwise... simply refusing to compile in that untested, unsupported > configuration is prudent, for the time being. > > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/NeoTech > > > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development [-- Attachment #2: Type: text/html, Size: 10034 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:42 ` Jannis Froese @ 2014-06-04 10:51 ` Mike Hearn 2014-06-04 12:13 ` Wladimir 2014-06-06 8:29 ` Wladimir 2 siblings, 0 replies; 14+ messages in thread From: Mike Hearn @ 2014-06-04 10:51 UTC (permalink / raw) To: Jannis Froese; +Cc: Bitcoin Dev [-- Attachment #1: Type: text/plain, Size: 5524 bytes --] Currently expensive checks are guarded with command line flags. It'd be nice if there could be one unified command line flag -expensivechecks that subsumes -checkmempool and so on. On Wed, Jun 4, 2014 at 6:42 PM, Jannis Froese <s9jafroe@stud•uni-saarland.de > wrote: > There are reasons to have assertions enabled by default in software like > Bitcoin Core, where incorrect behaviour can be costly. But this comes at a > prize: our assertions have to satisfy certain performance requirements. > It's no longer possible to do expensive, redundant checks in performance > critical code, which is one of the main advantages of asserts. Imho, > asserts are not intended for small range checks etc, but are meant for > checks that a hash hasn't changed, that a tree structure is still a tree, > that data is still sorted, or that data structures are in sync. > > I think most concerns about the current use of asserts would be resolved > if the currently used asserts would be changed to a nicer definition which > is independent of NDEBUG, and a second class of debugging asserts would be > introduced, which is exclusively for expensive, redundant checks and is > disabled by NDEBUG. > > > > Am 2014-06-04 12:15, schrieb Gregory Maxwell: > > On Wed, Jun 4, 2014 at 2:51 AM, Mike Hearn <mike@plan99•net> wrote: > >> Hi Ron, >> >> FYI your mail is being spamfoldered due to Yahoo's DMARC policy and the >> brokenness of the SF.net mailing list software. I would not expect to get >> replies reliably whilst this is the case. I think we should move away from >> SF.net for hosting mailing lists personally, because it's this list that's >> at fault not Yahoo, but until then you may wish to send to the list with a >> different email address. >> >> As to your question, >> >> assert() should have *no* side effects, that is the problem. >>> >>> See >>> >>> http://books.google.com/books?id=L5ZbzVnpkXAC&pg=PA72&lpg=PA72&dq=Gotcha+%2328+Side+Effects&source=bl&ots=Rn15TlPmje&sig=tymHqta0aSANwaM2GaXC-1Di_tk&hl=en&sa=X&ei=uVKNU47fCcvTsAT6goHIBA&ved=0CCAQ6AEwAA#v=onepage&q=Gotcha%20%2328%20Side%20Effects&f=false >>> >>> a great book, BTW. Everyone who thinks they know what they are doing >>> when they write C++ should read this book! They will realize that they >>> don't know Jack [image: Roll Eyes] >>> >>> Why weren't these and all the other examples of amateur, i.e., >>> non-professional, software fixed way back in version 0.3.0 in 2010, before >>> any more releases were done? And why were these and other sub-standard >>> coding practices continued and expanded in later releases, right up until >>> the present? >>> >> >> Back in 2010 most code was still being written by Satoshi so perhaps >> you should ask him. Regardless, it's very common for professional codebases >> to require assertions be enabled. For example the entire Google C++ >> codebase uses always-on assertions that have side effects liberally: it's >> convenient and safe, when you have the guarantee the code will always run, >> and the performance benefits of compiling out assertions are usually >> non-existent. >> >> So for this reason I think Bitcoin Core currently will fail to build if >> assertions are disabled, and that seems OK to me. >> > > As a matter of procedure we do not use assertions with side effects— the > codebase did at one point, but have cleaned them up. In an abundance of > caution we also made it refuse to compile without assertions enabled: A > decision who's wisdom was clearly demonstrated when not long after, some > additional side-effect having assert was contributed. In the real world > errors happen here and there, and making robust software involves defense > in depth. > > Considering the normal criticality of the software it should always be > with the assertions. Without them is an untested configuration. It would > probably be superior to use our own assertion macros (for one, they can > give some better reporting…) that don't have the baggage ordinary > assertions have, but as a the codebase is a production thing, making larger > changes all at once to satisfy aesthetics would be unwise... simply > refusing to compile in that untested, unsupported configuration is prudent, > for the time being. > > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today!http://p.sf.net/sfu/NeoTech > > > > _______________________________________________ > Bitcoin-development mailing listBitcoin-development@lists•sourceforge.nethttps://lists.sourceforge.net/lists/listinfo/bitcoin-development > > > > > ------------------------------------------------------------------------------ > Learn Graph Databases - Download FREE O'Reilly Book > "Graph Databases" is the definitive new guide to graph databases and their > applications. Written by three acclaimed leaders in the field, > this first edition is now available. Download your free book today! > http://p.sf.net/sfu/NeoTech > _______________________________________________ > Bitcoin-development mailing list > Bitcoin-development@lists•sourceforge.net > https://lists.sourceforge.net/lists/listinfo/bitcoin-development > > [-- Attachment #2: Type: text/html, Size: 10915 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:42 ` Jannis Froese 2014-06-04 10:51 ` Mike Hearn @ 2014-06-04 12:13 ` Wladimir 2014-06-06 8:29 ` Wladimir 2 siblings, 0 replies; 14+ messages in thread From: Wladimir @ 2014-06-04 12:13 UTC (permalink / raw) To: Jannis Froese; +Cc: Bitcoin Dev [-- Attachment #1: Type: text/plain, Size: 446 bytes --] On Wed, Jun 4, 2014 at 12:42 PM, Jannis Froese < s9jafroe@stud•uni-saarland.de> wrote: > I think most concerns about the current use of asserts would be resolved > if the currently used asserts would be changed to a nicer definition which > is independent of NDEBUG, and a second class of debugging asserts would be > introduced, which is exclusively for expensive, redundant checks and is > disabled by NDEBUG. > Sounds good to me. Wladimir [-- Attachment #2: Type: text/html, Size: 876 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-04 10:42 ` Jannis Froese 2014-06-04 10:51 ` Mike Hearn 2014-06-04 12:13 ` Wladimir @ 2014-06-06 8:29 ` Wladimir 2014-06-06 8:40 ` Pieter Wuille 2 siblings, 1 reply; 14+ messages in thread From: Wladimir @ 2014-06-06 8:29 UTC (permalink / raw) To: Jannis Froese; +Cc: Bitcoin Dev [-- Attachment #1: Type: text/plain, Size: 1108 bytes --] On Wed, Jun 4, 2014 at 12:42 PM, Jannis Froese < s9jafroe@stud•uni-saarland.de> wrote: I think most concerns about the current use of asserts would be resolved if > the currently used asserts would be changed to a nicer definition which is > independent of NDEBUG, and a second class of debugging asserts would be > introduced, which is exclusively for expensive, redundant checks and is > disabled by NDEBUG. > Also, most assertion errors that happen to people running Bitcoin Core are not caused by software bugs but database corruption errors (usually due to unclean shutdown). For example in case we detect missing/truncated block files or UTXO db consistency we should, instead of raising an assertion error, propose a -reindex - see also https://github.com/bitcoin/bitcoin/issues/2202 . So instead of using assertions we need a fatal error function for those problems which are probably recoverable in a certain specific way. In principle starting a reindex wouldn't even need to take down the entire process (though that's easier for implementation due to cleanup and assumptions made). Wladimir [-- Attachment #2: Type: text/html, Size: 1731 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-06 8:29 ` Wladimir @ 2014-06-06 8:40 ` Pieter Wuille 2014-06-07 0:57 ` Jeff Garzik 0 siblings, 1 reply; 14+ messages in thread From: Pieter Wuille @ 2014-06-06 8:40 UTC (permalink / raw) To: Wladimir; +Cc: Bitcoin Dev On Fri, Jun 6, 2014 at 10:29 AM, Wladimir <laanwj@gmail•com> wrote: > On Wed, Jun 4, 2014 at 12:42 PM, Jannis Froese > <s9jafroe@stud•uni-saarland.de> wrote: > >> I think most concerns about the current use of asserts would be resolved >> if the currently used asserts would be changed to a nicer definition which >> is independent of NDEBUG, and a second class of debugging asserts would be >> introduced, which is exclusively for expensive, redundant checks and is >> disabled by NDEBUG. There are a few examples of things that would classify as expensive/redundant checks: * addrman consistency checks (only enabled with -DDEBUG_ADDRMAN). * mempool consistency checks (only enabled with -checkmempool). * deadlock detection (only enabled with -DDEBUG_LOCKORDER). I'm not sure all of these make sense to put under a single runtime flag. For example, addrman consistency is unlikely to be affected unless you're working on addrman code, and is pretty expensive. Still, I do like the idea of optional consistency checks, that help guarantee the software always has a consistency state. -- Pieter ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT 2014-06-06 8:40 ` Pieter Wuille @ 2014-06-07 0:57 ` Jeff Garzik 0 siblings, 0 replies; 14+ messages in thread From: Jeff Garzik @ 2014-06-07 0:57 UTC (permalink / raw) To: Pieter Wuille; +Cc: Bitcoin Dev Speaking very generally, the Linux kernel wisdom on this tends to be, * Compile in as many cheap, compiler-predictable asserts as possible into the production runtime. * Debug builds are of limited value. Users do not recompile software, just to provide better bug reports/diagnostics. * Make it as easy as possible for users to send reports that are useful to programmers. * Expensive diagnostics are fine. Compile in, but disable by default at runtime (and make sure these features, when turned off, do not slow down the system). * Make sure the assert/dump provides a high level of diagnostics. Stack trace of each thread + multi-threaded core dump are a good start. -- Jeff Garzik Bitcoin core developer and open source evangelist BitPay, Inc. https://bitpay.com/ ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <mailman.192896.1401886427.2163.bitcoin-development@lists.sourceforge.net>]
* Re: [Bitcoin-development] error "Bitcoin cannot be compiled without assertions." <<<<NOT [not found] <mailman.192896.1401886427.2163.bitcoin-development@lists.sourceforge.net> @ 2014-06-04 19:13 ` Ron 0 siblings, 0 replies; 14+ messages in thread From: Ron @ 2014-06-04 19:13 UTC (permalink / raw) To: bitcoin-development ________________________________________________________ Message: 2 Date: Wed, 4 Jun 2014 08:15:08 -0400 From: Jeff Garzik <jgarzik@bitpay•com> Subject: Re: [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT To: Mike Hearn <mike@plan99•net> Cc: "bitcoin-development@lists•sourceforge.net" <bitcoin-development@lists•sourceforge.net>, Ron <rdwnj@yahoo•com> Message-ID: <CAJHLa0PTTHfvd-1BR5s2k-4UW6V6qLEyAbF2YSRxOOSjtH9+_Q@mail•gmail.com> Content-Type: text/plain; charset=UTF-8 Yes, check macros like that can be useful. I like the kernel's policy, which parallels our direction: 1) Enable and use lightweight assertions for most users. 2) No assertions with side effects If you want to compile them out, that's fine, but they should always be present in production software. _____________________________________________________ I don't think many of you actually read what I said, and you went off on your own tangents. I said: this commit and code with all side effects removed from the assertions. in Vol 37, Issue 4 I intentionally left the gcc code alone. Only the MSC code is assert "fixed". I would have hoped that someone would have noticed and incorporated these changes into the gcc code. Simply by removing the #ifdef _MSC_VER ... #else ... #endif etc. etc. Did I say I compiled them out? No! Did anyone bother to look at my code or what I said? Here is an example from main.cpp CTransaction::UpdateCoins(... - // add outputs + // add outputs sure looks like an assert with side effects here!? +#ifdef _MSC_VER + bool + fTest = inputs.SetCoins(txhash, CCoins(*this, nHeight)); + #ifdef _DEBUG + assert(fTest); + #else + if( !fTest ) + releaseModeAssertionfailure( __FILE__, __LINE__, __PRETTY_FUNCTION__ ); + #endif +#else assert(inputs.SetCoins(txhash, CCoins(*this, nHeight))); +#endif Note that I do the test, and if debugging, I let it abort() the program if it fails. Note that in release mode it calls a new function on failure, that I leave you to discover what it does. I see that this assert has been "fixed" in 0.9.x, but I think my "fix" is better, since it allows release mode code to run better, if not identically. I changed every assert() in the bitcoind 086 sources to behave this way. Since C++ is perniciously baroque, it is not clear if a side effect can or has occurred in the most innocuous of C++ statements. Is the example above side-effect free? Here is a piece of what I made my decision on: http://www.gnu.org/software/libc/manual/html_node/Consistency-Checking.html and the link to the book previously given. Also, no one answered the question about bitcoin-qt, to whit, can or should it be compiled in RELEASE mode because of this? Should it have always been compiled in DEBUG mode in the past too? Ron ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-06-07 0:57 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-03 19:07 [Bitcoin-development] # error "Bitcoin cannot be compiled without assertions." <<<<NOT Ron
2014-06-04 9:51 ` Mike Hearn
2014-06-04 10:12 ` Wladimir
2014-06-04 10:15 ` Gregory Maxwell
2014-06-04 10:20 ` Mike Hearn
2014-06-04 10:31 ` Gregory Maxwell
2014-06-04 12:15 ` Jeff Garzik
2014-06-04 10:42 ` Jannis Froese
2014-06-04 10:51 ` Mike Hearn
2014-06-04 12:13 ` Wladimir
2014-06-06 8:29 ` Wladimir
2014-06-06 8:40 ` Pieter Wuille
2014-06-07 0:57 ` Jeff Garzik
[not found] <mailman.192896.1401886427.2163.bitcoin-development@lists.sourceforge.net>
2014-06-04 19:13 ` [Bitcoin-development] " Ron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox