Flax Engine. Знакомство с игровым движком и анализ его исходного кода

"Как будто у Unreal и Unity родился ребёнок" — такое трогательное описание дали этому движку в GameDev-сообществе. Эта фраза не только мило звучит, но и точно передаёт его суть, ведь движок действительно задумывался как нечто среднее между Unity Engine и Unreal Engine.

Привет вам, дорогие читатели! Я хочу познакомить вас с ещё одной интересной находкой с бескрайних просторов GitHub. Flax Engine — это полноценный мультиплатформенный коммерческий игровой движок от польских разработчиков.

В этой статье мы кратко рассмотрим основные особенности движка, а после разберём наиболее интересные проблемы, найденные в его исходном коде с помощью статического анализатора кода PVS-Studio. Это хороший способ немного прокачать своё умение замечать баги и опечатки в C# и C++ коде.

PVS-Studio — это статический анализатор кода, предназначенный для поиска потенциальных ошибок и уязвимостей в С#, C, C++ и Java коде без его фактического выполнения. С помощью PVS-Studio можно проанализировать как отдельные файлы, так и проект целиком.

Результатом такого анализа является отчёт с предупреждениями, с которым можно удобно работать через специальную панель в ваших любимых IDE:

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

Итак, уважаемые читатели, знакомьтесь, это Flax Engine:

Что же выделяет Flax среди других движков?

Поддержка скриптинга как на С++, так и на C#. Также имеется возможность визуального программирования;

Поддержка .NET 8 и C# 12, что означает возможность не только использовать новейшие функции языка, но и получить прирост в производительности за счёт значительных улучшений среды выполнения. Для более подробного ознакомления с этими нововведениями можете обратиться к нашей статье. Также хочется отметить, что на момент написания статьи, в Unity поддержка этих версий отсутствует;

Если вам показалось, что данный движок больше ориентирован на C# разработку, нежели на C++, то вы ошибаетесь. Напротив, ядро движка реализовано на С++, что позволяет соответствующим разработчикам использовать его API напрямую. Это даёт преимущество в производительности, а также возможность использовать некоторые дополнительные API;

Наличие весьма солидного для Open Source движка набора инструментов, со списком которых можно ознакомиться на отдельной странице движка;

Абсолютно весь функционал движка можно использовать бесплатно, если доход от ваших проектов не превышает $250000 за квартал. При преодолении этого порога стоимость движка будет равна 4% прибыли от ваших проектов, реализованных на его основе.

Теперь, когда мы немного познакомились с движком, почему бы не узнать, что думает анализатор PVS-Studio о его исходном коде? Далее мы рассмотрим потенциальные проблемы в C# и C++ коде самой актуальной на момент написания статьи версии движка — 1.8.6512.2. Готовы? Тогда поехали!

Избыточные проверки условий

Case 1

public override string TypeDescription { get { // Translate asset type name var typeName = TypeName; string[] typeNamespaces = typeName.Split('.'); if ( typeNamespaces.Length != 0 && typeNamespaces.Length != 0) // <= { typeName = Utilities.Utils .GetPropertyNameUI( typeNamespaces[typeNamespaces.Length - 1]); } return typeName; } }

Предупреждение PVS-Studio:

V3001 There are identical sub-expressions 'typeNamespaces.Length != 0' to the left and to the right of the '&&' operator. AssetItem.cs: 83.

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

typeNamespaces[typeNamespaces.Length - 1].Length != 0

Вот ещё один аналогичный случай, найденный в проекте.

Case 2

public override bool OnMouseDown(Float2 location, MouseButton button) { .... if (_rightMouseDown || (_middleMouseDown && _middleMouseDown)) // <= { // Start navigating StartMouseCapture(); Focus(); return true; } .... }

Предупреждение PVS-Studio:

V3001 There are identical sub-expressions '_middleMouseDown' to the left and to the right of the '&&' operator. VisjectSurface.Input.cs: 495.

Ошибка из-за невнимательного copy-paste

partial class Window { .... public void Show() .... public void Hide() .... } public class ContextMenuBase : ContainerControl { private Window _window; .... public void Show() // <= { _window.Show(); } public void Hide() // <= { _window.Show(); } public void Minimize() { _window.Minimize(); } }

Предупреждение PVS-Studio:

V3013 It is odd that the body of 'Show' function is fully equivalent to the body of 'Hide' function (70, line 78). WindowRootControl.cs: 70, 78.

Типичная ошибка из-за невнимательности при использовании Copy-Paste. В методе Hide вызывается метод _window.Show вместо _window.Hide.

Выход за границы массива

public Matrix2x2(float[] values) { .... if (values.Length != 4) throw new ArgumentOutOfRangeException(....); M11 = values[0]; M12 = values[1]; M21 = values[3]; M22 = values[4]; // <= }

Предупреждение PVS-Studio:

V3106 Possibly index is out of bound. The '4' index is pointing beyond 'values' bound. Matrix2x2.cs: 98.

Из условия видно, что в массиве values может быть только строго четыре элемента. Так как индексация элементов начинается с 0, индекс самого последнего элемента массива будет равен 3. Однако в последнем присваивании выполняется обращение по индексу 4, что непременно приведёт к исключению.

Недостижимый код

Case 1

public void SetMemberValue(object instance, object value) { .... if (type.IsEnum) value = Convert.ChangeType(value, Enum.GetUnderlyingType(type.Type)); else if (type.Type == typeof(byte)) value = Convert.ToByte(value); else if (type.Type == typeof(sbyte)) value = Convert.ToSByte(value); else if (type.Type == typeof(short)) value = Convert.ToInt16(value); else if (type.Type == typeof(int)) // <= value = Convert.ToInt32(value); else if (type.Type == typeof(long)) value = Convert.ToInt64(value); else if (type.Type == typeof(int)) // <= value = Convert.ToUInt16(value); .... }

Предупреждение PVS-Studio:

V3003. The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 78, 82. MemberComparison.cs: 78, 82.

Анализатор обнаружил два одинаковых условия внутри связанных else if. В случае, если type.Type будет иметь значение int, код выполнится только в теле первого из них, а код в теле второго будет недостижим. Так как в теле второго рассматриваемого else if выполняется конвертация к типу UInt16, логичным решением была бы замена соответствующего условия на следующее:

type.Type == typeof(ushort)

Case 2

private void UpdateDragPositioning(....) { if (....) _dragOverMode = DragItemPositioning.Above; else if (....) _dragOverMode = DragItemPositioning.Below; else _dragOverMode = DragItemPositioning.At; // Update DraggedOverNode var tree = ParentTree; if (_dragOverMode == DragItemPositioning.None) // <= { if (tree != null && tree.DraggedOverNode == this) tree.DraggedOverNode = null; } else if (tree != null) tree.DraggedOverNode = this; }

Предупреждение PVS-Studio:

V3022 Expression '_dragOverMode == DragItemPositioning.None' is always false. TreeNode.cs: 566.

Анализатор обнаружил всегда ложное if-условие, из-за чего код внутри then-ветви никогда не будет выполнен.

Обратите внимание, что в результате выполнения первой условной конструкции поле _dragOverMode всегда получает новое значение, отличное от DragItemPositioning.None. Из-за этого следующий if становится бессмысленным.

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

private void UpdateDragPositioning(....) { // Update DraggedOverNode var tree = ParentTree; if (_dragOverMode == DragItemPositioning.None) .... if (....) _dragOverMode = DragItemPositioning.Above; else if (....) _dragOverMode = DragItemPositioning.Below; else _dragOverMode = DragItemPositioning.At; }

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

Проблема с синхронизацией потоков из-за оптимизаций компилятора

protected Window _window; .... public DialogResult ShowDialog(Window parentWindow) { // Show window Show(parentWindow); // Set wait flag Interlocked.Increment(ref _isWaitingForDialog); // Wait for the closing do { Thread.Sleep(1); } while (_window); // <= // Clear wait flag Interlocked.Decrement(ref _isWaitingForDialog); return _result; }

Предупреждение PVS-Studio:

V3032 Waiting on this expression is unreliable, as compiler may optimize some of the variables. Use volatile variable(s) or synchronization primitives to avoid this. Dialog.cs: 108.

Этот код содержит потенциальную проблему, которую невозможно отловить при работе с Debug конфигурацией. В чём же секрет неуловимости этого зверя? Дело в том, что возникнуть эта проблема может только при сборке Release версии вследствие оптимизации кода компилятором. Помимо этого, её возникновение зависит и от других факторов, таких как используемая версия .NET и количество процессоров в системе.

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

Потенциальное разыменование nullptr

Case 1

void Append(const T* data, int32 length) { .... auto prev = Base::_data; .... Base::_data = (T*)Allocator::Allocate(Base::_length * sizeof(T)); Platform::MemoryCopy(Base::_data, prev, prevLength * sizeof(T)); // <= .... if (_isAllocated && prev) .... }

Предупреждение PVS-Studio:

V595 The 'prev' pointer was utilized before it was verified against nullptr. Check lines: 'PlatformBase.h: 178', 'DataContainer.h: 339', 'DataContainer.h: 342'. DataContainer.h 339.

Анализатор указывает на использование prev в качестве второго аргумента метода MemoryCopy. Потенциальная проблема заключается в том, что prev может иметь значение nullptr, на что указывает соответствующая проверка. Но действительно ли передача nullptr в MemoryCopy опасна? Вдруг этот случай обрабатывается внутри самого метода? Чтобы ответить на эти вопросы, взглянем на реализацию MemoryCopy:

FORCE_INLINE static void MemoryCopy(void* dst, const void* src, uint64 size) { memcpy(dst, src, static_cast<size_t>(size)); }

Теперь видно, что значение второго параметра напрямую передаётся в функцию memcpy без какой-либо предварительной проверки на неравенство nullptr, что создаёт риск возникновения неопределённого поведения.

Вдумчивый читатель может не согласиться с этим, возразив: "Здесь же дополнительно передаётся size (количество байт, которое необходимо скопировать), а значит, если этот параметр равен 0, то и копирование выполнено не будет".

Увы, но это не совсем так. Документация на функцию memcpy ясно даёт понять, что в неё нельзя передавать невалидные указатели. Даже если количество копируемых данных равно нулю:

"If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero"

Case 2

void Variant::SetAsset(Asset* asset) { if (Type.Type != VariantType::Asset) SetType(VariantType(VariantType::Asset)); if (AsAsset) { asset->OnUnloaded.Unbind<Variant, // <= &Variant::OnAssetUnloaded>(this); asset->RemoveReference(); // <= } AsAsset = asset; if (asset) { asset->AddReference(); asset->OnUnloaded.Bind<Variant, &Variant::OnAssetUnloaded>(this); } }

Предупреждение PVS-Studio:

V595 The 'asset' pointer was utilized before it was verified against nullptr. Check lines: 2706, 2709. Variant.cpp: 2706, 2709.

Весьма странный код. Кажется, что в теле первого оператора if должно обрабатываться старое значение AsAsset перед его заменой на новое, на что указывает условие этого if.

Также на ошибку в этом месте указывает условие второго if, в котором выполняется проверка параметра asset на неравенство nullptr. Если ожидается, что asset может быть равен nullptr, значит его разыменование внутри первого if может привести к неопределённому поведению.

Наиболее логичным исправлением в данном случае является замена asset на AsAsset внутри первого if:

void Variant::SetAsset(Asset* asset) { .... if (AsAsset) { AsAsset ->OnUnloaded.Unbind<Variant, // <= &Variant::OnAssetUnloaded>(this); AsAsset ->RemoveReference(); // <= } AsAsset = asset; if (asset) { asset->AddReference(); asset->OnUnloaded.Bind<Variant, &Variant::OnAssetUnloaded>(this); } }

Потенциальная ошибка при побитовом сдвиге

void StringUtils::ConvertUTF162UTF8(....) { Array<uint32> unicode; .... const uint32 uni = unicode[j]; const uint32 count = uni <= 0x7FU ? 1 : uni <= 0x7FFU ? 2 : uni <= 0xFFFFU ? 3 : uni <= 0x1FFFFFU ? 4 : uni <= 0x3FFFFFFU ? 5 : uni <= 0x7FFFFFFFU ? 6 : 7; // <= to[i++] = (char)(count <= 1 ? (byte)uni : ((byte(0xFFU) << (8 - count)) | byte(uni >> (6 * (count - 1))))); // <= .... }

Предупреждение PVS-Studio:

V610 Undefined behavior. Check the shift operator '>>'. The right operand ('(6 * (count - 1))' = [6..36]) is greater than or equal to the length in bits of the promoted left operand. StringUtilsBase.cpp. 253.

Анализатор обращает внимание на возможность получить неожиданный результат при битовом смещении в выражении uni >> (6 * (count - 1)). Произойти это может из-за того, что uni имеет тип int32. Смещение этого значения на 32 бита и более вправо может привести к неопределённому поведению.

Анализатор вычислил, что наибольший возможный шаг при побитовом сдвиге переменной uni равен 36. Как он это определил? Обратите внимание на тернарный оператор, используемый для инициализации переменной count:

const uint32 count = uni <= 0x7FU ? 1 : uni <= 0x7FFU ? 2 : uni <= 0xFFFFU ? 3 : uni <= 0x1FFFFFU ? 4 : uni <= 0x3FFFFFFU ? 5 : uni <= 0x7FFFFFFFU ? 6 : 7;

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

6 * (count - 1) = 6 * (7 – 1) = 36

Что же, в этот раз анализатор оказался прав. Дело закрыто!

На этом статья завершается, надеюсь, она вам понравилась :)

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

Статьи с разбором проблем в C++ коде:

От винта! Смотрим движок War Thunder и говорим с его создателями;

Проверка игрового движка qdEngine, часть первая: топ 10 предупреждений PVS-Studio;

Проверка игрового движка qdEngine, часть вторая: упрощение C++ кода;

Проверка игрового движка qdEngine, часть третья: дополнительная десятка багов.

Герои Кода и Магии: анализ игрового движка VCMI

Статьи с разбором проблем в C# коде:

Повторная проверка Unity статическим анализатором PVS-Studio;

Разбор ошибок в игровом движке Stride;

Игра с null: проверка MonoGame статическим анализатором PVS-Studio.

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

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

До встречи в следующих статьях!

#cpp #cplusplus #csharp #pvsstudio #staticanalysis #flaxengine

7 комментариев

Реил

14 авг

Ответить

MGDS Studio

15 авг

'''Если вам показалось, что данный движок больше ориентирован на C# разработку, нежели на C++, то вы ошибаетесь. Напротив, ядро движка реализовано на С++, что позволяет соответствующим разработчикам использовать его API напрямую..'''

А бывает иначе в коммерческих движках?