Статический анализ в видеоигровой индустрии: топ-10 программных ошибок

• Почему вам следует использовать статический анализ
  • Статический анализ в Unreal Engine
  • Джон Кармак о статическом анализе
  • Ошибки, найденные с помощью статического анализа в играх и игровых движках
  • Команды, уже использующие статический анализ
• Топ-10 ошибок из программного кода в видеоигровой индустрии
• Заключение

Если вы занимаетесь разработкой ПО в сфере видеоигровой индустрии и задаётесь вопросом о том, что ещё можно сделать, чтобы повысить качество продукта \ упростить процесс разработки, и при этом не используете статический анализ - самое время начать. Сомневаетесь? Что ж, я попробую вас в этом убедить. Если же вам просто интересно посмотреть на ошибки, которые допускают разработчики из сферы видеоигровой индустрии, то, опять же, вы попали по адресу - специально для вас отобраны наиболее интересные.

Почему вам следует использовать статический анализ

Несмотря на то, что разработка видеоигр включает в себя большое количество этапов, одним из основных остаётся программирование. Даже если вы не пишете тысячи строк кода, так или иначе приходится использовать различные инструменты, от качества которых зависят в том числе удобство работы и конечный результат. Если же вы являетесь разработчиком таких инструментов (например, игровых движков), вам и так должно быть всё понятно.

Почему статический анализ полезен в сфере разработки программного обеспечения в целом?

Есть несколько основных причин:

• Стоимость и сложность исправления ошибки возрастает со временем. Одно из основных преимуществ статического анализа - обнаружение ошибок на ранних этапах разработки (можно находить ошибку в момент написания кода). Следовательно, используя статический анализатор, можно упростить жизнь себе и коллегам, обнаруживая и исправляя многие ошибки до того, как они станут причиной головной боли.
• С помощью статического анализа можно выявлять множество различных паттернов ошибок (copy-paste, опечатки, неправильное использование функций и т.д.).
• Как правило, статический анализ хорош в выявлении тех проблем, которые плохо выявляются с помощью динамического анализа. Впрочем, справедливо и обратное утверждение.
• Негативные моменты использования (например, ложные срабатывания), как правило, 'сглаживаются' командами-разработчиками серьёзных статических анализаторов за счёт различных функций подавления предупреждений (единичное, по паттерну и т.п.), отключения неактуальных диагностик, исключения из анализа файлов и директорий. Благодаря этому можно значительно уменьшить процент 'шума', правильно настроив анализатор. Мой коллега Андрей Карпов продемонстрировал в статье про проверку EFL Core Libraries, что в случае настройки анализатора количество ложных срабатываний составит всего 10-15%.

Но так или иначе, это всё теория, а вам, наверняка, интересны более практические примеры. Что же, они представлены ниже.

Статический анализ в Unreal Engine

Если вы дочитали до этого момента, что-то рассказывать вам о том, что такое Unreal Engine или что за компания такая - Epic Games - не стоит, и, если эти ребята не пользуются у вас авторитетом - напишите мне, кто тогда пользуется.

Команда PVS-Studio несколько раз сотрудничала с командой Epic Games, помогая им внедрять и использовать статический анализ в проекте Unreal Engine и исправлять ошибки \ ложные срабатывания, обнаруженные анализатором. Уверен, это был интересный и полезный опыт для обеих сторон.

Одним из следствий стало добавление в Unreal Engine специального флага, который позволил удобно интегрировать статический анализ в сборочную систему Unreal Engine проектов.

Ключевая мысль проста - ребята заботятся о качестве своего кода и используют для этого разные подходы, одним из которых стало внедрение статического анализа.

Джон Кармак о статическом анализе

Джон Кармак, один из самых известных разработчиков в сфере видеоигровой индустрии, в своё время признал активное использование методики статического анализа одним из главных своих достижений как программиста: "The most important thing I have done as a programmer in recent years is to aggressively pursue static code analysis." В следующий раз, когда кто-то из ваших знакомых скажет, что статический анализ – инструмент для новичков, покажите ему эту цитату. Своё знакомство со статическим анализом Кармак описал в соответствующей статье, с которой я настоятельно рекомендую ознакомиться: как для мотивации, так и для общего развития.

Ошибки, найденные с помощью статического анализа в играх и игровых движках

Пожалуй, один из лучших способов показать пользу статического анализа – демонстрация его возможностей на практике. Этим, например, занимается команда PVS-Studio, проверяя open source проекты.

В выигрыше остаются все:

• Разработчики проектов получают уведомления об ошибках и могут их поправить. В идеале, конечно, подход должен быть иным, нежели исправление ошибок по логу \ статье – стоит самостоятельно проверить анализатором проект и изучить выданные предупреждения. Это важно хотя бы по той причине, что авторы статей могут что-то упускать из виду или непреднамеренно делать акцент на тех ошибках, которые не критичны для проекта.
• Разработчики анализатора могут улучшить инструмент по результатам анализа, а также демонстрируют возможности анализатора по обнаружению ошибок.
• Читатели видят, какие паттерны ошибок допускаются, набираются опыта, знакомятся с методикой статического анализа.

В общем – чем не доказательство эффективности и пользы?

Команды, уже использующие статический анализ

Пока кто-то размышляет о том, стоит ли внедрять статический анализ в свой процесс разработки, некоторые команды уже вовсю его используют и извлекают пользу! Например, Rocksteady, Epic Games, ZeniMax Media, Oculus, Codemasters, Wargaming и другие (источник).

Топ-10 ошибок из программного кода в видеоигровой индустрии

Сразу стоит оговориться, что это не какой-то мега-глобальный 'топ', а те ошибки, найденные анализатором PVS-Studio в играх и игровых движках, которые показались мне наиболее интересными.

Как обычно, для большего интереса предлагаю вам сначала самостоятельно попробовать найти ошибку в приведённом фрагменте кода, а лишь затем читать предупреждение анализатора и описание проблемы.

Десятое место

Источник: Ищем аномалии в X-Ray Engine

На десятом месте расположилась ошибка из игрового движка X-Ray Engine, использовавшегося в играх S.T.A.L.K.E.R. Те, кто играл в игры из этой серии, наверняка помнят много забавных (и не очень) багов, возникавших во время игрового процесса. Особенно это касается S.T.A.L.K.E.R.: Clear Sky, в которую без патчей играть было вообще нереально (до сих пор помню баг, 'убивавший' все сохранения). Как показал анализ игрового движка - баги в коде действительно есть, и их достаточно много. Ниже представлен один из них.

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

Предупреждение PVS-Studio: V530 The return value of function 'empty' is required to be utilized.

Проблема проста - не используют возвращаемое логическое значение метода empty, указывающее, пуст ли контейнер. Судя по тому, что в приведённом выражении кроме вызова метода ничего нет, можно предположить, что разработчик хотел очистить контейнер, но ошибся, вызвав метод empty вместо clear.

Кто-то может сказать, что это слишком простая ошибка для Топ-10. Но в этом и её прелесть! Несмотря на то, что вот так, взглядом со стороны, ошибка кажется достаточно простой и очевидной, подобные 'простые' ошибки не перестают появляться (и обнаруживаться) в различных проектах.

Девятое место

Источник: Долгожданная проверка CryEngine V

Продолжаем раскрывать тему ошибок в игровых движках. На девятом месте - код из CryEngine V. В играх на этом движке я не замечал столько багов, сколько в играх на X-Ray Engine, но 'вскрытие' показало, что подозрительных фрагментов кода движок содержит немало.

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: V519 The 'BlendFactor[2]' variable is assigned values twice successively. Perhaps this is a mistake.

Как неоднократно упоминалось в наших статьях, никто не застрахован от опечаток. На практике также неоднократно подтверждалось, что статический анализ отлично справляется с обнаружением copy-paste и опечаток. Переписывали значения из массива m_auBlendFactor в BlendFactor, но случайно опечатались и два раза написали BlendFactor[2]. Из-за этой путаницы в BlendFactor[2] будет записано значение из m_auBlendFactor[3], а в BlendFactor[3] значение не поменяется.

Восьмое место

Источник: Единорог в космосе: проверяем исходный код 'Space Engineers'

Немного изменим курс и перейдём от C++ к C#. Перед нами - код из проекта Space Engineers - игры-'песочницы' о строительстве и обслуживании космических структур. Сам я в игру не играл, но как сказал один из комментаторов статьи: "Я не сильно удивлён результатами :)". Что ж, интересные ошибки действительно удалось найти, ниже - две из них.

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:

• V3010 The return value of function 'Format' is required to be utilized.
• V3010 The return value of function 'Format' is required to be utilized.

Как видите, ошибки игнорирования возвращаемых значений методов встречаются не только в коде на C++, но и на C#. Метод String.Format составляет результирующую строку на основе строки формата и объектов для форматирования, после чего возвращает результат как возвращаемое значение метода. В коде выше в else-ветви есть два вызова string.Format, но их возвращаемые значения не используются. Скорее всего, необходимо было залогировать эти сообщения по аналогии с тем, как это сделано в then-ветви оператора if, используя метод MySandboxGame.Log.WriteLine.

Седьмое место

Источник: Проверка проекта Quake III Arena GPL

Я уже говорил, что статический анализ хорошо обнаруживает опечатки? Пример ниже - ещё одно тому подтверждение.

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

Предупреждение PVS-Studio: V537 Consider reviewing the correctness of 'scale_x' item's usage.

Происходит запись в переменные x и y, но в обоих присвоениях в выражении вычисления значения используется подвыражение p->scale_x. Выглядит очень подозрительно и похоже на то, что во втором случае необходимо заменить выражение p->scale_x на p->scale_y.

Шестое место

Источник: Проверяем исходный C#-код Unity

Компания Unity Technologies не так давно открыла исходный код собственного игрового движка - Unity. Мы не могли пройти мимо такого события и при анализе нашли немало интересных мест. Ниже - одно из них.

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

Предупреждение PVS-Studio: V3063 A part of conditional expression is always true if it is evaluated: pageSize <= 1000.

Имеем неверную проверку диапазона для pageSize. Скорее всего планировалось проверить, что значение pageSize лежит в пределах [1; 1000]. Однако допущена досадная опечатка: вместо оператора '&&' программист случайно написал '||'. Получается, что подвыражение на самом деле ничего не проверяет.

Пятое место

Источник: Анализируем ошибки в открытых компонентах Unity3D

На соседнем месте расположилась интересная ошибка из компонентов Unity3D. Статья-источник была написана более чем за год до того, как был открыт исходный код движка Unity. Тем не менее, уже тогда можно было найти что-то интересное.

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: V3080 Possible null dereference. Consider inspecting 't.staticFieldBytes'.

Обратите внимание на лямбда-выражение, переданное в качестве аргумента методу Where. Судя по коду, коллекция typeDescriptions может содержать такие элементы, у которых член staticFieldBytes может иметь значение null. Именно поэтому, прежде чем обратиться к свойству Length, выполняется проверка, что staticFieldBytes != null. Вот только были перепутаны операторы, и вместо '&&' используется '&'. Это значит, что независимо от результата вычисления левого выражения (true \ false) правое выражение также будет вычисляться, из-за чего, если staticFieldBytes == null, при обращении к свойству Length будет сгенерировано исключение типа NullReferenceException. При использовании '&&' такой ситуации не возникло бы, так как, если staticFieldBytes == null, правая часть выражения не вычислялась бы.

Несмотря на то, что Unity – единственный движок, дважды попавший в данный "Топ-10", энтузиастам это не мешает создавать на нём много замечательных игр. В том числе – про борьбу с 'багами'.

Четвертое место

Источник: Анализ исходного кода движка Godot

Иногда встречаются интересные ошибки, связанные с пропущенными ключевыми словами. Пример: создаётся объект исключения, но никак не используется, так как разработчик забывает указать ключевое слово throw. Встречаются такие ошибки и в C# проектах, и в C++ проектах. Пропущенное ключевое слово встретилось и в игровом движке Godot 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: V607 Ownerless expression 'ie.action.pressed'.

В приведённом фрагменте кода видно, что в зависимости от значений ie.type и str хотели вернуть определённое значение типа Variant. Вот только один раз выражение возврата написали нормально - return ie.action.action;, а во второй раз забыли оператор return, из-за чего нужное значение не будет возвращено, а метод продолжит своё выполнение.

Третье место

Источник: PVS-Studio: анализируем код Doom 3

Вот мы и подобрались к "Топ-3". Замыкает его небольшой фрагмент исходного кода из игры Doom 3. Как я говорил выше, то, что со стороны ошибка выглядит просто и может быть непонятно, как можно так ошибиться, ещё ничего не значит - на практике каких типов ошибок только не встретишь...

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

Предупреждение PVS-Studio: V575 The 'memset' function processes '0' elements. Inspect the third argument.

Чтобы лучше понять проблему, стоит вспомнить сигнатуру функции memset:

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

Сопоставив сигнатуру функции с её вызовом, можно заметить, что последние два аргумента перепутаны местами. Как следствие - какой-то блок памяти, который требовалось обнулить, так и останется неизменённым.

Второе место

Второе место заняла ошибка, найденная в C# коде игрового движка Xenko.

Источник: Ищем ошибки в игровом движке 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: V3065 Parameter 'height' is not utilized inside method's body.

Ошибку допустили, передавая аргументы методу CreateDescription. Если посмотреть на его сигнатуру, можно увидеть, что второй, третий и четвёртый параметры имеют названия width, height и depth соответственно. При вызове же передаются аргументы width, width и depth. Подозрительно? Вот и анализатор тоже счёл это место достаточно подозрительным, чтобы выдать предупреждение.

Первое место

Источник: Долгожданная проверка Unreal Engine 4

Возглавляет наш сегодняшний "Топ-10" ошибка из игрового движка Unreal Engine. Как и в случае со статьёй "Toп 10 ошибок в C++ проектах за 2017 год", увидев приведённую ниже ошибку, я сразу понял, кто должен возглавить список самых интересных ошибок.

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: V564 The '&' operator is applied to bool type value. You've probably forgotten to include parentheses or intended to use the '&&' operator.

Не удивлюсь, если, сопоставляя предупреждение анализатора и приведённый код, вы задались вопросом: "А где здесь, собственно, использование '&' вместо '&&'?". Но, если обратить внимание, что последний параметр функции VertInfluencedByActiveBone имеет значение по умолчанию, и упростить условное выражение оператора if, всё станет более понятно:

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

Обратите особое внимание на последнее подвыражение:

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

Параметр со значением по умолчанию сыграл злую шутку - если бы значения по умолчанию не было, данный код бы просто не скомпилировался. Но так как оно есть, всё успешно компилируется, а ошибка не менее успешно маскируется. Вот это подозрительное место и заметил анализатор - инфиксная операция '&', в которой левый операнд имеет тип bool, а правый - int32.

Заключение

Надеюсь, мне удалось показать, что статический анализ будет полезен при разработке игр и игровых движков и является ещё одним ответом на вопрос о повышении качества кода (и конечного продукта, как следствие). Если вы занимаетесь разработкой в сфере видеоигровой индустрии, то просто обязаны рассказать коллегам о статическом анализе и показать эту статью. Думаете с чего начать? Начните с PVS-Studio.