Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-06 Thread Wladimir
On Wed, Jun 4, 2014 at 12:42 PM, Jannis Froese 
s9jaf...@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
--
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


Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-06 Thread Jeff Garzik
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/

--
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


Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Mike Hearn
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=L5ZbzVnpkXACpg=PA72lpg=PA72dq=Gotcha+%2328+Side+Effectssource=blots=Rn15TlPmjesig=tymHqta0aSANwaM2GaXC-1Di_tkhl=ensa=Xei=uVKNU47fCcvTsAT6goHIBAved=0CCAQ6AEwAA#v=onepageq=Gotcha%20%2328%20Side%20Effectsf=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.
--
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


Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Wladimir


  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
--
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


Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Jannis Froese
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 m...@plan99.net
 mailto:m...@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=L5ZbzVnpkXACpg=PA72lpg=PA72dq=Gotcha+%2328+Side+Effectssource=blots=Rn15TlPmjesig=tymHqta0aSANwaM2GaXC-1Di_tkhl=ensa=Xei=uVKNU47fCcvTsAT6goHIBAved=0CCAQ6AEwAA#v=onepageq=Gotcha%20%2328%20Side%20Effectsf=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

--
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 

Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Mike Hearn
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 s9jaf...@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 m...@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=L5ZbzVnpkXACpg=PA72lpg=PA72dq=Gotcha+%2328+Side+Effectssource=blots=Rn15TlPmjesig=tymHqta0aSANwaM2GaXC-1Di_tkhl=ensa=Xei=uVKNU47fCcvTsAT6goHIBAved=0CCAQ6AEwAA#v=onepageq=Gotcha%20%2328%20Side%20Effectsf=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 

Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Wladimir
On Wed, Jun 4, 2014 at 12:42 PM, Jannis Froese 
s9jaf...@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
--
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


Re: [Bitcoin-development] # error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Jeff Garzik
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 m...@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/

--
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


Re: [Bitcoin-development] error Bitcoin cannot be compiled without assertions. NOT

2014-06-04 Thread Ron

 Message: 2
 Date: Wed, 4 Jun 2014 08:15:08 -0400
 From: Jeff Garzik jgar...@bitpay.com
 Subject: Re: [Bitcoin-development] # error Bitcoin cannot
 be compiled
     without assertions. NOT
 To: Mike Hearn m...@plan99.net
 Cc: bitcoin-development@lists.sourceforge.net
     bitcoin-development@lists.sourceforge.net,
 Ron rd...@yahoo.com
 Message-ID:
    
 cajhla0ptthfvd-1br5s2k-4uw6v6qleyabf2ysrxoosjth9...@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 

--
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