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.

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

Coverity Scan Integration #64

Closed
neusdan opened this issue Mar 14, 2015 · 14 comments
Closed

Coverity Scan Integration #64

neusdan opened this issue Mar 14, 2015 · 14 comments
Labels
enhancement Enhancement or new feature

Comments

@neusdan
Copy link
Collaborator

neusdan commented Mar 14, 2015

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

https://scan.coverity.com/travis_ci

@randombit
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor

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
Copy link
Owner

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

@cdesjardins
Copy link
Contributor

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

Pretty simple to resolve.

@webmaster128
Copy link
Collaborator

Is this one done?

@webmaster128 webmaster128 added the enhancement Enhancement or new feature label Jun 16, 2015
@randombit
Copy link
Owner

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
Copy link
Collaborator

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

@randombit
Copy link
Owner

Yes, they ratelimits scanning to a few builds a day

@webmaster128
Copy link
Collaborator

Created the branch. Let the show begin!

@webmaster128
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

Done, thanks all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or new feature
Projects
None yet
Development

No branches or pull requests

4 participants