Статический анализ как часть процесса разработки в Unreal Engine

Автор: Андрей Карпов

Unreal Engine продолжает развиваться по мере добавления нового кода и изменения ранее написанного кода. Каковы неизбежные последствия постоянного развития проекта? Появление в коде новых ошибок, которые программист хочет выявить как можно раньше. Один из способов уменьшить количество ошибок - использование статического анализатора типа PVS-Studio. Более того, анализатор не только развивается, но и постоянно учится искать новые паттерны ошибок, некоторые из которых мы обсудим в этой статье. Если вы заботитесь о качестве кода, эта статья для вас.

Статья подготовлена Андреем Карповым; фрагменты кода предоставили Илья Иванов и Сергей Васильев из команды PVS-Studio. Эта статья изначально была опубликована в блоге Unreal Engine.

Статический анализ кода, теоретическая справка

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

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

Как правило, проверка кода работает неплохо, поскольку программисты гораздо легче замечают ошибки в чужом коде, чем в собственном. Вы можете найти более подробную информацию о методологии проверки кода в замечательной книге Стива МакКоннелла «Code Complete».

Методология проверки кода имеет два недостатка:

Чрезвычайно высокая цена. Необходимо отвлечь нескольких программистов от их основных задач, чтобы просмотреть вновь написанный код или переписать код после внесения рекомендуемых модификаций. При этом программистам следует регулярно делать перерывы для отдыха во время работы. Если человек пытается просмотреть большие фрагменты кода, есть опасность быстро потерять внимание и использовать его.
Также сложно обнаружить ошибки, не связанные напрямую с новым / измененным кодом. Глядя на свежий фрагмент кода, нелегко предположить, что функция malloc работает некорректно, потому что файл заголовка stdlib.h не включен . Подробнее об этой ситуации читайте в статье Хорошая 64-битная ошибка в C ». Еще один пример: изменение типа функции или переменной в заголовочном файле. В идеале программист должен просмотреть весь код, в котором эта функция или переменная используется после таких изменений. На практике это занимает слишком много времени, и, как правило, обзор ограничивается только теми фрагментами, где программист что-то изменил.

С одной стороны, хочется регулярно проводить ревью кода. С другой стороны, это слишком дорого. Компромисс - статический анализ. Инструменты статического анализа проверяют исходные тексты программ и дают рекомендации программистам по проверке определенных фрагментов кода. Анализаторы не устают и проверяют весь код, на который повлияли изменения в заголовочных файлах. Конечно, программа не заменит полноценную проверку кода, выполняемую командой разработчиков. Однако соотношение выгода / цена делает статический анализ весьма полезным методом, принятым многими компаниями.

Как и у любой другой методологии обнаружения ошибок, статический анализ имеет свои сильные и слабые стороны. Не существует идеального метода тестирования программ. Наилучших результатов можно достичь, используя комбинацию различных подходов, таких как: хороший стиль кодирования, статический анализ кода, динамический анализ кода, модульное тестирование, регрессионное тестирование и т. Д.

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

Дело в том, что чем раньше обнаружена ошибка, тем дешевле ее исправить. Так, согласно книге МакКоннелла «Code Complete», исправление ошибки на этапе тестирования кода в десять раз дороже, чем на этапе написания кода:

Таблица N1. Рисунок 7 - Средние затраты на исправление дефектов в зависимости от времени их обнаружения (данные, представленные в таблице, взяты из книги С. МакКоннелла «Code Complete»)

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

Актуальность статических анализаторов со временем будет расти. Это связано с постоянным ростом кодовой базы современных приложений. Программы становятся больше и сложнее. При этом плотность ошибок зависит от размера кода нелинейно.

Чем крупнее проект, тем больше ошибок на 1000 строк кода. Взгляните на этот график:

Таблица 2. Размер проекта и типичная плотность ошибок. Источник: «Качество программ и продуктивность программистов» (Jones, 1977), «Оценка стоимости программного обеспечения» (Jones, 1998).

Давайте построим графики, чтобы нам было легче понять данные.

График 1. Типичная плотность ошибок в проекте. Синий - максимальное количество. Красный - среднее число. Зеленый - наименьшее количество ошибок.

График показывает, что с ростом проекта программисты вынуждены использовать больше инструментов, позволяющих поддерживать требуемое качество проекта. Невозможно создать качественный код так, как это делали, скажем, 8 лет назад. Для команды это может быть неприятным открытием: вроде как обычно пишут код, но с кодом ситуация ухудшается.

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

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

Джон Кармак. Статический анализ кода.
Википедия. Статический анализ кода.
Википедия. Список инструментов для статического анализа кода.
Эл Бесси, Кен Блок, Бен Чельф, Энди Чоу, Брайан Фултон, Сет Халлем, Чарльз Анри-Грос, Ася Камски, Скотт МакПик, Доусон Энглер. Несколько миллиардов строк кода позже: использование статического анализа для поиска ошибок в реальном мире.
Екатерина Миловидова. Видео о статическом анализе кода.
Блог команды PVS-Studio.

Теперь пора перейти от теории к практике и посмотреть, как статический анализ помогает в таком проекте, как Unreal Engine.

Unreal Engine

Нашей команде снова выпала честь работать с кодом Unreal Engine!

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

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

Кодовая база Unreal Engine существенно изменилась за два года. Некоторые фрагменты добавлялись, некоторые удалялись, иногда пропадали целые папки. Поэтому не всем частям кода уделено должное внимание, а значит, над PVS-Studio есть над чем поработать.

Хочу поблагодарить компанию Epic Games за заботу о своем коде и использование таких инструментов, как PVS-Studio. Читатель может воспринять это с улыбкой: «Конечно, ваша команда должна хвалить компанию Epic Games, потому что она ваш заказчик». Если честно, у нас есть мотив оставить положительный отзыв о разработчиках из компании Epic Games. Однако я говорю слова похвалы абсолютно искренне. Тот факт, что компания использует инструменты статического анализа, показывает зрелость цикла разработки проекта и внимание, уделяемое обеспечению надежности и безопасности кода.

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

GCC (ошибки)
Clang (баги, баги, баги)
Valgrind (баги)
GDB (ошибки)
FreeBSD (ошибки, ошибки)
"и так далее".

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

Интересные ошибки

Я не буду рассказывать обо всех ошибках, которые мы нашли и исправили, я выделю только те, которые, на мой взгляд, заслуживают внимания. Желающие могут ознакомиться с другими ошибками в pull request на Github. Чтобы получить доступ к исходному коду и указанному запросу на перенос, у вас должен быть доступ к репозиторию Unreal Engine на GitHub. Для этого у вас должны быть учетные записи на GitHub и EpicGames, которые должны быть связаны на сайте unrealengine.com. После этого вам необходимо принять приглашение присоединиться к сообществу Epic Games на GitHub. Инструкция.

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

uint8* Data = (uint8*)PointerVal;
if (Data != nullptr || DataLen == 0)
{
  NUTDebug::LogHexDump(Data, DataLen);
}
else if (Data == nullptr)
{
  Ar.Logf(TEXT("Invalid Data parameter."));
}
else // if (DataLen == 0)
{
  Ar.Logf(TEXT("Invalid DataLen parameter."));
}

Предупреждение PVS-Studio: V547 Выражение ‘Data == nullptr’ всегда истинно. unittestmanager.cpp 1924 г.

Если условие (Data! = Nullptr || DataLen == 0) неверно, это означает, что указатель Data определенно равен nullptr . Поэтому дальнейшая проверка (Data == nullptr) не имеет смысла.

Правильный вариант кода:

if (Data != nullptr && DataLen > 0)

Диагностическая версия V547 была написана в 2010 году. Однако механизм оценки значений переменных был несовершенным, и это не позволяло обнаружить эту ошибку. Анализатор смутил при проверке значения переменной DataLen и не смог определить, чему равны значения переменных в различных условиях. Человеку, наверное, не составит труда проанализировать такой код, но не все так просто, когда дело доходит до написания алгоритмов для поиска таких ошибок.

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

Мы также делаем «внешние» улучшения, поддерживая новые конструкции, появляющиеся в новых версиях языка C ++. Тем не менее, недостаточно научиться разбирать C ++ 11, C ++ 14 и так далее. Не менее важно усовершенствовать старую диагностику и внедрить новую диагностику, которая обнаружит ошибки в новых языковых конструкциях. В качестве примера рассмотрим диагностику V714, которая ищет неправильные петли на основе диапазона. В Unreal Engine диагностика V714 указывает на следующий цикл:

for (TSharedPtr<SWidget> SlateWidget : SlateWidgets)
{
  SlateWidget = nullptr; 
}

Предупреждение PVS-Studio: V714 Переменная не передается в цикл foreach по ссылке, но ее значение изменяется внутри цикла. vreditorradialfloatingui.cpp 170

Программист хотел присвоить значение nullptr всем элементам в контейнере SlateWidgets. Ошибка в том, что SlateWidget - это обычная локальная переменная, которая является создается во время каждой новой итерации цикла. Присвоение значения этой переменной не приводит к изменению элемента в контейнере. Мы должны использовать ссылку, чтобы код работал правильно:

for (TSharedPtr<SWidget> &SlateWidget : SlateWidgets)
{
  SlateWidget = nullptr; 
}

Конечно, мы также добавляем диагностику, не связанную с языком. Например, диагностики V767 не существовало в 2015 году, когда наша команда писала предыдущую статью о проверке Unreal Engine. Эта диагностика появилась в PVS-Studio в версии 6.07 (8 августа 2016 г.). Благодаря этой диагностике мы обнаружили такую ошибку:

for(int i = 0; i < SelectedObjects.Num(); ++i)
{
  UObject* Obj = SelectedObjects[0].Get();
  EdObj = Cast<UEditorSkeletonNotifyObj>(Obj);
  if(EdObj)
  {
    break;
  }
}

Предупреждение PVS-Studio: V767 Подозрительный доступ к элементу массива SelectedObjects по постоянному индексу внутри цикла. skeletonnotifydetails.cpp 38

Цикл должен содержать поиск элемента, имеющего тип UEditorSkeletonNotifyObj. Из-за опечатки во время выбора элемента вместо переменной i был написан числовой литерал 0.

Правильный вариант кода:

UObject* Obj = SelectedObjects[i].Get();

Рассмотрим еще одну диагностику V763, которая также появилась в PVS-Studio 6.07. Эта ошибка довольно забавная, но я должен процитировать довольно длинный текст функции RunTest:

bool FCreateBPTemplateProjectAutomationTests::RunTest(
  const FString& Parameters)
{
  TSharedPtr<SNewProjectWizard> NewProjectWizard;
  NewProjectWizard = SNew(SNewProjectWizard);
  TMap<FName, TArray<TSharedPtr<FTemplateItem>> >& Templates =
    NewProjectWizard->FindTemplateProjects();
  int32 OutMatchedProjectsDesk = 0;
  int32 OutCreatedProjectsDesk = 0;
  GameProjectAutomationUtils::CreateProjectSet(Templates, 
    EHardwareClass::Desktop, 
    EGraphicsPreset::Maximum, 
    EContentSourceCategory::BlueprintFeature,
    false,
    OutMatchedProjectsDesk,
    OutCreatedProjectsDesk);
  int32 OutMatchedProjectsMob = 0;
  int32 OutCreatedProjectsMob = 0;
  GameProjectAutomationUtils::CreateProjectSet(Templates, 
    EHardwareClass::Mobile,
    EGraphicsPreset::Maximum,
    EContentSourceCategory::BlueprintFeature,
    false,
    OutMatchedProjectsMob,
    OutCreatedProjectsMob);
  return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
         ( OutMatchedProjectsMob  == OutCreatedProjectsMob  );
}

Следующая часть является наиболее важной:

Программист пытается инициализировать переменные OutMatchedProjectsDesk и OutCreatedProjectsDesk с помощью первого вызова функции CreateProjectSet .
Используя второй вызов функции CreateProjectSet, выполняется попытка инициализировать переменные OutMatchedProjectsMob и OutCreatedProjectsMob.

Затем проверяется соответствие значений этих переменных условию:

return ( OutMatchedProjectsDesk == OutCreatedProjectsDesk ) &&
       ( OutMatchedProjectsMob  == OutCreatedProjectsMob  );

Не ищите ошибки в теле проверенной функции, их там нет. Я привел этот код, чтобы показать, что функция CreateProjectSet должна записывать значения в две переменные, переданные как два последних фактических аргумента.

Ошибка скрывается в функции CreateProjectSet:

static void CreateProjectSet(.... int32 OutCreatedProjects,
                                  int32 OutMatchedProjects)
{
  ....
  OutCreatedProjects = 0;
  OutMatchedProjects = 0;
  ....
  OutMatchedProjects++;
  ....
  OutCreatedProjects++;
  ....
}

PVS-Studio выдаст здесь два предупреждения:

V763 Параметр OutCreatedProjects всегда перезаписывается в теле функции перед использованием. gameprojectautomationtests.cpp 88
Параметр V763 OutMatchedProjects всегда перезаписывается в теле функции перед использованием. gameprojectautomationtests.cpp 89

Совершенно прав анализатор, когда предупреждает, что значения аргументов OutCreatedProjects и OutMatchedProjects никак не используются, а сразу перезаписываются на 0 .

Ошибка проста: программист забыл передать параметры по ссылке. Правильный вариант кода:

static void CreateProjectSet(.... int32 &OutCreatedProjects,
int32 &OutMatchedProjects)

Я привел ошибки, для обнаружения которых требуется хотя бы некоторая внимательность. Однако есть еще много простых и банальных ошибок. Например, отсутствуют операторы break:

{
  case EWidgetBlendMode::Opaque:
    ActualBackgroundColor.A = 1.0f;
  case EWidgetBlendMode::Masked:
    ActualBackgroundColor.A = 0.0f;
}

Или некорректное сравнение нескольких переменных на равенство:

checkf(GPixelFormats[PixelFormat].BlockSizeX 
    == GPixelFormats[PixelFormat].BlockSizeY 
    == GPixelFormats[PixelFormat].BlockSizeZ 
    == 1, 
  TEXT("Tried to use compressed format?"));

Если кто-то плохо знаком с C ++ и не понимает, почему это сравнение некорректно, предлагаю посмотреть описание диагностики V709.

Эти ошибки являются наиболее многочисленными среди обнаруженных PVS-Studio. Но если они выглядят такими простыми, почему до сих пор остаются незамеченными?

Они настолько банальны, если освещены в статье для читателя. Их действительно сложно найти в коде реальных приложений. Даже делая обзор кода, можно посмотреть на блок кода

{
  case EWidgetBlendMode::Opaque:
    ActualBackgroundColor.A = 1.0f;
  case EWidgetBlendMode::Masked:
    ActualBackgroundColor.A = 0.0f;
}

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

Теперь давайте обсудим вопрос: можем ли мы каким-либо образом уменьшить количество ошибок?

Вывод

Разработчики Unreal Engine заботятся о качестве своего кода, и команда PVS-Studio делает все возможное, чтобы помочь им в их начинаниях.

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

Желаю как можно меньше ошибок в программах.

Статический анализ как часть процесса разработки в Unreal Engine

Статический анализ кода, теоретическая справка

Unreal Engine

Интересные ошибки

Рекомендация

Вывод

Вопросы по теме