Чем PVS-Studio может помочь инди-разработчикам

Обучающий материал

nsodoBo

В процессе инди-разработки любой энтузиаст или даже целая группа сталкивается с одной серьёзной проблемой — с необходимостью поиска ошибок в коде игры. Ресурсы больших компаний на тестирование и долгую отладку недоступны абсолютному большинству инди-разработчиков или даже инди-студий. И как раз с этим может помочь широкий список инструментов QA кода. Среди которых немаловажным является статический анализ кода. О нем и пойдёт речь в этой статье. А конкретно, хотелось бы рассказать о том, как PVS-Studio может помочь инди-разработчикам со сложным процессом поиска ошибок.

Начнем с краткого описания того, что такое статический анализ. Это процесс выявления ошибок или недочетов и подозрительных мест в исходном коде программ. Самой очевидной аналогией для этого процесса будет код-ревью. Только вместо вашего коллеги код проверяет программа, которая не устанет и у которой не «замылится» глаз.

PVS-Studio, в свою очередь, это как раз статический анализатор, который ищет ошибки в исходном коде программ, написанных на C, C++, C# и Java. Кроме поиска ошибок PVS-Studio имеет диагностики, которые подсчитывают некоторые метрики кода, отлавливает использование bad practices (например, нарушение «The Law of The Big Two»: V690) и т. д.

Как это выглядит на практике

Для наглядности приведу две вариации одной и той же ошибки, деления на ноль, на языке C++.

Найти деление на ноль может быть очень просто, если выглядеть оно будет вот так:

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

Хотя инструментарий самой Visual Studio молчит на таком простейшем примере:

3AR8MU0

PVS-Studio легко обнаруживает эту ошибку:

MhjCzYJ

В результате исполнения такого кода мы получим неопределённое поведение. На этот счёт в нашем мануале есть даже отдельная заметка.

Но абсолютно такая же по своей сути ошибка может выглядеть и таким образом:

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); // <=
    }
  }
}

И такое деление на ноль также будет обнаружено анализатором: V609 Divide by zero. Denominator 'limits <int>::digits — 31' == 0. ConsoleApplication.cpp 12

Кто-то может подумать, что разработка игр это не rocket science. Будет там ошибка, ну и ничего страшного -если найдётся, поправим. Но когда несколько дней пытаешься отловить баг в коде, который был написан неделю/месяц/год назад, отношение к ошибкам становится уже не таким легкомысленным. И чем дольше ошибка сохраняется в продукте, тем дороже она стоит, а уж если она достигнет пользователя, то можно заработать не только бессонные ночи дебага, но и репутационные потери.

lwec7mP

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

Далее приведу несколько примеров ошибок, которые PVS-Studio обнаружил в исходном коде достаточно известных open-source игровых проектов. Ведь никто не застрахован от ошибок.

Пример № 1 (C++)

Эта ошибка пряталась в исходном коде «Bullet Physics SDK», который использовался при разработке «Red Dead Redemption», а также для разработки спецэффектов в «Шерлоке Холмсе» Гая Ричи.

Более того, эта ошибка приводила к реальному багу в работе движка, из-за неё силы могли применяться к объектам не с той стороны. Взглянем на ошибку:

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

Итак, в этом участке кода забыли произвести сравнение со значением перечисления. Вместо этого проверялось само значение перечисления V_TwoSided.

Правильный код будет выглядеть следующим образом:

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

Почитать подробнее об ошибках, найденных в «Bullet Physics SDK» с помощью PVS-Studio можно в этой статье.

Пример № 2 (С++)

Абсолютно аналогичную ошибку я описывала в своей недавней статье «Amnesia: The Dark Descent или как забыть поправить копипасту». Разработчики упомянутой серии игр как раз перед выходом новой части «Amnesia: Rebirth» выложили на GitHub исходный код двух первых частей.

Перейдём к самой ошибке:

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;
      ....
    }
    ....
  }
}

На этот код и его продолжение (else-if продолжаются и дальше) анализатор выдал несколько похожих предупреждений вида:

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

Это предупреждение выдавалось на следующую строку:

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

В этом случае потерялось сравнение значения переменной prevMoveState с элементом перечисления eLuxEnemyMoveState_Jogging. В оригинале это все было еще и записано в одну строку.

Скорее всего программист просто писал код по ходу своих мыслей и написал выражение так, как и думал: «prevMoveState должно быть равно eLuxEnemyMoveState_Running или eLuxEnemyMoveState_Jogging». Ну и к тому же, условный тернарный оператор тут лишний, выражение и так является логическим и вернёт или true или false.

Пример №3 (Java)

Следующая ошибка была найдена PVS-Studio в исходном коде клиент-серверного приложения XMage для легендарной «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)

Тут происходит разыменовывание нулевой ссылки savedSpecialRares. Скорее всего подразумевалось, что если этот контейнер пуст, то при вызове функции в него нужно добавить определённые редкие карты. Более того, savedSpecialRares изначально объявляется пустой коллекцией и нигде больше не переприсваивается. Поэтому значение null для неё попросту невозможно. В итоге этот метод всегда будет возвращать пустую коллекцию, никогда не выполняя условия savedSpecialRares == null.

Наиболее вероятный вариант исправления этой ошибки:

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

Эта ошибка рассматривалась в статье «Проверка кода XMage и почему недоступны специальные редкие карточки для коллекции Dragon’s Maze». И как раз из-за этой ошибки в этой игре возможно не удастся получить те самые редкие карточки.

Пример № 4 (C#)

И приведу еще один пример, скорее всего влияющий на логику работы игры. Эта ошибка была допущена в популярном музыкальном таппере «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

Судя по условию, если зажимание «ноты» прервалось и результат был Perfect, то надо снизить результат до Good. Но почему-то результат дважды проверяется на Perfect. Заглянем в возможные значения этого перечисления:

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

Видно, что между оценкой Good и Perfect, есть еще такая оценка, как Great. Возможно, вместо второй проверки на Perfect должна была быть именно проверка на Great. В таком случае логика присвоения промежуточных результатов работает неверно.

Эта ошибка освещалась в статье «В 'osu!' играй, про ошибки не забывай». Мне стала интересна дальнейшая судьба этого кода, и я полезла искать его в репозитории. Сначала не вышло найти, где этот код был исправлен, так как содержимое всего этого файла было полностью отрефакторено.

Потом я нашла вот этот коммит, который разбивал логику этого кода на еще пару файлов. В один из этих файлов и был вынесен код с ошибкой.

8xCb60W

Покопавшись в истории уже нового файла, я в итоге нашла коммит который, наконец, исправлял этот код. Причем исправление было капитальным:

// 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;

Здесь при прерывании или пропуске «ноты», которую надо зажать, оценка не просто снижается, а вообще сводится к самой низкой. Возможно, разработчики решили кардинально изменить логику. Но не менее вероятно, что исходный код был ошибочным и, если бы разработчики «osu!» использовали PVS-Studio, то анализатор указал бы им на место с такой подозрительной логикой и они могли бы исправить его гораздо раньше.

Как использовать PVS-Studio бесплатно

AbKQtzY

Ну и перейдём к самому интересному. Как же использовать PVS-Studio бесплатно. Есть несколько вариантов. Возможно один из них подойдёт и Вам.

Возможность бесплатно использовать анализатор предоставляются следующим категориям:

  1. Открытые проекты;
  2. Закрытые проекты;
  3. Эксперты безопасности;
  4. Microsoft MVP.

Вторая категория из этого списка может использовать PVS-Studio бесплатно с помощью добавления в проект специальных комментариев и ей могут воспользоваться:

В этом случае разработчику просто будет нужно оставить специальный комментарий для PVS-Studio в своём коде. Для остальных категорий предоставляется бесплатный лицензионный ключ.

Думаю, читателя может смутить, что открытым проектам выдаётся бесплатный ключ, и в то же время они могут воспользоваться вариантом с комментариями в своём исходном коде. Такая «путаница» возникла из-за того, что новые категории и варианты бесплатного использования появились не сразу. В итоге вариант с комментариями для открытых проектов остался с более ранних времен. Мы решили не убирать такую возможность, и теперь разработчики открытых проектов могут сами выбрать подходящий для себя вариант.

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

Заключение

Как видите, статический анализ и конкретно PVS-Studio способны находить реальные ошибки в исходном коде программ. Ошибки, которые я привела в этой статье спокойно жили в кодовой базе своих проектов и скорее всего приводили к неправильному поведению этих программ. А уж сколько времени (а время — это, как известно, деньги) было потрачено разработчиками на локализацию и исправление ошибок, которые легко было бы отловить анализатором, страшно представить. Избежать этого можно было бы очень просто, причем даже в сам момент написания этого кода, с помощью как раз-таки регулярного использования статического анализа.

В нашем корпоративном блоге вы можете найти другие наши статьи про проверку игровых проектов, перейдя по тегу #GameDev.

64 комментария

Написать комментарий