doc: Document how to analyze Bitcoin Core using Clang Static Analysis, clang-tidy and cppcheck #18920
Conversation
24899dd to
e8b04cd
Compare
e8b04cd to
bbfbdcf
Compare
| $ git clone https://github.com/bitcoin/bitcoin | ||
| $ cd bitcoin/ | ||
| $ ./autogen.sh | ||
| $ CC=clang CXX=clang++ ./configure --with-incompatible-bdb --disable-ccache |
There was a problem hiding this comment.
It may be helpful to mention why ccache needs to be disabled.
| # Analyze source code files ... | ||
| $ cppcheck/bin/cppcheck --language=c++ -D__cplusplus -DCLIENT_VERSION_BUILD \ | ||
| -DCLIENT_VERSION_IS_RELEASE -DCLIENT_VERSION_MAJOR -DCLIENT_VERSION_MINOR \ | ||
| -DCLIENT_VERSION_REVISION -DCOPYRIGHT_YEAR -DDEBUG |
There was a problem hiding this comment.
heh, nice example!
$ cppcheck/bin/cppcheck --language=c++ -D__cplusplus .../... src/net_processing.cpp
src/net_processing.cpp:936:17: error: Same iterator is used with different containers 'mapOrphanTransactions' and 'itPrev.second'. [iterators1]
itPrev->second.erase(it);
^
src/limitedmap.h:72:39: style: Same iterators expression are used for algorithm. [sameIteratorExpression]
iterator itTarget = map.erase(itIn, itIn);| $ cd bitcoin/ | ||
| $ ./autogen.sh | ||
| $ CC=clang CXX=clang++ ./configure --with-incompatible-bdb --disable-ccache | ||
| $ scan-build --use-cc=clang --use-c++=clang++ make |
There was a problem hiding this comment.
If this is OS-specific, I think it's necessary to state that. It seems to be, from looking at https://clang-analyzer.llvm.org/ and https://clang-analyzer.llvm.org/scan-build.html after seeing scan-build: 'ccc-analyzer' does not exist at '/usr/local/bin/ccc-analyzer' here and not finding a linux version.
| $ cd .. | ||
| $ bear/bear/bear -l $(pwd)/bear/libear/libear.so make | ||
| # Analyze source code files ... | ||
| $ clang-tidy src/test/crypto_tests.cpp |
|
I think I'd rather see this kind of documentation added to the devwiki (happy to help with that). That way it can be maintained / updated easier, without having to add even more documentation to this repo. There's also already other similar guides available, i.e I have one here: https://github.com/fanquake/core-review/blob/master/clang-tools.md. |
|
There is also bitcoin-core/docs, if the devwiki or fanquake/core-review are insufficient places to put this. In general I agree with @fanquake that documentation to compile with any imaginable compiler or checker or sanitizer or ... should probably be maintained outside of this repository. The project here is too big and moving too slow to keep every single piece of documentation up-to-date at all times. Also, we should be considerate of the precious review resource in this repository and use it on Bitcoin Core itself and not on integration of Bitcoin Core with meta developer tools. |
|
Feel free to take what you need from this PR and move to the appropriate place. Closing this PR :)
Personally I think we as a project have historically vastly underused the available tooling that is typically used in security critical projects to tame C++ (sanitizers, etc.). In other words I don't think we run the risk of using, integrating or documenting "too much" tooling (quite the opposite TBH!), but I see your point regarding moving the documentation to another repo :) |
|
If one of the tools turns out to be useful for the greater project (for example be an optional but recommended tool for developers), I think it can surely be added to the documentation here. |
Thanks for this info @practicalswift. Like @fanquake, I've been compiling info like this in a separate repository for my own reference and anyone else who may find it helpful. |
Thanks @fanquake for the reminder to spend more time looking at the excellent info in core-review. |
…ions as const 31b136e Don't declare de facto const reference variables as non-const (practicalswift) 1c65c07 Don't declare de facto const member functions as non-const (practicalswift) Pull request description: _Meta: This is the second and final part of the `const` refactoring series (part one: #20581). **I promise: no more refactoring PRs from me in a while! :)** I'll now go back to focusing on fuzzing/hardening!_ Changes in this PR: * Don't declare de facto const member functions as non-const * Don't declare de facto const reference variables as non-const Awards for finding candidates for the above changes go to: * `clang-tidy`'s [`readability-make-member-function-const`](https://clang.llvm.org/extra/clang-tidy/checks/readability-make-member-function-const.html) check ([list of `clang-tidy` checks](https://clang.llvm.org/extra/clang-tidy/checks/list.html)) * `cppcheck`'s `constVariable` check ([list of `cppcheck` checks](https://sourceforge.net/p/cppcheck/wiki/ListOfChecks/)) See #18920 for instructions on how to analyse Bitcoin Core using Clang Static Analysis, `clang-tidy` and `cppcheck`. ACKs for top commit: ajtowns: ACK 31b136e jonatack: ACK 31b136e theStack: ACK 31b136e ❄️ Tree-SHA512: f58f8f00744219426874379e9f3e9331132b9b48e954d24f3a85cbb858fdcc98009ed42ef7e7b4619ae8af9fc240a6d8bfc1c438db2e97b0ecd722a80dcfeffe
Document how to analyze Bitcoin Core using Clang Static Analysis,
clang-tidyandcppcheck.As requested by Sjors (#14676) and others :)