|
To follow up on my previous post, many of them were false alarms, but understandably so:
- object used after being freed: good find!
- possible
nullptr dereference: looks spurious - possible memory leaks: didn't realize that each relevant constructor assigns the object to an owner
- divide by zero: deliberate, to test the ability to handle
SIGFPE - expression will always evaluate to
false: because there is no override of a virtual framework function - expression will always evaluate to
true: because there is no override of a virtual framework function - unreachable code: each one a
break after a return  - use
empty() instead of size() == 0 to check for an empty string: alright operator new and operator delete should be implemented in pairs: didn't realize that internal memory management frameworks use a header that allows a superclass to return the memory to the correct pool or heap
modified 20-Mar-20 11:26am.
|
|
|
|
|
A worthwhile exercise, then. I'll have to give it a few tries myself.
[edit] I forgot to thank you for reporting on it. [/edit]
[edit] And I forgot again -- thank you for reporting on it. [/edit]
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
It's far better than the C++ code analysis plug-in provided with VS, which generates enough drool to float an aircraft carrier.
|
|
|
|
|
Do you have any experience with Coverity?
I love it when Coverity tells me that "if X is negative in function abc, and then the true branch is taken in function def, and then ... then the pointer will be null when you dereference it". I've seen it go to a depth of seven function call levels in my code, and that is probably nowhere close to a record breaker.
Coverity will, however, do "nothing" in 0.336 seconds. It is rather heavy, even more on RAM requirements than CPU. And for commercial use, it is far from free. Yet, lots of people consider it somewhat like a gold standard.
So I'd be very curious to see a shootout between Coverity and DeepCode! (but I wish DeepCode would get C# support before that!)
|
|
|
|
|
I have to look into it. If a free version that isn't a toy is available, I'll try it. I'd also found an outfit in Germany with a similar tool that looked to be very good. The reason I developed this[^] was because it was a challenge and I wasn't willing to pay for a commercial version. But I'd really like to see what they would find.
|
|
|
|
|
I don't think they provide any toy version, but you must have a license key.
For free, open source projects, you may (at least under a given set of circumstances) get a license for free, but require you to use a cloud service, so that you cannot run away with it. See Coverity Scan[^]
|
|
|
|
|
When I tested Coverity years ago, it was excellent.
|
|
|
|
|
The possible memory leaks (you gave a count of 109 in your first post) is an inidication of why I have come to favor automatic garbage collection. You do not mention dangling pointers and double freeing - maybe your code is so disciplined that you don't experience it; it can certainly create nasty bugs.
Many years ago, we had a basic General Object Dispenser, GOD, and whe didn't free objects explicitly but send them home to GOD. 
(and before you ask: I created GOD)
divide by zero: deliberate, to test the ability to handle SIGFPE In my student days, the Computing Center newsletter brought an article about the shocking number of divide by zero faults on the great Univac mainframe, it was something like a million a day. The next issure brought a note from the Mechanical Engineering dept: Some of their matrix operations most certainly did divisions by matrix elements with value zero (i.e. uninitialized value), but the following operations would never use those partial results. Identifying which values were not relevant and skipping the divide for those would be far more complex and time consuming than using a tight loop over all elements and simply accept the divide faults.
So the essential question is: Can you flag this code in a/the faults database as intentional, so that you won't see it reported the next time aound? (I hate cluttering up code with thousands of lint directives! The database approach is far better - but far more complex/expensive.)
expression will always evaluate to true Although the detail explanation is different: Embedded code in particular is overcrowded with "while (1)" (I tried to introduce "for ever", "ever" being a #define expanding to "(;;)", but the respones was certainly negative. Real Programmers write "while (1)"!)
I suspect that code checkers treat "while (1)" as a special case that is not reported. If not, being able to flag it as intentional is an absolute must!
|
|
|
|
|
The leaks were all false alarms, though some probably exist elsewhere. Double freeing is a problem, so a pointer must be cleared after being passed to delete. The introduction of unique_ptr and its kin has greatly reduced the risks. If only there was time to retrofit all that legacy code!
I use a form of garbage collection that can run intermittently rather than frequently, which is important when a system is heavily loaded[^].
|
|
|
|
|
Greg Utas wrote: each one a break after a return
I know this is an unpopular opinion; however, this is another good reason not to have multiple return statements. 
Social Media - A platform that makes it easier for the crazies to find each other.
Everyone is born right handed. Only the strongest overcome it.
Fight for left-handed rights and hand equality.
|
|
|
|
|
No harm done, and the number of reasons for having multiple return statements is far greater. You should see some of the garbage I've restructured because it was assiduously following that rule.
|
|
|
|
|
Greg Utas wrote: having multiple return statements is far greater.
I guess we have different experiences.
Social Media - A platform that makes it easier for the crazies to find each other.
Everyone is born right handed. Only the strongest overcome it.
Fight for left-handed rights and hand equality.
|
|
|
|
|
Thanks for the tests... I might give it a try too.
M.D.V. 
If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about?
Help me to understand what I'm saying, and I'll explain it better to you
Rating helpful answers is nice, but saying thanks can be even nicer.
|
|
|
|
|
Kent Sharkey wrote: Java, javaScript, Python, and now C and C++ code
I was interested until I saw it does not do C#. Then I realized that C# does not have flaws.
Social Media - A platform that makes it easier for the crazies to find each other.
Everyone is born right handed. Only the strongest overcome it.
Fight for left-handed rights and hand equality.
|
|
|
|
|
You owe me a new keyboard, because mine is now floating in sprayed coffee. But I'll overlook what happened to my nose.
|
|
|
|
|
TTFN - Kent
|
|
|
|
|
Years ago, I did a very extensive analysis of C++ static analyzers. The two best were PVS-Studio (for 'fast' analsysis) and Coverity (which found several really obscure bugs and relatively fewer false positives.)
How does this compare to those two?
Note: One big problem is that if not tuned right static analyzers generate a lot of false positives. With several products (esp. one) the false positives, even after tuning, were absurd. Worse, that one product was so wrong, sometimes were its suggestions followed, it would have made the code materially worse.
|
|
|
|
|
@Mark-Wallace @Member-7989122
In case you're interested, here[^] are the changes that I made as the result of DeepCode's analysis. I summarized what it found in a previous post[^].
I've now run PVS-Studio, made changes based on what it found, and have started regression testing. It generates lots of errors and warnings, many of which are false alarms (some understandably so) or violations of obsessive coding standards (e.g., MISRA). Some of the false alarms involve virtual functions, where it suggests making an argument const even though the function is an override and the argument has to be non-const for other overrides. But some of its "argument could be const" warnings look good but were missed by own static analysis tool[^], so I also have some escape analysis to do.
Here's a summary of most of the changes I made as the result of PVS-Studio's findings:
- changing
unique_ptr.release() to reset() to fix a memory leak - adding either a missing
break or [[fallthrough]] at the end of a case clause - removing an expression that always evaluates to
true or false (most were checks for < 0 on an unsigned type) - removing checks for
nullptr after invoking new (unreachable because an exception occurs if new fails) - changing occurrences of
(strlen(str) == 0) to (str[0] == NUL) for efficiency - moving an invariant call to
strlen out of a loop for efficiency - changing a type from signed to unsigned or vice versa (usually involving
size_t) - removing redundant checks or assignments
- changing the order of data members for more efficient memory usage
- making an argument
const
Mostly cosmetic, but there were some real bugs (the first two bullets).
It also told me that a multithreaded Windows application is supposed to use _beginthreadex instead of CreateThread. This was a revelation and something that I'll be changing later. My code seems to work using CreateThread, but maybe a tiger is waiting to pounce.
|
|
|
|
|
I'm quite impressed by the DeepCode results, especially if it's as quick as you say. It may not have flagged anything big enough to make the baby Jesus cry (presumably because you're not rubbish at it), but, for example, your For loop in ServiceSM is certainly improved a bit, so that makes it worth the effort.
Good stuff! Cheers for the update!
Greg Utas wrote: [PVS-Studio] also told me that a multithreaded Windows application is supposed to use _beginthreadex instead of CreateThread
I came across the _beginthread/_beginthreadex thing a while back, while trying to track a minor but annoying memory leak. Apparently, calling CreateThread directly from the API can cause tiddly chunks of memory (I think it was 72 bytes each time) to be leaked, because of some jiggery-pokery with the CRT signal function.
Like you say, it's one of those things you either find out about while not particularly looking for it, or when it bites you in the @rse.
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
Mark_Wallace wrote: Apparently, calling CreateThread directly from the API can cause tiddly chunks of memory (I think it was 72 bytes each time) to be leaked, because of some jiggery-pokery with the CRT signal function.
I also saw this while browsing through things found by a search but also recall someone claiming that the leak had been fixed.
|
|
|
|
|
Greg Utas wrote: but also recall someone claiming that the leak had been fixed.
Gawd, there was I, thinking that I was all super-duper cutting-edge, but I'm already behind the times!
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
The suspicious network activities revealed in the research by Positive Technologies are traffic hiding, VPN tunneling, connections to the Tor anonymous network, and network proxying. "A man who trusts everyone is a fool and a man who trusts no one is a fool."
|
|
|
|
|
Kent Sharkey wrote: "A man who trusts everyone is a fool and a man who trusts no one is a fool."
I prefer:
The first time you lie me, it will be your fault. The second one, it will be my fault.
M.D.V. 
If something has a solution... Why do we have to worry about?. If it has no solution... For what reason do we have to worry about?
Help me to understand what I'm saying, and I'll explain it better to you
Rating helpful answers is nice, but saying thanks can be even nicer.
|
|
|
|
|
Kent Sharkey wrote: traffic hiding, VPN tunneling, connections to the Tor anonymous network, and network proxying
They can't be very good at their jobs. All they seem to have found is people trying to protect their privacy from @rsehole companies that "analyse" what people are doing with the Internet.
Essentially, what they've discovered is that people don't like being snooped on by companies like them.
As for nasty boys on the Interwebs, just look as any server or router log for any ten minute period of any day, and you'll find "suspicious network activity" in all of them.
I wanna be a eunuchs developer! Pass me a bread knife!
|
|
|
|
|
Quote: The suspicious network activities revealed in the research by Positive Technologies are traffic hiding, VPN tunneling, connections to the Tor anonymous network, and network proxying.
Is it an example of Newspeak from 1984?
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
