randombit / botan

Cryptography Toolkit
https://botan.randombit.net
BSD 2-Clause "Simplified" License
2.58k stars 568 forks source link

Coverity Scan Integration #64

Closed neusdan closed 9 years ago

neusdan commented 9 years ago

Would be nice to have Coverity Scan integration with the travis ci build as described here:

https://scan.coverity.com/travis_ci

randombit commented 9 years ago

I'll set it up, though I'm not sure if the scanner supports C++11 yet (it didn't last I tried it, but that was some time ago)

cdesjardins commented 9 years ago

It seems like it would be worth while to just enable all warnings (/W4 with msvs, or -Wall on gnu/clang) Coverity is great, but it typically produces so many issues that it is impossible to wade through them all. So you may be able to knock out some easy ones by fixing all the warnings first.

randombit commented 9 years ago

The maintainer mode already uses "-Wall -Wextra -Wstrict-aliasing -Wstrict-overflow=5 -Wcast-align -Wmissing-declarations -Wpointer-arith -Wcast-qual -Wzero-as-null-pointer-constant -Wold-style-cast" plus -Werror and a few -Wno-errors to turn off things I can't always fix. I'm not aware of any additional (useful) warnings to enable for gcc 4.9

The last coverity run I saw had a huge number of false positives but was worth wading through (check the 1.10.6 changelog for things it found) so I do think this is worth integrating if possible.

cdesjardins commented 9 years ago

I see, that is a new one to me, unfortunately it doesn't seem to enable /W4 on windows, but even with /W3 there are 20 or so warnings with the following configure line:

configure.py --disable-shared --disable-modules=selftest,tls --prefix=C:\Users\chrisd\software_devel\repo\install --enable-debug --cpu=i386 --via-amalgamation --maintainer-mode

I see things like:

C:\Users\chrisd\software_devel\repo\botan>nmake install

Microsoft (R) Program Maintenance Utility Version 12.00.21005.1
Copyright (C) Microsoft Corporation.  All rights reserved.

        cl /MDd  /EHs /GR /Od /Zi /DDEBUG /W3 /wd4275 /wd4267 /Ibuild\include /nologo /c botan_all.cpp /Fobuild\obj\lib\botan_all.obj
botan_all.cpp
botan_all.cpp(2006) : warning C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for deta
ils.
        C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\INCLUDE\stdio.h(356) : see declaration of 'sprintf'
botan_all.cpp(8098) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(10068) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
botan_all.cpp(16933) : warning C4800: 'botan_pk_op_encrypt_t *' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(16976) : warning C4800: 'botan_pk_op_decrypt_t *' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(17018) : warning C4800: 'botan_pk_op_sign_t *' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(17060) : warning C4800: 'botan_pk_op_verify_t *' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(17107) : warning C4800: 'botan_pk_op_ka_t *' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(17627) : warning C4244: 'initializing' : conversion from 'std::streamsize' to 'size_t', possible loss of data
botan_all.cpp(17648) : warning C4244: '=' : conversion from 'std::streamsize' to 'size_t', possible loss of data
botan_all.cpp(17656) : warning C4244: '=' : conversion from 'std::streamsize' to 'size_t', possible loss of data
botan_all.cpp(18350) : warning C4244: 'argument' : conversion from 'std::streamsize' to 'size_t', possible loss of data
botan_all.cpp(21031) : warning C4244: 'argument' : conversion from 'double' to 'const size_t', possible loss of data
botan_all.cpp(21362) : warning C4244: 'argument' : conversion from 'std::streamsize' to 'unsigned int', possible loss of data
botan_all.cpp(24145) : warning C4244: 'argument' : conversion from 'const unsigned short' to 'const unsigned char', possible loss of data
botan_all.cpp(25084) : warning C4244: 'argument' : conversion from 'const unsigned short' to 'const unsigned char', possible loss of data
botan_all.cpp(25159) : warning C4146: unary minus operator applied to unsigned type, result still unsigned
botan_all.cpp(25210) : warning C4244: 'return' : conversion from 'double' to 'size_t', possible loss of data
No filesystem access enabled
botan_all.cpp(44024) : warning C4101: 'e' : unreferenced local variable
botan_all.cpp(44287) : warning C4800: 'int' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(44904) : warning C4101: 'e' : unreferenced local variable
botan_all.cpp(45787) : warning C4800: 'Botan::u64bit' : forcing value to bool 'true' or 'false' (performance warning)
botan_all.cpp(45801) : warning C4800: 'Botan::u64bit' : forcing value to bool 'true' or 'false' (performance warning)

Without looking into it, I don't really see anything that is a real problem, but I do have an affinity for clean builds especially clean builds with all warnings enabled. But keep in mind that I haven't pulled from your repo in a week or two, so some of these things may already be gone or different...

randombit commented 9 years ago

I also prefer warning free builds, though the meaning of those "performance warnings" is beyond me.

cdesjardins commented 9 years ago

Performance warnings: https://msdn.microsoft.com/en-us/library/b6801kcy.aspx

Pretty simple to resolve.

webmaster128 commented 9 years ago

Is this one done?

randombit commented 9 years ago

Coverity can scan C++11 now and did a decent job with several real bugs found and not too many false positives. Only thing missing on this ticket is creating a coverity_scan branch in order to do recurring scans (waiting on github cutover for this).

webmaster128 commented 9 years ago

Cool. I don't understand why you need a separate branch to do the coverty scan. Is this to reduce load?

randombit commented 9 years ago

Yes, they ratelimits scanning to a few builds a day

webmaster128 commented 9 years ago

Created the branch. Let the show begin!

webmaster128 commented 9 years ago

Configuration must be broken because Botan is built twice now. Has someone experience with setting up coverity scan? See https://travis-ci.org/randombit/botan/branches

webmaster128 commented 9 years ago

I think I made it. Just waiting for Jack to grant me access to the scan results.

It turns out we need different travis configuration files on both branches because the build matrix must be less or equal 5 jobs for coverity scan quota.

webmaster128 commented 9 years ago

Done, thanks all.