clang analyzer static analysis results #1877
Comments
|
Visual Studio agrees on many of those. I think it caught a couple not listed. Most of the notes from Visual Studio, though, were in upstream packages which we don't want to fix unless they can be shown to be actual problems. I ran across an online tool, Coverity, which purports to run exhaustive static analysis. Wesnoth is set up there but hasn't been run in a year. I sent a request to be added so I could check it out but whoever manages Wesnoth there has not acted on my request, yet. My opinion is any notes about Wesnoth code should be carefully considered and corrected. |
|
I didn't look through all of that, but it seemed like at least one report in a Boost header is likely related to a problem at the call site of that Boost function. The warnings in Lua should probably be ignored, as well. |
|
The Lua warnings were the ones I dug into. They're mainly due to struct member alignment and padding. I could fix them (using unspecified/undefined behaviors) or convert the members to another type. Lua, however, is specified to be solely ISO Standard C and any fix using unspecified or undefined behavior would be rejected upstream. Many static analyzers, remember, are simply suggestions. While a goal of zero messages is laudable, it is often unreasonable and even unattainable. Each suggestion, however, should be carefully considered (even if it is in an upstream package, like Boost). A note about memcpy, for example, probably appears because, over the decades, memcpy has been such a security headache. While it might not be an issue for Wesnoth, the comment should be carefully considered. We might not be able to replace the function, and might not be able to add call-site checks, we can, at least, audit each use and document the issue and why it's not a problem for Wesnoth. |
|
Here's the output with the Boost, Lua, tests, SDL_SavePNG, and compiler output stuff stripped: |
|
You're using the SVN / Development build for Clang 6.0? I reran using the current release and get NO messages at all. A glance through the messages @Vultraz pulled out and most do look like they deserve attention. But I'm not sure I want to rely upon a development version of the compiler. Since I run Arch, and keep fairly up-to-date, I tend to run into these sorts of warnings within a few days of the compiler releasing. I'll do some research and if I can do a side-by-side install I'll take a look at these warnings; otherwise I'll wait for 6.0 to come out. |
|
Yeah, I'm compiling llvm + tools from git sources myself. Anyway, I also had this situation where I would not get any messages at all, which happened when I only ran scan-build with make but not with cmake. worked though. 5.0 was just branched a few days ago, so the next version is major going to be 5.0, with 6.0 being released in a few months. |
|
I can get 6.0 from Arch AUR on a 6-hour refresh cycle from the SVN but don't want to commit to it unless I can side-by-side with the release version. I saw your commands and tried them but not in an out-of-tree. I'll try again. |
|
For the record, I was able to reproduce all the findings that Vultraz extracted with 4.0. |
|
Well, I wish I could. I did rm -fR on ~/build and ~/CMakeFiles after doing make/scons cleans. And then followed your process above using cut-and-paste. I don't get any messages at all. I just sync'd to master and hit a couple new minor errors; but still cannot get the static analysis to run. I'll keep looking into it. |
|
Did it say "scan-build: Using <compiler ..>" before giving any build output? |
|
I copied and pasted the command lines you used, in the OP. And yes it says it's using clang 4.0. I'm looking at the VC15 static analysis output right now and will get back to clang in a bit. |
|
can you see scan-build and its flags being used when running make in verbose mode? |
|
with VERBOSE=1 I do not see scan-build and all those flags. |
|
Odd, I don't understand what is wrong :/ |
|
|
It's working here with these commands. Apparnetly I was wrong about the the scan-build flags (security.insecureAPI.rand etc) showing up in the call by make but I can see that it does call c++-analyzer instead of clang++. |
|
s'ok now that I know where to look I'll probably figure it out. Most likely it's related to my system having both GCC and CLang installed. |
|
Hmm right. On most linuxes, CXX points to g++ but I have set my CXX to clang++ via zshrc. |
I left alone external libraries, as well as false positives and a bunch of miscellaneous cases where the existing code is fine (e.g. using floating point loop indices in scale_surface_sharp() is intentional). Closes wesnoth#1877.
scan-build.txt
Hi, I ran clang static analyzer aka scan-build on wesnoth, these are the results.
Unfortunately it failed to generated the reports as html for some reason, but I have the raw text.
Wesnoth @ 34d13ad
results:
The text was updated successfully, but these errors were encountered: