Для получения триального ключа
заполните форму ниже
Team License (базовая версия)
Enterprise License (расширенная версия)
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Запросите информацию о ценах
Новая лицензия
Продление лицензии
--Выберите валюту--
USD
EUR
GBP
RUB
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Для получения лицензии для вашего открытого
проекта заполните, пожалуйста, эту форму
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Мне интересно попробовать плагин на:
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

** На сайте установлена reCAPTCHA и применяются
Политика конфиденциальности и Условия использования Google.
Ваше сообщение отправлено.

Мы ответим вам на


Если вы так и не получили ответ, пожалуйста, проверьте папку
Spam/Junk и нажмите на письме кнопку "Не спам".
Так Вы не пропустите ответы от нашей команды.

>
>
>
Анализируем ошибки в открытых компонент…

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

23 Авг 2016

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

0423_Unity3D_ru/image1.png

Введение

Мы решили проверить все компоненты, библиотеки и демки, написанные на языке C#, чей исходный код предоставлен в официальном репозитории разработчиков Unity3D:

  • UI System - система для реализации графического интерфейса.
  • Networking - система для реализации мультиплеера.
  • MemoryProfiler - система профилирования используемых ресурсов.
  • XcodeAPI - компонент для взаимодействия со средой разработки Xcode.
  • PlayableGraphVisualizer - система визуализации процесса выполнения проекта.
  • UnityTestTools - утилиты тестирования Unity3D (без Unit тестов).
  • AssetBundleDemo - проект, содержащий исходники AssetBundleServer'а и демонстрирующий использование AssetBundle системы.
  • AudioDemos - проекты, демонстрирующие использование аудио системы.
  • NativeAudioPlugins - аудио плагины (нас интересует только код, демонстрирующий их использование).
  • GraphicsDemos - проекты, демонстрирующие использование графической системы.

Было бы очень интересно взглянуть на исходники непосредственно ядра движка, но кроме разработчиков ни у кого такой возможности пока нет. Поэтому сегодня на нашем операционном столе лишь малая часть исходных кодов движка, которые мы можем проверить. Наиболее интересными проектами для нас являются: новая UI система, предназначенная для реализации более гибкого графического интерфейса относительно старого топорного GUI, и сетевая библиотека, которая верой и правдой нам служила до появления UNet.

Также не меньшего интереса заслуживает MemoryProfiler, как мощный и гибкий инструмент профилирования ресурсов и нагрузок.

Найденные ошибки и подозрительные места

Все предупреждения, выданные анализатором, можно разделить на 3 уровня:

  • Высокий - наиболее вероятная ошибка.
  • Средний - возможная ошибка или опечатка.
  • Низкий - предупреждение о маловероятно возможной ошибке или опечатке.

Мы будем рассматривать только высокий и средний уровни.

В таблице ниже представлен список проверенных проектов и итоговый результат проверки по всем проектам. Столбцы "Название проекта" и "Количество строк кода", думаю, всем понятны и не должны вызывать вопросов, а вот назначение столбца "Срабатывания анализатора" стоит объяснить. Он содержит в себе информацию о количестве срабатываний анализатора. Позитивными срабатываниями считаются те, которые прямо или косвенно указывают на ошибки или опечатки в коде. Ложные срабатывания - ложные сообщения анализаторы, которые указывают на корректные участки кода, подозревая наличие в них ошибки или опечатки. Как уже и говорилось ранее - все срабатывания разделены на 3 уровня. Мы будем рассматривать только высокий и средний, так как низкий уровень, в основном, содержит информационные сообщения или маловероятные ошибки.

0423_Unity3D_ru/image2.png

По итогам проверки 10 проектов было получено 16 предупреждений высокого уровня, 75% из которых верно указали на проблемные места в коде, и 18 срабатываний среднего уровня, 39% из которых верно указали на проблемные места. Качество кода следует признать высоким, так как анализатор находит в среднем только одну ошибку на 2000 строк кода. Это хороший результат.

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

Ошибочное регулярное выражение

V3057 Invalid regular expression patern in constructor. Inspect the first argument. AssetBundleDemo ExecuteInternalMono.cs 48

private static readonly Regex UnsafeCharsWindows = 
  new Regex("[^A-Za-z0-9\\_\\-\\.\\:\\,\\/\\@\\\\]"); // <=

При попытке создания экземпляра класса Regex с данным паттерном мы получим исключение System.ArgumentException с сообщением:

parsing \"[^A-Za-z0-9\\_\\-\\.\\:\\,\\/\\@\\]\" -
Unrecognized escape sequence '\\_'.

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

Возможно обращение к объекту с нулевой ссылкой

V3080 Possible null dereference. Consider inspecting 't.staticFieldBytes'. MemoryProfiller CrawledDataUnpacker.cs 20

.... = packedSnapshot.typeDescriptions.Where(t => 
  t.staticFieldBytes != null & t.staticFieldBytes.Length > 0 // <=
)....

После проверки объекта на null происходит обращение к нему. При этом обращение происходит независимо от результата проверки. Это может привести к возникновению исключения NullReferenceException. Вероятнее всего программист планировал использовать оператор условного и &&, но вследствие опечатки используется оператор логического и &.

Обращение к объекту перед проверкой его на null

V3095 The 'uv2.gameObject' object was used before it was verified against null. Check lines: 1719, 1731. UnityEngine.Networking NetworkServer.cs 1719

if (uv2.gameObject.hideFlags == HideFlags.NotEditable || 
    uv2.gameObject.hideFlags == HideFlags.HideAndDontSave)
  continue;
....
if (uv2.gameObject == null)
  continue;

Сначала происходит обращение к объекту, и только потом его проверка на null. Вероятнее всего, если ссылка на объект окажется равной null, то мы получим исключение NullReferenceException, так и не дойдя до проверки.

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

  • V3095 The 'm_HorizontalScrollbarRect' object was used before it was verified against null. Check lines: 214, 220. UnityEngine.UI ScrollRect.cs 214
  • V3095 The 'm_VerticalScrollbarRect' object was used before it was verified against null. Check lines: 215, 221. UnityEngine.UI ScrollRect.cs 215

Ранее уже встречается оператор if с таким же условием, содержащий в then части безусловный оператор return

Очень интересное срабатывание, показывающее всю силу копипасты, классический пример опечатки.

V3021 There are two 'if' statements with identical conditional expressions. The first 'if' statement contains method return. This means that the second 'if' statement is senseless UnityEngine.UI StencilMaterial.cs 64

if (!baseMat.HasProperty("_StencilReadMask"))
{
  Debug.LogWarning(".... _StencilReadMask property", baseMat);
  return baseMat;
}
if (!baseMat.HasProperty("_StencilReadMask")) // <=
{
  Debug.LogWarning(".... _StencilWriteMask property", baseMat);
  return baseMat;
}

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

Исходя из этой опечатки, можно сказать, что вторая проверка должна иметь вид:

if (!baseMat.HasProperty("_StencilWriteMask"))

Создание экземпляра класса исключения без дальнейшего использования

V3006 The object was created but it is not being used. The 'throw' keyword could be missing: throw new ApplicationException(FOO). AssetBundleDemo AssetBundleManager.cs 446

if (bundleBaseDownloadingURL.ToLower().StartsWith("odr://"))
{
#if ENABLE_IOS_ON_DEMAND_RESOURCES
  Log(LogType.Info, "Requesting bundle " + ....);
  m_InProgressOperations.Add(
    new AssetBundleDownloadFromODROperation(assetBundleName)
  );
#else
  new ApplicationException("Can't load bundle " + ....); // <=
#endif
}

Создается класс ApplicationException, но никак не используется. Вероятнее всего, программист хотел выбросить исключение, но забыл добавить оператор throw перед созданным исключением.

Неиспользуемые аргументы при форматировании строки

Как известно, при форматировании строк количество выражений типа {N} должно соответствовать количеству передаваемых аргументов.

V3025 Incorrect format. A different number of format items is expected while calling 'WriteLine' function. Arguments not used: port. AssetBundleDemo AssetBundleServer.cs 59

Console.WriteLine("Starting up asset bundle server.", port); // <=
Console.WriteLine("Port: {0}", port);
Console.WriteLine("Directory: {0}", basePath);

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

Цикл, который может превратиться в вечный при определенных условиях

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. AssetBundleDemo AssetBundleServer.cs 16

Process masterProcess = Process.GetProcessById((int)processID);
while (masterProcess == null || !masterProcess.HasExited) // <=
{
  Thread.Sleep(1000);
}

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

while (true) {
  Process masterProcess = Process.GetProcessById((int)processID);
  if (masterProcess == null || masterProcess.HasExited) // <=
    break;
  Thread.Sleep(1000);
}

Небезопасная инициация события

Анализатор обнаружил потенциально небезопасный вызов обработчика события (event). Возможно возникновение исключения NullReferenceException.

V3083 Unsafe invocation of event 'unload', NullReferenceException is possible. Consider assigning event to a local variable before invoking it. AssetBundleDemo AssetBundleManager.cs 47

internal void OnUnload()
{
  m_AssetBundle.Unload(false);
  if (unload != null)
    unload(); // <=
}

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

Однако представим, что у события есть один подписчик. И в момент между проверкой на null и непосредственными вызовом обработчика события существует вероятность, что будет произведена отписка от события, например, в другом потоке. Чтобы обезопасить себя в данной ситуации можно сделать так:

internal void OnUnload()
{
  m_AssetBundle.Unload(false);
  unload?.Invoke(); // <=
}

Таким образом, проверка события на null и вызов его обработчика будут выполнены в виде одной команды, что обеспечит безопасный вызов события.

Часть логического выражения всегда истинна или ложна

V3063 A part of conditional expression is always false: connId < 0. UnityEngine.Networking ConnectionArray.cs 59

public NetworkConnection Get(int connId)
{
  if (connId < 0)
  {
    return m_LocalConnections[Mathf.Abs(connId) - 1];
  }

  if (connId < 0 || connId > m_Connections.Count) // <=
  {
    ...
    return null;
  }

  return m_Connections[connId];
}

Выражение connId < 0 во второй проверке функции get всегда будет равно false, так как, используя это выражение в первой проверке, всегда производится выход из функции. Исходя из этого, во второй проверке это выражение не несет никакой смысловой и функциональной нагрузки.

Также была найдена еще одна похожая ошибка.

public bool isServer
{
  get
  {
    if (!m_IsServer)
    {
        return false;
    }

    return NetworkServer.active && m_IsServer; // <=
  }
}

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

public bool isServer
{
  get
  {
    return m_IsServer && NetworkServer.active;
  }
}

Помимо представленных выше двух примеров, в проектах были найдены еще 6 аналогичных ошибок:

  • V3022 Expression 'm_Peers == null' is always false. UnityEngine.Networking NetworkMigrationManager.cs 710
  • V3022 Expression 'uv2.gameObject == null' is always false. UnityEngine.Networking NetworkServer.cs 1731
  • V3022 Expression 'newEnterTarget != null' is always true. UnityEngine.UI BaseInputModule.cs 147
  • V3022 Expression 'pointerEvent.pointerDrag != null' is always false. UnityEngine.UI TouchInputModule.cs 227
  • V3063 A part of conditional expression is always true: currentTest != null. UnityTestTools TestRunner.cs 237
  • V3063 A part of conditional expression is always false: connId < 0. UnityEngine.Networking ConnectionArray.cs 86

Заключение

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

В свою очередь, вы также можете проверить свой, или любой другой проект, написанный на языке C/C++/C#, с помощью данного статического анализатора.

Спасибо всем за внимание! Желаю вам безбажных программ.

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

Дата: 21 Ноя 2018

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

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

Дата: 22 Дек 2018

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

В канун празднования нового 2019 года команда PVS-Studio решила сделать приятный подарок всем контрибьюторам open-source проектов, хостящихся на GitHub, GitLab или Bitbucket. Им предоставляется возмо…
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

После большой статьи про проверку операционной системы Tizen мне было задано много вопросов о проценте ложных срабатываний и о плотности ошибок (сколько ошибок PVS-Studio выявляет на 1000 строк кода)…
PVS-Studio для Java

Дата: 17 Янв 2019

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

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

Дата: 14 Апр 2016

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

Вы угадали, ответ - "42". Здесь приводится 42 рекомендации по программированию, которые помогут избежать множества ошибок, сэкономить время и нервы. Автором рекомендаций выступает Андрей Карпов - тех…
Как и почему статические анализаторы борются с ложными срабатываниями

Дата: 20 Мар 2017

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

В своей предыдущей статье я писал, что мне не нравится подход, при котором статические анализаторы кода оцениваются с помощью синтетических тестов. В статье приводился пример, воспринимаемый анализат…
Любите статический анализ кода!

Дата: 16 Окт 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 30 Янв 2019

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

Время от времени нам задают вопрос, какую пользу в денежном эквиваленте получит компания от использования анализатора PVS-Studio. Мы решили оформить ответ в виде статьи и привести таблицы, которые по…
Статический анализ как часть процесса разработки Unreal Engine

Дата: 27 Июн 2017

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

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

Комментарии (0)

Следующие комментарии

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