PVS-Studio for Indie Developers

posted in PVS-Studio blog
Published November 10, 2020
Advertisement

Independent game developers, whether single enthusiasts or teams, are faced with the grave problem of having to eliminate bugs from their code. Most independent developers and even studios can't afford the amounts of resources available to large companies to spend on testing and long debugging. And here's where the extensive list of QA tools comes in handy, among which static analysis is one of the crucial ones. It's this methodology that will be covered in this article. Specifically, you will find out how PVS-Studio can help indie developers in the difficult process of bug search.

Let's start with a brief definition of static analysis. It is a process of detecting bugs or defects and suspicious spots in software's source code. Code review is the closest analogy, except that in static analysis, the review is done by a software program rather than your human teammate. And the program can't grow tired or lose focus.

That's exactly what PVS-Studio is – a static analyzer. It looks for bugs in the source code of programs in C, C++, C#, and Java. In addition to the bug-searching diagnostics, the analyzer carries diagnostics to measure some code metrics, detect the use of bad practices (for example, "The Law of The Big Two": V690), and so on.

What it looks like in practice

To make it clear, I'm going to show you two variations of one bug – division by zero – in C++ code.

Division by zero can be easily noticed if the code looks something like this:

int x = 123;
int div = 3;
int y = x / (3 - div);

Yet Visual Studio itself is blind even to this simplest case:

On the contrary, PVS-Studio has no problem noticing the bug:

Executing code like that leads to undefined behavior. We even have a separate post about this issue in our manual.

But the same type of bug may take quite another form:

template <class T> class numeric_limits {
  ....
}
namespace boost {
  ....
}
namespace boost {
  namespace hash_detail  {
    template <class T> void dsizet(size_t x) {
      size_t length = x / (limits<int>::digits - 31); // <=
    }
  }
}

The analyzer will easily recognize this variation as well: V609 Divide by zero. Denominator 'limits <int>::digits - 31' == 0. ConsoleApplication.cpp 12

Some may say game development is not rocket science. "It's not a big deal if you have a bug or two – just fix them if they show up." But after you've spent several days trying to hunt down a bug introduced a week/month/year ago, you start to take bugs more seriously. The longer a bug sits in the product, the costlier it gets, and if it shows up at the user's side, not only do you risk spending a few sleepless nights debugging the code but your reputation is at stake.

The wonderful thing about static analysis is that it helps you find bugs at the earliest development stage, while you are still writing the code. The strategy of running an analyzer once a week/month/year brings to nothing this advantage since most bugs will have been found and fixed by that time through the long and expensive processes of debugging and testing.

Below I show a few examples of bugs detected by PVS-Studio in the code of pretty famous open-source game projects. After all, everyone makes mistakes.

Example 1 (C++)

This error was lurking in the code of "Bullet Physics SDK", a project that was used in development of "Red Dead Redemption" and special effects in Guy Ritchie's "Sherlock Holmes".

What's more, this error led to a real bug in the engine's work, causing forces to be applied on the wrong side of objects. Let's take a look at it:

struct eAeroModel
{
  enum _
  {
    V_Point,            
    V_TwoSided,
    ....
    END
  };
};
 
void btSoftBody::addAeroForceToNode(....)
{
  ....
  if (....)
  {
    if (btSoftBody::eAeroModel::V_TwoSided)
    {
      ....
    }
    ....
  }
....
}

V768 The enumeration constant 'V_TwoSided' is used as a variable of a Boolean-type. btSoftBody.cpp 542

Instead of comparing a variable with the V_TwoSided enumeration's value, the programmer checks that value itself.

This is what the fixed code should look like:

if (m_cfg.aeromodel == btSoftBody::eAeroModel::V_TwoSided)
{
  ....
}

See this article for more bugs found in "Bullet Physics SDK" with PVS-Studio.

Example N2 (C++)

I described an absolutely similar error in my recent article "Amnesia: The Dark Descent or How to Forget to Fix Copy Paste". The developers of the mentioned games series posted the source code of the first two parts on GitHub just before the release of the new part "Amnesia: Rebirth".

Let's move on to the error itself:

void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep)
{
  ....
  if(prevMoveState != mMoveState)
  {
    ....
 
    //Backward
    if(mMoveState == eLuxEnemyMoveState_Backward)
    {
      ....
    }
    ....
    //Walking
    else if(mMoveState == eLuxEnemyMoveState_Walking)
    {
      bool bSync =    prevMoveState == eLuxEnemyMoveState_Running
                   || eLuxEnemyMoveState_Jogging
                    ? true : false;
      ....
    }
    ....
  }
}

The analyzer issued several similar warnings for this code and what follows it (else-if continues further):

V768 The enumeration constant 'eLuxEnemyMoveState_Jogging' is used as a variable of a Boolean-type. LuxEnemyMover.cpp 672

This warning was issued on the following line:

bool bSync =    prevMoveState == eLuxEnemyMoveState_Running
             || eLuxEnemyMoveState_Jogging
              ? true : false;

In this case, comparison of the value of the prevMoveState variable with the eLuxEnemyMoveState_Jogging enumeration element was lost. Moreover in the original code, it was all written in one line.

Most likely, the programmer just wrote the code on the fly resulting in the expression: "prevMoveState must be equal to eLuxEnemyMoveState_Running or eLuxEnemyMoveState_Jogging". Well, besides, the conditional ternary operator is redundant here, the expression is already boolean and will return either true or false.

Example 2 (Java)

PVS-Studio found this defect in the source code of the client-server application XMage for the legendary "Magic: The Gathering".

public final class DragonsMaze extends ExpansionSet {
  ....
  private List<CardInfo> savedSpecialRares = new ArrayList<>();
  ....
  @Override
  public List<CardInfo> getSpecialRare() {
    if (savedSpecialRares == null) {                    // <=
      CardCriteria criteria = new CardCriteria();
      criteria.setCodes("GTC").name("Breeding Pool");
      savedSpecialRares.addAll(....);                   // <=
      criteria = new CardCriteria();
      criteria.setCodes("GTC").name("Godless Shrine");
      savedSpecialRares.addAll(....);
      ....
    }
    return new ArrayList<>(savedSpecialRares);
  }
}

V6008 Null dereference of 'savedSpecialRares'. DragonsMaze.java(230)

The null reference savedSpecialRares gets dereferenced. The programmer probably wanted the method to add some specific rare cards to the container if that container proved to be empty. What's more, savedSpecialRares is declared as an empty collection from the beginning and is never assigned any value. That's why it simply can't have the value null. As a result, this method will always return an empty collection, never meeting the savedSpecialRares == null condition.

The most likely fix for this bug is as follows:

if (savedSpecialRares.isEmpty()) {
....
}

This bug was discussed in the article "Checking the Code of XMage, and Why You Won't Be Able to Get the Special Rare Cards of the Dragon's Maze Collection". If you can't get those rare cards, you should probably blame this very bug.

Example 3 (C#)

Here's another error that seems to affect the game's logic. It was found in the popular tapper "osu!".

protected override void CheckForResult(....)
{
  ....
  ApplyResult(r =>
  {
    if (holdNote.hasBroken
      && (result == HitResult.Perfect || result == HitResult.Perfect))
      result = HitResult.Good;
    ....
  });
}

V3001 There are identical sub-expressions 'result == HitResult.Perfect' to the left and to the right of the '||' operator. DrawableHoldNote.cs 266

Judging by the condition, if the player releases the "note" and their current result is Perfect, it should be downgraded to Good. But for some reason, the result is checked for the value Perfect twice. Let's see what values this enumeration holds:

public enum HitResult
{
    None,
    Miss,
    Meh,
    Ok,
    Good,
    Great,
    Perfect,
}

Between the grades Good and Perfect, there's another grade – Great. I guess it is Great that should be checked for instead of Perfect in the second case. If so, the logic of assigning intermediate results is broken.

This bug was described in the article "Play "osu!", but Watch Out for Bugs". I was curious to find out what had happened to this code after that check, so I dived into the repository looking for it. At first, I failed to find the exact spot where the fix had been applied since this whole file had been refactored completely.

But then I came across this commit, which had split the above code's logic into two more files. It was one of those files that the faulty code had been put into.

Digging through the new file's history, I finally came upon the commit that included the fix. And quite a substantial one:

// If the head wasn't hit or the hold note was broken,
// cap the max score to Meh.
if (   result > HitResult.Meh
    && (!holdNote.Head.IsHit || holdNote.HasBroken))
  result = HitResult.Meh;

Here, if the note isn't hit or the held note is released, your score not simply decreases but drops to the lowest grade possible. It looks like the developers decided to rework the logic radically. But it's also possible that the original snippet had a bug, and if the developers of "osu!" had used PVS-Studio, it would have drawn their attention to the suspicious spot with strange logic and they would have fixed it much earlier.

Using PVS-Studio for free

We've reached the most interesting section. So, how can you use PVS-Studio for free? We provide a number of options, one of which may suit you.

We provide a free license to the following categories:

  1. Open-source projects;
  2. Proprietary projects;
  3. Software security experts;
  4. Microsoft MVPs.

Users from the second category may use PVS-Studio for free after adding special comments to their project, and this option applies to the following types of users:

In this case, you simply need to leave a special comment for PVS-Studio in your code. The other categories are provided a free registration key.

You may find it confusing that open-source developers can both be granted a free key and use the comment option. This "confusion" arises from the fact that it took some time before we introduced new categories and free-use options. The adding of comments as an option for open-source projects dates back to earlier times. We decided to leave it, thus allowing open-source developers to choose the option that suits them most.

To learn more about the free use of PVS-Studio, see this post.

Conclusion

As you can see, static analysis in general and PVS-Studio in particular can detect genuine bugs in software's source code. The errors I showed above were sitting untroubled inside their projects and, almost surely, made them act up every now and then. And if you stop to think how much time (which is money, as is commonly known) it has taken the developers to locate and fix those bugs, which the analyzer could have caught in no time, it gets really frustrating. They could have avoided all this tedious work – without having to get distracted from writing the code – had they used static analysis regularly.

Visit our corporate blog for other posts about the checks of game projects – follow the #GameDev tag.

0 likes 0 comments

Comments

Nobody has left a comment. You can be the first!
You must log in to join the conversation.
Don't have a GameDev.net account? Sign up!
Advertisement

Latest Entries

Advertisement