search

kvirc / KVIrc

The KVIrc IRC Client
http://www.kvirc.net/
GNU General Public License v2.0
240 stars 75 forks source link

clang's 'clang-analyzer' (scan-build) static analysis of kvirc (git) codebase. #2558

Closed abazaba855 closed 8 months ago

abazaba855 commented 11 months ago

Hi,

Not sure if anybody is actually interested in a static analysis report of the kvirc codebase by clang, but just for fun I decided to give it a go. The full results in the html report are included in the attachment.

To me, it looks like the analyzer either is totally wrong, or the codebase needs some love :)

I ran the analysis on Fedora 38, with clang 16.

If you want to reproduce this for yourself, it's actually quite easy to do: run the 'scan-build' command in front of both the 'cmake' and the 'make' commands, and otherwise compile as usual (a debug build is preferred as the tool is supposed to pick up clues from that). Just make sure that the package that contains the clang command 'scan-build' is installed ('clang-analyzer' on Fedora). And from there on it's something like this :

git clone https://github.com/kvirc/kvirc.git mkdir ~/tmp/scan-build-kvirc mkdir build && cd build scan-build -o ~/tmp/scan-build-kvirc cmake ../kvirc -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=debug scan-build -o ~/tmp/scan-build-kvirc make

ctrlaltca commented 11 months ago

I had a quick look at the first entries in the report. The first 3 "bugs" are in the 3rdparty rijndael code. The 4th "bug" (KviIrcSocket::flushSendQueue) assumes that the value of "KviOption_boolLimitOutgoingTraffic" is both true and false inside the same function. The 5th bug (UglyBase64::byteswap_buffer) assumes that we are passing wrong parameters to a function. KviModeEditor::KviModeEditor is even nicer, we have a Assuming 'pChan' is null and a lot of lines later the supposed bug Called C++ object pointer is null referring to pChan.

Maybe there's a few real bugs in the report, but the one i looked seems to be false positives mostly caused by KVIrc code being messy and clang making wild assumptions.

abazaba855 commented 11 months ago

Fair enough.

I did as you already noted just notice that a few 'issues' were in third party code; there is an option to '--exclude' code, and I now think I should have just excluded all things in '/usr/include' and/or '/kvirc/src/modules'.

However this turns out, thanks for looking at the report so far.

Thanks.

abazaba855 commented 11 months ago

Just to be slightly more up-to-date, I repeated the scan with Fedora 39 (as-of-writing still in beta) and clang 17, but got (nearly) identical results. Oh, well. scan-build-kvirc-fedora-39.tar.gz