Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Static analysis #108

Open
JeremyRand opened this issue Oct 22, 2016 · 15 comments
Open

Static analysis #108

JeremyRand opened this issue Oct 22, 2016 · 15 comments
Assignees
@JeremyRand

Comments

@JeremyRand
Copy link
Member

There's some interest in running static analysis of the Namecoin Core codebase. @khaledJabr is currently tinkering around with this; I figure it's useful to make a GitHub issue for this work so that everyone's on the same page.
@JeremyRand
Copy link
Member Author

JeremyRand commented Oct 22, 2016 •
edited
Loading

For reference, the following seems to work reasonably well on Fedora 23 (inside Qubes 3.2):

# Assuming you're in the namecoin-core directory after cloning the Git repo
sudo dnf install csbuild
csbuild -g origin/bitcoin..origin/dev -c "./autogen.sh && ./configure && make"

The above displays all of the warnings from the compiler, Clang Analyzer, and cppcheck that appear in Namecoin Core but not Bitcoin Core.

It finds a decent number of warnings, 1 of which involves a null pointer dereference (although by eye I don't think it's particularly dangerous).

It took somewhere around an hour to run, although I didn't time it. Might be short enough to fit into a Travis CI build, not entirely sure.

EDIT: Added note about working directory

@JeremyRand
Copy link
Member Author

JeremyRand commented Oct 22, 2016 •
edited
Loading

Potentially relevant links:

EDIT: Added some links.

@josephbisch
Copy link
Member

Here is the source code repo for csbuild: https://git.fedorahosted.org/git/csmock.git.

@khaledJabr
Copy link

Hello,
I have run cppCheck on namecoin-core, and it has found some errors. I am not completely sure how relevant they are. The results are attached to this post.

cppCheck.txt

@domob1812
Copy link

Thanks for your analysis! From a quick glance, those errors seem to be unrelated to Namecoin (and are instead in code that is untouched from upstream Bitcoin) - but I may be mistaken. @JeremyRand, where is the nullptr dereference?

If an error is in code related to Namecoin, please point it out explicitly for me - then I'll fix it. Pull requests with fixes are also very welcome in general, of course. :)

@josephbisch
Copy link
Member

I ended up trying to install csbuild from the csmock repo and had trouble, so I just extracted the deb files for Ubuntu and installed that. From the below errors (when just running csbuild --help) it seems like Python can't find some of the code that is installed. I put the csmock/common/cflags.py using that directory structure in /usr/local/lib/python2.7/site-packages, which is where I thought it should go.

Traceback (most recent call last):
  File "/usr/local/bin/csbuild", line 28, in <module>
    from csmock.common.cflags import flags_by_warning_level
ImportError: No module named csmock.common.cflags

I get that csbuild is useful because it allows us to just see the errors that are introduced by Namecoin, but it would be nice to use something that is developed without just rpm-based systems in mind. Or just use the tools directly like what @khaledJabr is doing with cppCheck. I know I am on Arch and it isn't as if we need everybody to be able to run the tools, but it would be a nice extra consideration if possible.

@khaledJabr - There is also a cppcheck-htmlreport tool that can be used to turn the cppCheck.txt you got into an HTML report that can be opened in a web browser.

cppcheck-htmlreport --file=cppCheck.txt --report-dir=/some/directory/for/output/ --source-dir=/Users/kld/Documents/workspace/namecoin-core/

Then look in /some/directory/for/output/ for the HTML to open.

@JeremyRand
Copy link
Member Author

@domob1812 the null pointer dereference was found by Clang Analyzer. Interestingly csbuild reported that no issues unique to Namecoin were found by cppcheck; it's unclear to me whether I did something wrong there.

I'll refrain from posting the csbuild output here, since there was some criticism last year that I appeared to be doing too much with regards to projects from new contributors, and I'd rather that @khaledJabr contribute that rather than myself.

@josephbisch my take is that it doesn't necessarily need to run on a lot of platforms, as long as it runs on a platform that Travis CI can handle. (It looks to me like it's not resource-intensive enough to be a problem for Travis CI, but I could be wrong.) The links I gave for Travis CI OS support include instructions for using either Fedora or Ubuntu Precise, both of which csbuild supports.

I do agree with @josephbisch that it would be nice if csbuild were packaged for more distros. I spent several hours looking for similar tools, and didn't find any that looked anywhere near as good. Maybe I missed something.

@khaledJabr so my suggestion for next steps would be the following:

  1. Set up a VM with either Fedora 23/24 or Ubuntu Precise, and try running csbuild on the Namecoin Core codebase, to compare Namecoin Core against Bitcoin Core. Feel free to post the summary output.
  2. Modify the Travis CI config to run csbuild. My guess is that Ubuntu Precise is going to be easier than Fedora for Travis CI, and that it will result in less invasive changes to the Travis CI config, so Ubuntu Precise is probably preferable. Feel free to submit a pull request.

Of course, these suggestions might be invalidated if @domob1812 or @josephbisch have other ideas.

And as usual, feel free to ask questions (and let us know how things are going) on IRC. Usually IRC will get responses faster than GitHub.

@josephbisch
Copy link
Member

We could try modifying the existing packaging source to rebuild the csbuild deb packages for Ubuntu Trusty if that would make things any easier with Travis CI. I don't know if there is any technical reason for there just being Precise packages currently, or if it is just a matter of time or resources.

@JeremyRand
Copy link
Member Author

Ubuntu trusty packages for csbuild have been uploaded. So, that in
theory makes it quite a bit easier to set up csbuild to run on Travis.

@JeremyRand
Copy link
Member Author

JeremyRand commented Nov 4, 2016 via email

@khaledJabr
Copy link

fixed and created a pull request.

On Thu, Nov 3, 2016 at 8:45 PM, JeremyRand [email protected] wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Typo on this line:

https://github.com/namecoin/namecoin-core/blob/fddcd12297c7b3930f0d28a61
15c2a3bb599850f/.travis.yml#L48

sudp should be sudo
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJYG+f1AAoJELPy0WV4bWVwhRsP/2OhfXiYlqzfiyFQrESgysqA
S/yQDGMaN+p9sf8hg52JiwoQCH0eespBxCqlMFWhmyFjval60t4oIzcx9HI8sFrw
ahUUK+/mFYSefrv/U/LcZOW+wyPqoyLNLgJ0yHwFez5MUB45srOU1Nq/O9od4Jpz
B7yUUufUwYP3ULudruCFQZpfJQm0y9bacb5U9dLq3YYNvNMzkdFCaxEgNjNWJFKh
7JA/EpywvcaNk7m9slnuDM32kbnOSa0T8S1qFcGwuqOZbjG5UsumoXuJsUMl8M8b
qDVvA9Ankt7cIZWiL+DyZZXZt/joMe/5eLhWTgQQbn5atC2L93Wb/CwgkiSvvs95
RPvWKnNeZusQvwuDTpRnhxlOCHpXaK8k5W7xB6+7sV12a0Sij5YooJinkEzsWFLR
LPuActy8J7q4nJz89y9xt0j5OfvNxurd+m2Ya+siK8PRnb+LDTECQ8fM2QR8Rvc9
6TG5EEdc3FQu9ixv94N6XT/bS52GYJ6XeqWrW8htLw/a20cM06pr6VmNpdTZPuTh
YFiZAwGrzIZT/525xAszWaRl6BKdXBKlJJ2elkGqQG2KsXuEsdjOwecKtWpGvmNb
KYbODusKsMumEOi1SeB4+rHkliieplfsUSd3lxTS62BNjsxTCQwxyk5Bj/vf9obs
mGOReERNcU9dgcDWQryK
=O9+S
-----END PGP SIGNATURE-----

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#108 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGXb88roDNya7R056BLa6qMrhDlx2zqEks5q6o5BgaJpZM4KdxIk
.

@JeremyRand
Copy link
Member Author

JeremyRand commented Nov 4, 2016 via email

@khaledJabr
Copy link

fixed and made another pull request. Sorry about this, promise it'll be
better next time!

On Thu, Nov 3, 2016 at 9:03 PM, JeremyRand [email protected] wrote:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Another error:

https://travis-ci.org/namecoin/namecoin-core/jobs/173141190#L157

Looks like --qq should be -qq (only 1 hyphen).

(Sorry if this message gets duplicated; it appears that it failed to
send the first time.)
-----BEGIN PGP SIGNATURE-----

iQIcBAEBCgAGBQJYG+xNAAoJELPy0WV4bWVwjFsQANBW6rfsB+tPsq3cdDbIcKV1
1RStUDtQCgVSfnTCsTnaIShbXFtFOnS9yF7TvH9dOT0H3/BzZ5iUf9hp4l+c1kWf
kcij26E48duLEV4gBRUKGcJPoAwvEn+yA3OEss36R4+39sdP6RG8DcXyTqQZY5YD
PbOAy8HbuAUQrQuy59YWyfyPIeGRoLSjrr3s3A0xIzTdGMBYuadCkbjpjFlILyAg
/RPy3Y1IF8iJE6BuTAmwXxUML/GA7vYuqadHU13AU8Wl0JjwEpHjIvHPU98pZDG2
uscbGlaCAwW9QT/M2/PD7g9KEsY+BsbKrFlatf6kmzvEb5yDmYnO9EaP/UnM0cCg
ICvxSFxT6R+NX3h5UmasqFSiQNu2GWDGKeQxBXSJYy//OpCjh5jRjlIsOKAbMw3w
Y4xVc06FYmctPc7K5SRQGZnOv/dIyS88FX3Ev7kG4sR194INlh5D6arxzGrgTYpw
WgXyeGx/gFhi1ymxwcR67Juh+ipFxQUKtUkLdF7WOzk5bp676Ydr6dB+TtMEB6xK
n8LnLzZd6T81KjeYa6nshOJIbcVgKpI/tt0cYpmJuRlaoj3wVTF499QncrB4NaQD
EnskYRrfszFz7mxQDfPdqvl2MbUrlPtqCh/G17uW6WXljd+PsFaspZ+4HaF5ABkI
/1TLXcY3UHuWh4lYnqw5
=Xmvv
-----END PGP SIGNATURE-----

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#108 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AGXb810ILr-XfYc6cIehxmzdQF6SG-5rks5q6pJigaJpZM4KdxIk
.

@JeremyRand
Copy link
Member Author

JeremyRand commented May 7, 2017 via email

@JeremyRand JeremyRand mentioned this issue May 7, 2017
Closed
@JeremyRand
Copy link
Member Author

For adapting upstream scripts from Bitcoin Core to do a differential scan between Namecoin and Bitcoin, revgrep might be useful. It's in a memory-safe language (Go), which is an advantage over csbuild (which depends on the C++ tool csdiff).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JeremyRand JeremyRand

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@JeremyRand @josephbisch @domob1812 @khaledJabr