Static Code Analysis With PVS-Studio (Part 1)

UPDATE 3rd June 2016: the fixes below were added to the code base in UE4.12

Static Code Analyzers are a fantastic tool to have in your arsenal during development… often, they find and enable you to fix bugs before you even know about them.

PVS-Studio (www.viva64.com/en/pvs-studio/) is one of the best that I’ve used. Highly recommended, particularly to AAA development studios.

In part 1 of this series, I’ll give you just two out of the many things that PVS brought to our attention.

First up, let’s take a look at a “duplicate code” warning that it gave us in ShowFlags.cpp:-

EViewModeIndex FindViewMode(const FEngineShowFlags& EngineShowFlags)
{
  if(EngineShowFlags.VisualizeBuffer)
  {
    return VMI_VisualizeBuffer;
  }
  else if(EngineShowFlags.StationaryLightOverlap)
  {
    return VMI_StationaryLightOverlap;
  }
  else if(EngineShowFlags.ShaderComplexity)
  {
    return VMI_ShaderComplexity;
  }
  // Test QuadComplexity before ShaderComplexity because QuadComplexity also use ShaderComplexity
  else if(EngineShowFlags.QuadComplexity)
  {
    return VMI_QuadComplexity;
  }
  else if(EngineShowFlags.WantedMipsAccuracy)
  {
    return VMI_WantedMipsAccuracy;
  }
  else if(EngineShowFlags.TexelFactorAccuracy)
  {
    return VMI_TexelFactorAccuracy;
  }
  else if(EngineShowFlags.ShaderComplexity)
  {
    return VMI_ShaderComplexity;
  }
  else if(EngineShowFlags.VisualizeLightCulling)
  {
    return VMI_LightComplexity;
  }
<-- LOTS MORE CODE -->
  return EngineShowFlags.Lighting ? VMI_Lit : VMI_Unlit;
}

See the comment in the middle about testing QuadComplexity before ShaderComplexity? Well, right before it – a ShaderComplexity test. And, not too far after, a second ShaderComplexity test. Doh! The correct code should have the first test removed, of course.

The second problem that I’m going to show you is within UnrealHeaderTool. Note the last 2 lines of the following function:-

bool FManifestModule::IsCompatibleWith(const FManifestModule& ManifestModule)
{
  return Name == ManifestModule.Name
    && ModuleType == ManifestModule.ModuleType
    && LongPackageName == ManifestModule.LongPackageName
    && BaseDirectory == ManifestModule.BaseDirectory
    && IncludeBase == ManifestModule.IncludeBase
    && GeneratedIncludeDirectory == ManifestModule.GeneratedIncludeDirectory
    && BaseDirectory == ManifestModule.BaseDirectory
    && PublicUObjectClassesHeaders == ManifestModule.PublicUObjectClassesHeaders
    && PublicUObjectHeaders == ManifestModule.PublicUObjectHeaders
    && PrivateUObjectHeaders == ManifestModule.PrivateUObjectHeaders
    && PCH == ManifestModule.PCH
    && GeneratedCPPFilenameBase == ManifestModule.GeneratedCPPFilenameBase
    && SaveExportedHeaders == SaveExportedHeaders
    && GeneratedCodeVersion == GeneratedCodeVersion;
}

which, of course, should be changed to:-

    && SaveExportedHeaders == ManifestModule.SaveExportedHeaders
    && GeneratedCodeVersion == ManifestModule.GeneratedCodeVersion;

So, as you can see, PVS-Studio is pretty cool. The errors above are both pretty obvious, when they’re staring you in the face, but when you’re dealing with codebases as large as UE4, these problems can often stay hidden for quite some time.

Credit(s): Robert Troughton (Coconut Lizard)
Status: Fixed in 4.12