Static Analysis in the Video Game Industry: Top 10 Software Errors

If you are developing software in the video game industry and are wondering what else can be done to improve the quality of the product \ simplify the development process, while not using static analysis - it's time to start. Do you doubt? Well, I will try to convince you of this. If it is just interesting for you to look at the mistakes made by the developers in the video game industry, then, again, you got to the address - the most interesting ones were selected for you.

Why you should use static analysis

Despite the fact that the development of video games involves a large number of stages, programming remains one of the main ones. Even if you do not write thousands of lines of code, one way or another you have to use various tools, the quality of which depends including the convenience of work and the final result. If you are a developer of such tools (for example, game engines), then everything should be clear to you.

Why is static analysis useful in software development in general?

There are several main reasons:
')

• The cost and complexity of correcting an error increases with time. One of the main advantages of static analysis is the detection of errors in the early stages of development (you can find an error at the time of writing the code). Therefore, using a static analyzer, you can simplify the life of yourself and your colleagues by detecting and correcting many errors before they become the cause of a headache.
• Using static analysis, you can identify many different error patterns (copy-paste, typos, misuse of functions, etc.).
• As a rule, static analysis is good at identifying problems that are poorly identified using dynamic analysis. However, the converse is also true.
• Negative moments of use (for example, false positives) are usually 'smoothed out' by the teams that develop serious static analyzers due to various warning suppression functions (single, by pattern, etc.), disabling irrelevant diagnostics, excluding files and directories from the analysis . This allows you to significantly reduce the percentage of 'noise' by properly configuring the analyzer. My colleague, Andrey Karpov, demonstrated in an article about testing EFL Core Libraries that if the analyzer is configured, the number of false positives will be only 10-15%.

But anyway, this is all theory, and you are probably interested in more practical examples. Well, they are presented below.

Static analysis in the Unreal Engine

If you have read up to this point, it’s not worth telling you what the Unreal Engine is or what kind of company it is - Epic Games - and if these guys do not have authority, write me who then uses it.

The PVS-Studio team collaborated several times with the Epic Games team, helping them implement and use static analysis in the Unreal Engine project and correct errors / false positives detected by the analyzer. I am sure it was an interesting and useful experience for both parties.

One of the consequences was the addition of a special flag to the Unreal Engine, which made it possible to conveniently integrate static analysis into the Unreal Engine build system of projects.

The key idea is simple - the guys care about the quality of their code and use different approaches for this, one of which was the introduction of static analysis.

John Carmack on static analysis

John Carmack, one of the most well-known developers in the video game industry, at one time recognized the active use of static analysis techniques as one of his main achievements as a programmer: " The most important thing I have been doing in recent years is to aggressively pursue static code analysis. "The next time someone you know says that static analysis is a tool for newbies, show him this quote. Carmack described his acquaintance with static analysis in a relevant article , which I strongly recommend to read: both for motivation and for general development.

Errors found using static analysis in games and game engines

Perhaps one of the best ways to show the benefits of static analysis is to demonstrate its capabilities in practice. This is done, for example, by the PVS-Studio team, checking open source projects.

Everybody wins:

• Project developers are notified of errors and can correct them. Ideally, of course, the approach should be different than correcting errors in the log \ article - you should check the project yourself with the analyzer and examine the issued warnings. This is important, if only for the reason that the authors of the articles may lose sight of something or inadvertently focus on errors that are not critical for the project.
• Analyzer developers can improve the tool according to the results of the analysis, and also demonstrate the analyzer's ability to detect errors.
• Readers see which error patterns are allowed, gain experience, get acquainted with the method of static analysis.

In general - why not proof of effectiveness and benefits?

Commands that already use static analysis

While someone is pondering whether to embed static analysis in their development process, some teams are already using it and gaining it! For example, Rocksteady, Epic Games, ZeniMax Media, Oculus, Codemasters, Wargaming and others ( source ).

Top 10 bugs from software code in the video game industry

Immediately it is worth making a reservation that this is not some kind of mega-global 'top', but those errors found by the PVS-Studio analyzer in games and game engines that seemed to me the most interesting.

As usual, for more interest, I suggest you first try to find the error in the above code snippet, and only then read the analyzer warning and problem description.

Tenth place

Source: Looking for anomalies in the X-Ray Engine

On the tenth place is the error from the X-Ray Engine game engine used in STALKER games. Those who played the games from this series will surely remember many funny (and not so) bugs that occurred during the game process. This is especially true of STALKER: Clear Sky, in which it was impossible to play without patches at all (I still remember the bug that 'killed' all the save). As the analysis of the game engine showed, there are really bugs in the code, and there are quite a lot of them. Below is one of them.

BOOL CActor::net_Spawn(CSE_Abstract* DC) { .... m_States.empty(); .... } 

PVS-Studio warning : V530 The return value of the function is 'empty' is required to be utilized.

The problem is simple - do not use the returned logical value of the empty method, indicating whether the container is empty. Judging by the fact that in the above expression, there is nothing but the method call, we can assume that the developer wanted to empty the container, but made a mistake by calling the empty method instead of clear .

Some may say that this is too simple a mistake for the Top 10. But this is her charm! In spite of the fact that just like this, from the outside, the error seems quite simple and obvious, such 'simple' errors do not cease to appear (and be discovered) in various projects.

Ninth place

Source: CryEngine V Welcome Check

We continue to reveal the theme of errors in game engines. In ninth place - the code from CryEngine V. In games on this engine, I did not notice as many bugs as in games on the X-Ray Engine, but the “showdown” showed that the engine contains a lot of suspicious code fragments.

 void CCryDXGLDeviceContext:: OMGetBlendState(...., FLOAT BlendFactor[4], ....) { CCryDXGLBlendState::ToInterface(ppBlendState, m_spBlendState); if ((*ppBlendState) != NULL) (*ppBlendState)->AddRef(); BlendFactor[0] = m_auBlendFactor[0]; BlendFactor[1] = m_auBlendFactor[1]; BlendFactor[2] = m_auBlendFactor[2]; BlendFactor[2] = m_auBlendFactor[3]; *pSampleMask = m_uSampleMask; } 

PVS-Studio warning : V519 The 'BlendFactor [2]' variable is assigned values ​​twice successively. Perhaps this is a mistake.

As repeatedly mentioned in our articles, no one is immune from typos. In practice, it has also been repeatedly confirmed that static analysis copes with the detection of copy-paste and typos. They copied the values ​​from the m_auBlendFactor array to BlendFactor , but were accidentally sealed and wrote BlendFactor [2] twice. Because of this confusion, the value from m_auBlendFactor [3 ] will be recorded in BlendFactor [2] , and the value will not change in BlendFactor [3] .

Eighth place

Source: Unicorn in Space: Checking the source code of 'Space Engineers'

Let's change the course a bit and move on from C ++ to C #. Before us is the code from the Space Engineers project, the sandbox games about building and maintaining space structures. I myself did not play the game, but as one of the commentators of the article said: " I am not very surprised by the results :) ". Well, interesting mistakes were really found, below - two of them.

 public void Init(string cueName) { .... if (m_arcade.Hash == MyStringHash.NullOrEmpty && m_realistic.Hash == MyStringHash.NullOrEmpty) MySandboxGame.Log.WriteLine(string.Format( "Could not find any sound for '{0}'", cueName)); else { if (m_arcade.IsNull) string.Format( "Could not find arcade sound for '{0}'", cueName); if (m_realistic.IsNull) string.Format( "Could not find realistic sound for '{0}'", cueName); } } 

PVS-Studio warnings :

• V3010
• V3010

As you can see, errors of ignoring return values ​​of methods are found not only in C ++ code, but also in C #. The String.Format method compiles the resulting string based on the format string and the objects to be formatted, and then returns the result as the return value of the method. In the code above in the else- branch there are two calls to string.Format , but their return values ​​are not used. Most likely, it was necessary to log these messages in the same way as it was done in the then- branch of the if statement , using the MySandboxGame.Log.WriteLine method.

Seventh place

Source: Check Quake III Arena GPL project

Did I mention that static analysis well detects typos? The example below is another confirmation of this.

 void Terrain_AddMovePoint(....) { .... x = ( v[ 0 ] - p->origin[ 0 ] ) / p->scale_x; y = ( v[ 1 ] - p->origin[ 1 ] ) / p->scale_x; .... } 

PVS-Studio warning : V537 Consider reviewing the correctness of the 'scale_x' item's usage.

The x and y variables are written, but in both assignments, the value expression expression p-> scale_x is used in the value calculation expression. It looks very suspicious and it seems that in the second case it is necessary to replace the expression p-> scale_x with p-> scale_y .

Sixth place

Source: Checking the source C # code Unity

Unity Technologies recently opened the source code of its own game engine - Unity. We could not pass by such an event and in the analysis found a lot of interesting places. Below is one of them.

 public override bool IsValid() { .... return base.IsValid() && (pageSize >= 1 || pageSize <= 1000) && totalFilters <= 10; } 

PVS-Studio warning : V3063 A part of conditional expression is always true if it is evaluated: pageSize <= 1000.

We have an invalid range check for pageSize . Most likely, it was planned to check that the pageSize value lies within [1; 1000]. However, an annoying typo was made: the programmer accidentally wrote '||' instead of the '&&' operator. It turns out that the subexpression actually does not check anything.

Fifth place

Source: We analyze errors in open components of Unity3D

In the next place there is an interesting error from the components of Unity3D. The source article was written more than a year before the source code of the Unity engine was discovered. However, even then it was possible to find something interesting.

 public static CrawledMemorySnapshot Unpack(....) { .... var result = new CrawledMemorySnapshot { .... staticFields = packedSnapshot.typeDescriptions .Where(t => t.staticFieldBytes != null & t.staticFieldBytes.Length > 0) .Select(t => UnpackStaticFields(t)) .ToArray() .... }; .... } 

PVS-Studio warning : V3080 Possible null dereference. Consider inspecting 't.staticFieldBytes'.

Notice the lambda expression passed as an argument to the Where method. Judging by the code, the typeDescriptions collection can contain elements whose staticFieldBytes member can be null . That is why, before accessing the Length property, we check that staticFieldBytes! = Null . That's just when the operators were confused, and instead of '&&', '&' is used. This means that regardless of the result of evaluating the left expression ( true \ false ), the right expression will also be calculated, which is why, if staticFieldBytes == null , when accessing the Length property, an exception of type NullReferenceException will be thrown . Using '&&' would not have caused this situation, since, if staticFieldBytes == null , the right side of the expression would not be evaluated.

Despite the fact that Unity is the only engine that has fallen into this “Top-10” twice, this does not prevent enthusiasts from creating great games on it. Including - about the fight against 'bugs'.

Fourth place

Source: Godot Source Code Analysis

Sometimes there are interesting errors associated with missing keywords. Example: an exception object is created, but not used at all, since the developer forgets to specify the throw keyword. There are such errors in C # projects and in C ++ projects . The missing keyword met in the Godot Engine game engine.

 Variant Variant::get(const Variant& p_index, bool *r_valid) const { .... if (ie.type == InputEvent::ACTION) { if (str =="action") { valid=true; return ie.action.action; } else if (str == "pressed") { valid=true; ie.action.pressed; } } .... } 

PVS-Studio warning : V607 Ownerless expression 'ie.action.pressed'.

In the above code snippet, you can see that, depending on the values, ie.type and str wanted to return a specific value of type Variant . Only once the return expression was written normally - return ie.action.action; , and the second time forgot the operator return , because of what the desired value will not be returned, and the method will continue its execution.

Third place

Source: PVS-Studio: analyze the Doom 3 code

So we got to the "Top 3". It is closed by a small fragment of the source code from the Doom 3 game. As I said above, the fact that from the side the error looks simple and may be incomprehensible, how can you make such a mistake yet does not mean anything - in practice, what types of errors just cannot be met ...

 void Sys_GetCurrentMemoryStatus( sysMemoryStats_t &stats ) { .... memset( &statex, sizeof( statex ), 0 ); .... } 

PVS-Studio warning : V575 The 'memset' function processes '0' elements. Inspect the third argument.

To better understand the problem, it is worth remembering the memset function signature:

 void* memset(void* dest, int ch, size_t count); 

Comparing the signature of the function with its call, you can see that the last two arguments are mixed up in places. As a result, any block of memory that was required to be reset will remain unchanged.

Second place

The second place was taken by the error found in the C # code of the Xenko game engine.

Source: We are looking for errors in the game engine Xenko

 private static ImageDescription CreateDescription(TextureDimension dimension, int width, int height, int depth, ....) { .... } public static Image New3D(int width, int height, int depth, ....) { return new Image(CreateDescription(TextureDimension.Texture3D, width, width, depth, mipMapCount, format, 1), dataPointer, 0, null, false); } 

PVS-Studio warning : V3065 Parameter 'height' is not utilized inside method's body.

The error was made by passing arguments to the CreateDescription method . If you look at its signature, you can see that the second, third, and fourth parameters are named width , height, and depth, respectively. When called, the arguments are passed in width , width and depth . Suspicious? So the analyzer also found this place suspicious enough to issue a warning.

First place

Source: The long-awaited test of Unreal Engine 4

Headed by our today's "Top 10" error from the game engine Unreal Engine. As in the case of the article “ Top 10 errors in C ++ projects for 2017 ”, seeing the error below, I immediately understood who should lead the list of the most interesting errors.

 bool VertInfluencedByActiveBone( FParticleEmitterInstance* Owner, USkeletalMeshComponent* InSkelMeshComponent, int32 InVertexIndex, int32* OutBoneIndex = NULL); void UParticleModuleLocationSkelVertSurface::Spawn(....) { .... int32 BoneIndex1, BoneIndex2, BoneIndex3; BoneIndex1 = BoneIndex2 = BoneIndex3 = INDEX_NONE; if(!VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[0], &BoneIndex1) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[1], &BoneIndex2) && !VertInfluencedByActiveBone( Owner, SourceComponent, VertIndex[2]) &BoneIndex3) { .... } 

PVS-Studio warning : V564 The '&' operator is applied to bool type value. You've probably forgotten to include the operator.

I would not be surprised if, by comparing the analyzer warning and the above code, you asked the question: “Where is the use of '&' instead of '&&'?”. But, if we note that the last parameter of the VertInfluencedByActiveBone function has a default value, and to simplify the conditional expression of the if operator, everything will become more clear:

 if (!foo(....) && !foo(....) && !foo(....) & arg) 

Pay particular attention to the last subexpression:

 !VertInfluencedByActiveBone(Owner, SourceComponent, VertIndex[2]) &BoneIndex3 

The parameter with the default value played a cruel joke - if there were no default values, this code would simply not compile. But since it is, everything compiles successfully, and the error is no less successfully masked. This analyzer noticed this suspicious place - an infix operation '&', in which the left operand is of type bool and the right operand is of type bool .

Conclusion

I hope I managed to show that static analysis will be useful in developing games and game engines and is another answer to the question of improving the quality of the code (and the final product, as a result). If you are developing in the video game industry, then you simply have to tell your colleagues about static analysis and show this article. Thinking about where to start? Start with PVS-Studio .

If you want to share this article with an English-speaking audience, then please use the link to the translation: Sergey Vasiliev. Static Analysis in Video Game Development: Top 10 Software Bugs