Welcome to the Lounge

Robust Services Core | Software Techniques for Lemmings | Articles

12-May-20 14:14

Does anyone have experience running Coverity within Visual Studio? If you use their software to build your open-source code and submit it, they'll analyze it. I've heard good things about their tool and want to try it out. Right now this is where I'm at:

I go to their download page[^] and get the .zip file for Win64.
When extracting the .zip file, I get numerous “Error 0x80010135: Path too long” errors. I look this up online and learn that I have to use a different extraction tool. OK, I have 7-Zip.
7-Zip extracts it in 80 seconds, whereas Windows failed after beavering away for what, 10 minutes?! FPoS.
The next instruction is “Add the bin directory to your path”. Presumably “bin directory” refers to an extracted directory with executables. OK, I find it. But I have no idea what “your path” refers to. My project folder, the one that maps to its top-level folder on GitHub? Right now this is where I’m stuck.
Next, I’d have to figure out how to configure the tool for Microsoft’s C++ compiler using some command line stuff. The download page says to refer to coverity build tool/docs/en/help/cov-configure.txt for help. But there's no sign of a /docs folder in the extracted folder or its subfolders.
Finally, I’d have to do a build using command line stuff that I’ve never had to bother with, because Visual Studio takes care of it all when you click on “Build Solution”.

If anyone has gone through this and can help, I'd appreciate it. We can take this to the Product Lifecycle/Free Tools[^] forum if that would be more appropriate.

Mircea Neacsu12-May-20 14:47

Mircea Neacsu

12-May-20 14:47

Off top of my head I can help only with point 4: Add the path of the bin folder to your PATH environment variable (start > type "env" > Edit Environment variables > "environment variables" button > Select "Path" variable > append ";<your bin="" path="">")

Jon McKee12-May-20 16:05

Jon McKee

12-May-20 16:05

Never used that tool before but a reference for command line for #6: MSBuild Command-Line Reference - Visual Studio | Microsoft Docs

Just point it to the .vcxproj file for your C++ project. If you need to add or change a property you can do it in the file (there should already be PropertyGroups in the file as a template) or use the -property/p switch for something like configuration (e.g. -p:Configuration=Release).

Stuart Dootson13-May-20 1:07

13-May-20 1:07

Greg Utas wrote:
7-Zip extracts it in 80 seconds, whereas Windows failed after beavering away for what, 10 minutes?! FPoS.

Yup - it's even worse when transferring a big zip file over a network... I disable Windows zip handling, 'cause it sucks.

Greg Utas wrote:
The next instruction is “Add the bin directory to your path”. Presumably “bin directory” refers to an extracted directory with executables. OK, I find it. But I have no idea what “your path” refers to. My project folder, the one that maps to its top-level folder on GitHub? Right now this is where I’m stuck.

Open up a command prompt and use the following command, replacing the bit in angle brackets with the path of Coverity's bin folder on your machine:

SET PATH=<path to Coverity's bin folder>%PATH%

Greg Utas wrote:
Next, I’d have to figure out how to configure the tool for Microsoft’s C++ compiler using some command line stuff. The download page says to refer to coverity build tool/docs/en/help/cov-configure.txt for help. But there's no sign of a /docs folder in the extracted folder or its subfolders.

You don't need to do this step - it already knows how to cope with Visual C++.

Greg Utas wrote:
Finally, I’d have to do a build using command line stuff that I’ve never had to bother with, because Visual Studio takes care of it all when you click on “Build Solution”.

Open a Visual Studio command-line using the Start Menu (Start -> Visual Studio 2017 -> Developer Command Prompt for VS 2017, as in this screenshot).
Change directory to the one containing your solution file.
Use 'msbuild' to build your solution, like so:
```
MSBuild.exe <solution-file> -t:Build -p:Configuration=Debug -p:Platform=Win32 -m
```
Obviously, change 'Debug' to 'Release' or 'Win32' to 'x64' to build a different configuration if you need to.

HTH!

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

kalberts13-May-20 1:42

kalberts

13-May-20 1:42

Stuart Dootson wrote:
SET PATH=<path to Coverity's bin folder>%PATH%

That will update the path for the current command window only. As soon as you close that window, your path update is lost.

You can use SETX insttead of SET. SETX updates the path value in the registry, but remember that already running processes will not have their path changed. Updates apply to processes (including command windows) started/opened after you did the SETX.

I never use neither SET nor SETX! When I found Rapid Environment Editor[^], I never turned back. With REE, you can reorder path entries by dragging. You can drag any environment value between the user specific part and the system part. When adding entries e.g. to the path, you can select it by navigating, rather than typing a long path. REE is highly recommended!

(SETX can do things that REE cannot, e.g. update an environment variable on a remote system, or update from a registry value. I never had a need for such functionality.)

Clarification:
Window-born applications may be programmed to look up, in the registry, path and other environment variables on every use. They will get the updated value as soon as you have saved it (either by SETX or REE). Few Windows programs make use of path and environment variables; they rather have their own registry entries. Those using path/environment are mostly *nix-born software that knows nothing about the registry, and they are mostly command line programs. At startup, command.exe initializes its environment from the registry, and command line programs inherit this environment in the *nix style, not knowing where the values came from.

A final note: If you prefer to write in lowercase, you can write 'path' (and 'set'/'setx') rather than uppercasing them. Case sensitivity is a *nix artifact. When you use set/setx to create a new environment variable, you may want to use upper, lower, camel, Pascal or some other casing convention, that will be preserved - just like with NTFS file names. In use, you can case differently.

Greg Utas13-May-20 1:46

Robust Services Core | Software Techniques for Lemmings | Articles

13-May-20 1:46

It's actually starting a compile. Then I'll have to find the output, zip it, and submit it.

The only tweak was that it wants x86 or x64 for the platform instead of Win32 or Win64.

I owe you a 🍺. I was going to work on tax returns today, but now there are better things to do!

Stuart Dootson13-May-20 2:30

13-May-20 2:30

Greg Utas wrote:
The only tweak was that it wants x86 or x64 for the platform instead of Win32 or Win64.

That'll be something that's changed since VS2013 - I snuck the msbuild command-line out of a build script we use on a project built with VS2013.

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

Greg Utas13-May-20 2:12

Robust Services Core | Software Techniques for Lemmings | Articles

13-May-20 2:12

Hmm, that was a direct MSBuild, but it wants you to build via its own tool, which is configured for gcc with no sign of the instructions for how to change this...

Stuart Dootson13-May-20 2:38

13-May-20 2:38

Hmmm - that's odd - I've got cov-build to work for me, admittedly with a Visual Studio solution autogenerated by CMake.

Anyway - try running this command... I ran it, thinking it would allow me to tweak my MSVC configuration & maybe it changed my default config, or enabled msvc?

cov-configure --msvc

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

Greg Utas13-May-20 2:54

Robust Services Core | Software Techniques for Lemmings | Articles

13-May-20 2:54

It appears to be doing a compile through cov-build now, thanks.

Greg Utas13-May-20 3:09

Robust Services Core | Software Techniques for Lemmings | Articles

13-May-20 3:09

384 C/C++ compilation units (100%) are ready for analysis
The cov-build utility completed successfully.

Thanks for your help! Now I have to worry about what they'll find. Laugh | :laugh:

Stuart Dootson13-May-20 3:22

13-May-20 3:22

I've uploaded my (very minimal) code & have to confess I'm perplexed at some of the 'defects' Coverity raises:

Apparently, this line of code raises an exception (which isn't caught in main, as I don't use a global exception handler).

C++
```
std::wostringstream oss;
```
Trouble is... that statement doesn't raise an exception... I've looked at the source code for wostringstream (and the associated classes), and while an exception can be raised if the stream's state is set to 'bad' or 'fail', the code path that Coverity is complaining about is setting the stream state to 'good', using a static constexpr value (goodbit, as seen here). Maybe Coverity doesn't understand what constexpr means?
This one is fair enough (although slightly trivial?) - Coverity moans at me for using std::hex to change the default base output by a std::ostream and not restoring the old format. That's one of the reasons I prefer using libfmt (soon to be part of C++20 - yay!) rather than plain old iostreams...

I've used clang-tidy a lot, as well as the static analyzer built into clang, and they haven't raised false positives like this...

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

kalberts13-May-20 3:51

kalberts

13-May-20 3:51

I am familiar with the full, commercial Coverity with a local database; there may be differences from the cloud solution.

You have a lot of settings, e.g. the "agressiveness" level. If you set it to maximum, you certainly get a share of false warnings (and the documententation clearly warns about this). I rarely catch any non-false warnings by running at max agressivness.

Also, you can select an arbitrary set of checkers to be run, and many of them can be configured, checker by checker. When you triage the issue in the database, you may flag it as false warning, and you will not be see any more of that issue. It may take some effort tuning the options to your preferences and environment, to give you only those reports that you wish; it isn't perfectly configured out of the box.

Disclaimer: I have used coverity with plain C and with C#, not with C++.

With C# I have insisted for weeks in a row that some of the issues are false warnings, until I learn something new and realize that Coverity was right after all! (This is mostly resource leak issues, but also exception issues.)

Also, I usually get extremely helpful tracebacks explaining the root cause of the problem - I had one case of fourteen levels of traceback, where the true culprit was found. But that is from a complete analysis, not the desktop analysis that I believe gives a more lightweight check.

Stuart Dootson13-May-20 4:47

13-May-20 4:47

Thing is that the error I saw implied that Coverity wasn't correctly analysing the code in the C++ runtime - when a function is called with a parameter supplied with a statically constant argument value (defined using C++'s constexpr keyword, which is equivalent to a numeric literal), the flow analysis should be able to detect that the conditional code (which would have raised the exception) is not going to be executed, so the exception cannot be raised. That is not something that would be solved by suppressing some of the checks.

The tracebacks were very helpful - that part was absolutely what I'd want to see! In the end, though, the error came down to this function:

C++

    void __CLR_OR_THIS_CALL clear(iostate _State, bool _Reraise)
            {       // set state, possibly reraise exception
            _State &= _Statmask;
            _Mystate = _State;
            const auto _Filtered = _State & _Except;
            if (_Filtered)
                    {
                    if (_Reraise)
                            {
                            _RERAISE;
                            }

                    const char * _Msg;
                    if (_Filtered & ios_base::badbit)
                            {
                            _Msg = "ios_base::badbit set";
                            }
                    else if (_Filtered & ios_base::failbit)
                            {
                            _Msg = "ios_base::failbit set";
                            }
                    else
                            {
                            _Msg = "ios_base::eofbit set";
                            }
1. exception_thrown: An exception of type std::ios_base::failure is thrown.
                    _THROW(failure(_Msg));
                    }
            }

Trouble is that _State was passed as std::ios_base::goodbit, which the C++ standard defines as being zero (and it's defined as such in therelevant header). This mans that the local variable _Filtered must be zero (as zero bitwise-and anything equals zero), so the if (_Filtered) if block will never be executed.

I do have a history of stumbling over bugs in static analysis tools, mind - I found one in PRQA-C when using it at work to analyse an embedded software project. That bug caused PRQA to think that arrays were one element smaller than they'd been declared (i.e. PRQA would think that bool array[10]; had 9 elements, not 10...

Java, Basic, who cares - it's all a bunch of tree-hugging hippy cr*p

kalberts13-May-20 5:25

kalberts

13-May-20 5:25

I would be suprised if Coverity was flawless Smile | :)

From your description it seems like something I would raise as a support case (becauses we pay for support... Smile | :)

). But before that, I would check one thing: I have notices that the visibilty of a function can strongly affect which issues are raised. For a symbol (such as a function name) that is globally visible, the analysis must assume that any (syntactically legal) argument value may be supplied. For a private function, where all callers and calls are known, the analysis can ascertain that some code will never be called. (If that was the case here, Coverity would have flagged the entire if-statement as unreachable code - it is very good at that.)

In the general case, that exception may be thrown; the code is definitely reachable (from other calls, with other _State values) - you can't delete it. But for your specific call, with one specific _State value, the traceback should have been aborted and no issue raised.

The question is: Did the analyzer actually analyze the clear() source code? For the desktop analysis, it analyzes modified source lines only; for the rest, it uses the information from the previous complete analysis (somewhat similar to a referecded dotNet assembly: You have all the interface information, including an exception list, but none of the internal code flow). If it "knows" from earlier analysis results that clear() may raise an exception, it draw a fast conclusion from that.

I would be curious to know if you get the same result from a tabula rasa complete analysis, with all source code supplied, clear() inclusive. If your run was done as a desktop incremental analysis, I wouldn't be surprised if a full analysis did detect that the exception could not be reached in the specific call you make. The quick and dirty incremental analysis sure is quick compared to the full works, but it sometimes is a little dirty as well Smile | :)

.

Or maybe we should say: Make it as fast as possible, but no faster. The problem is that if you demand all the qualities of a full analysis but performed on your desktop, it will be no faster that a full analysis.

User 4041113-May-20 20:41

User 40411

13-May-20 20:41

Why not give PVS-Studio a try ? Built for Visual Studio and running as an extension, it gives very good insight into code. Also, open source projects can get a free licence.

modified 14-May-20 9:27am.

Greg Utas14-May-20 0:17

Robust Services Core | Software Techniques for Lemmings | Articles

14-May-20 0:17

I did try PVS-Studio, and here[^] is a post about what I changed as a result of its findings. It provides a one-week free trial, after which you have to pay. I didn't see anything for open-source projects, and an email inquiring about pricing has gone unanswered.

Trying out Deep Code led to these[^] changes.

And there's one that I wrote: A Static Analysis Tool for C++[^].

I'm now going through Coverity's findings. The majority of them flag destructors for calling things that can throw. I don't know whether I'll fix these, because I catch the ensuing SIGABRT and prevent the program from exiting. However, this has occasionally led to a SIGABRT loop, which is worse than exiting. So I also have code that checks the stack for a destructor or delete operator before throwing. But a non-throwing version of a few things could be an improvement.

Coverity is good at analyzing execution paths, so besides these destructor things it tells you stuff like

You already checked this for nullptr, so why are you checking it again?
You're checking this for nullptr now, so why didn't you bother when you first used it?
You're using an array index that could be out of bounds because it was returned by so-and-so function.

Naturally there are false positives, but I'd rather see them than not. A common one is that my parser will get the pointer from a unique_ptr, which then assigns ownership to another unique_ptr, after which I access the object through the original pointer. This gets flagged.

Message Closed

14-May-20 0:25

User 40411

14-May-20 0:25

Message Closed

modified 14-May-20 9:26am.

Greg Utas14-May-20 0:33