Slashdot Mirror
Unreal Engine Code Issues Fixed By Third-party Company
An anonymous reader writes: Unreal Engine is the famous game engine that was used to implement such games as Unreal Tournament, BioShock Infinite, Mass Effect and many more. On March 19, 2014 Unreal Engine 4 was made publicly available from a GitHub repository. It was a big event for the game development industry. One of the companies that took an interest in this was PVS-Studio, who created a static C/C++ code analyzer. They analyzed the Unreal Engine source code and reported to Epic Games's development team about the problems they found. Epic suggested a partnership with PVS-Studio to fix those bugs, and their challenge was accepted. Now, PVS-Studio shares their experience in fixing code issues and merging corrected code with new updates in a major project that shares its source code.
New article for a pull request?
The big question for me is do coding errors affect video rendering issues?
We tend to blame drivers for this, but recently there are opinions out there that suggest that the game code itself is responsible (as well).
Don't be apathetic. Procrastinate!
Why do I feel like this is an ad for the code analyzer?
Seven puppies were harmed during the making of this post.
An interesting read, but it really pushes their product hard. They did this to sell a product.
What issues are they "fixing" - is it academic issues - coding style issues, or actual functional and performance issues.
Where were the "issues" raised from - customers, third party developers or "in house"?
Modern app appers app app apps using app apps!
Apps!
It does make a good case for their product. I look at most of that stuff and think "I have done that" and think how much time has been wasted diagnosing such bugs.
However, what concerns me is the potential noise that is not in the article. I am pretty sure there are a few things that it reports that are actually OK and these things weren't included. Though I admit that I don't know for certain this is the case.
When Argumentum ad Hominem falls short, try Argumentum ad Matrem
call us to find out.. .Ok so that means it is at least 10k per year per seat.
Normally when someone approaches a company, they are treated like some kind of demonic enemy. This should be the model case to point to - let's work together. So what if its equivalent to "hire me".
Ah, but its a gaming software company, not some clothing / gadget marketplace multinational, and they did post all their code publicly - sigh, was a nice concept.
It's almost certain that a static code analyser will miss things, or mark false positives. There is usually a way to tell the analyser, "no, this is correct, please don't report it" - but IMO, usually it's a sign that the code should be restructured slightly to avoid confusion (because if the analyser is confused, there's a fair chance a human will be too.)
It's like any tool: there's an initial hurdle of getting things set up to use it effectively (source code repositories, for example), but the benefits in the long run usually far outweigh that initial effort.
For example, they changed some code to this:
float SGammaUIPanel::OnGetGamma() const
{
return GEngine ? GEngine->DisplayGamma : 2.2f;
}
Which should have triggered their own warning: PVS-Studio: V522 Dereferencing of the null pointer 'GEngine' might take place. It should, instead, have been changed to this:
float SGammaUIPanel::OnGetGamma() const
{
return (GEngine != null) ? GEngine->DisplayGamma : 2.2f;
}
This isn't exactly timely.. this happened march 2014.
I can see why one of the MISRA rules states that "limited dependence" should be placed on C operator precedence and expressions should be clarified (to the reader, not to the compiler) by using parentheses.
Company releases product as open source project. Other company submits code to open source project. Why is this even news?
I suspect his suspicion is false, ResolveRedirects might be a function that's relatively expensive to call and seldomly has to be called. Then it makes no sense for that function to always be called instead of the probably cheaper FindRef first.
The problem with articles like this one is that they tend to under-represent the benefits of static analysis. Products like PVS-Studio are designed to work with C++ and because they have to run in a big compile job, they get run in batch at the end of each day.
This is a problem because (a) C++ is very hard to statically analyse so performance is often poor and (b) the most critical time when you need/want static analysis feedback is when you're actually writing the code itself.
So let me insert a plug here for IntelliJ IDEA by JetBrains. Up until I used this (free, open source) program I didn't really appreciate static analysis. I mean, I appreciated it in a theoretical way, but my experience was that running it tends to generate thousands of spurious warnings that rarely reveal serious bugs. But that was because by the time the analysis got to run it was on code that had long since crashed in production, been debugged, unit tested, etc. So there was little meat left to harvest.
IntelliJ has a thing called the Inspector, which runs constantly in the background on spare CPU cores. It scans for hundreds of different kinds of bugs and when it spots one it highlights the bogus code in yellow, right in the editor. What impressed me most about this is that often the editor can highlight very complex bugs within seconds of you writing them, long before any time has been spent on unit testing or in a debugger. It can do this partly because the languages the inspector supports (things like Java, Kotlin, Scala etc) are much easier to parse and analyse than C++. You don't need to invoke a full blown compiler. Also the use of annotations to give the analysers more information is widespread.
But the best thing about IntelliJ is that when it does find a bug (and it frequently does), you can just press a hotkey and get a menu that lets you either suppress the warning ....... or automatically fix it, right there in the editor! So not only does IntelliJ find brainfarts like writing an if statement that will always yield true, but it can do it in real time and then it can often even fix it for you! This video I recorded a while ago shows a few seconds of this feature in action.
That is not workable in practice on a large scale, because it would involve admitting to a defect in a product. If that defect were to cause some actionable tort, the admission would be used to wring the company dry.
Do you know C at all?
Did you RTFA at all? A previous example on the same page threw V522 warnings from a similar construct.
The construct is similar, but not identical. The difference is significant, though.
Please explain under which condition(s) the code
will dereference a null pointer.