Продолжаем изучать Tizen: C# компоненты оказались высокого качества

в 9:19, , рубрики: .net, C#, open source, pvs-studio, static code analysis, tizen, Visual Studio, Блог компании PVS-Studio, Разработка под Tizen

Picture 1

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

Введение

Мой коллега Андрей Карпов не так давно публиковал две эпические статьи, посвященные анализу кода проекта Tizen, написанного на языках Си и Си++:

  1. 27000 ошибок в операционной системе Tizen
  2. Поговорим о микрооптимизациях на примере кода Tizen

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

Что я проверял

Исходный код Tizen доступен для загрузки по ссылке. Репозиторий содержит около 1 000 проектов, каждый из которых состоит из архива с исходным кодом, а также вспомогательных файлов. По именам архивов или описанию не всегда возможно понять, что же находится внутри. Поэтому потребовалась загрузка, распаковка и изучение архивов для всего репозитория.

В предыдущей статье я приводил общее число файлов исходного кода C# (4 929, исключая *.Designer.cs) и строк кода в них (около 691 000), содержащееся в проекте Tizen. Сейчас нам потребуется более детальный анализ. Для начала попробуем найти файлы с расширением .sln или .csproj. Наличие таких файлов позволит проводить анализ в IDE Visual Studio, что значительно упростит работу.

Итак, в ходе поиска было найдено 227 решений (*.sln) и 166 проектов C# (*.csproj). Из файлов решений я выбрал те, которые содержали в своем составе проекты C#. Таких решений оказалось всего 3:

  • Tizen.Xamarin.Forms.Extension.sln
  • Xamarin.Forms.Tizen.sln
  • Xamarin.Forms.sln

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

Далее, исключив файлы проектов (*.csproj), входящие в состав этих решений, я получил 107 проектов C#, которые не были связаны ни с какими решениями. Практически все они находятся в папках верхнего уровня с именами вида «csapi-*». Исключив из этого числа 11 тестовых проектов, а также 9 проектов, которые имели неподдерживаемый Visual Studio формат, я получил в остатке 87 проектов. Каждый из них я проверял отдельно.

Для чистоты исследования я решил отделить результаты, полученные для внутренних C# компонентов Tizen (те самые 87 проектов), от результатов проверки компонентов на базе Xamarin.Forms. Сначала я не хотел учитывать Xamarin.Forms, но, поразмыслив, пришел к выводу, что раз уж Tizen использует этот компонент для своих целей, то и ошибки Xamarin.Forms могут оказывать влияние на Tizen.

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

Picture 6

Результаты проверки

Внутренние C# компоненты Tizen

В ходе проверки этой части проекта Tizen анализатор PVS-Studio выдал 356 предупреждений, из них 18 — уровня достоверности High, 157 — уровня достоверности Medium и 181 — уровня достоверности Low. Было проанализировано около 325 000 строк кода.

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

Предупреждение PVS-Studio: V3008 The '_scanData' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 138, 137. Tizen.Network.Bluetooth BluetoothLeAdapter.cs 138
CWE-563. Assignment to Variable without Use ('Unused Variable')

internal BluetoothLeDevice(BluetoothLeScanData scanData)
{
  _scanData = new BluetoothLeScanData ();
  _scanData = scanData;
  ....
}

Полю _scanData дважды присваивают значение. Выглядит очень подозрительно. На всякий случай взглянем на объявление класса BluetoothLeScanData и его конструктор. Возможно, вызов конструктора содержит какие-то дополнительные действия. Класс небольшой, поэтому приведу его целиком, немного отформатировав оригинальный код:

internal class BluetoothLeScanData
{
  internal string RemoteAddress { get; set; }
  internal BluetoothLeDeviceAddressType AddressType { get; set; }
  internal int Rssi { get; set; }
  internal int AdvDataLength { get; set; }
  internal byte[] AdvData { get; set; }
  internal int ScanDataLength { get; set; }
  internal byte[] ScanData { get; set; }
}

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

Предупреждение PVS-Studio: V3009 It's odd that this method always returns one and the same value of '0'. Tizen.Applications.WidgetApplication WidgetType.cs 47
CWE-393. Return of Wrong Status Code

private int OnCreate(....)
{
  WidgetBase b = Activator.CreateInstance(ClassType) as WidgetBase;
  ....  
  if (b == null)
    return 0;
  ....  
  return 0;
}

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

private int OnCreate(....)
{
  WidgetBase b = Activator.CreateInstance(ClassType) as WidgetBase;
  ....  
  if (b == null)
    return 0;
  ....  
  return 1;
}

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

  • V3022 Expression '!LeftBoundIsForward' is always false. clipper_library clipper.cs 838
  • V3022 Expression '!LeftBoundIsForward' is always true. clipper_library clipper.cs 863

CWE-570/CWE-571 Expression is Always False/True

private TEdge ProcessBound(TEdge E, bool LeftBoundIsForward)
{
  ....
  if (LeftBoundIsForward)
  {
    ....
    if (!LeftBoundIsForward) Result = Horz.Prev;
    ....
  }
  else
  {
    ....
    if (!LeftBoundIsForward) Result = Horz.Next;
    ....
  }
  ....
}

Данный фрагмент кода содержит сразу 2 однотипные ошибочные проверки. При этом в первом случае переменная Result никогда не получит значения Horz.Prev, а во втором случае та же переменная Result всегда получит значение Horz.Next. Автору необходимо внимательно изучить данный код и самостоятельно исправить ошибку.

Предупреждение PVS-Studio: V3022 Expression 'e.OutIdx >= 0' is always true. clipper_library clipper.cs 3144
CWE-571 Expression is Always True

private void DoMaxima(TEdge e)
{
  ....
  if(....)
  {
    ....
  } else if( e.OutIdx >= 0 && eMaxPair.OutIdx >= 0 )
  {
    if (e.OutIdx >= 0) AddLocalMaxPoly(e, eMaxPair, e.Top);
    ....
  }
  ....
}

Ещё один фрагмент кода с ошибочной проверкой. Возможно, если бы в условии e.OutIdx >= 0 && eMaxPair.OutIdx >= 0 был использован оператор "||", то проверка e.OutIdx >= 0 во вложенном блоке if имела бы смысл. Сейчас это выглядит подозрительно.

Предупреждение PVS-Studio: V3110 Possible infinite recursion inside 'InsertBefore' method. ElmSharp Toolbar.cs 288
CWE-674 Uncontrolled Recursion

public ToolbarItem InsertBefore(ToolbarItem before, string label)
{
  return InsertBefore(before, label);
}

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

public ToolbarItem InsertBefore(ToolbarItem before, string label,
  string icon)
{
  ....
}

Пожалуй, это все интересные ошибки данного раздела. Есть ещё несколько подозрительных участков кода, но я не буду на них останавливаться. Код от Samsung Electronics, написанный на C#, демонстрирует хорошее качество. Почему я так уверен, что проверенный код имеет авторство Samsung? Да потому, что каждый из проверенных файлов содержал в своём заголовке комментарий вида «Copyright © 2016 Samsung Electronics Co., Ltd All Rights Reserved».

Компоненты Tizen на базе Xamarin.Forms

Используемая в Tizen надстройка над Xamarin.Forms содержит около 242 000 строк кода. В ходе её проверки анализатор PVS-Studio выдал 163 предупреждения, из них 10 — уровня достоверности High, 46 — уровня достоверности Medium и 107 — уровня достоверности Low (не рассматривались).

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

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

Предупреждение PVS-Studio: V3095 The 'context' object was used before it was verified against null. Check lines: 16, 18. Xamarin.Forms.Xaml XamlServiceProvider.cs 16
CWE-476 NULL Pointer Dereference

internal XamlServiceProvider(INode node, HydratationContext context)
{
  ....
  if (node != null && node.Parent != null
    && context.Values.TryGetValue(node.Parent,  // <=
    out targetObject))
    IProvideValueTarget = new XamlValueTargetProvider(....);
  if (context != null)  // <=
    IRootObjectProvider =
    new XamlRootObjectProvider(context.RootElement);
  ....
}

Переменную context сначала используют, а только затем проверяют на равенство null.

Предупреждение PVS-Studio: V3095 The 'type' object was used before it was verified against null. Check lines: 147, 149. Xamarin.Forms.Xaml ExpandMarkupsVisitor.cs 147
CWE-476 NULL Pointer Dereference

public INode Parse(....)
{
  ....
  var xmltype = new XmlType(namespaceuri, type.Name, null);  // <=
  
  if (type == null)
    throw new NotSupportedException();
  ....
}

Еще один пример возможного выброса исключения NullReferenceException. Переменную type используют для создания экземпляра класса XmlType, а уже затем проверяют на равенство null.

Другие аналогичные ошибки:

  • V3095 The 'e.NewElement' object was used before it was verified against null. Check lines: 32, 46. Xamarin.Forms.Platform.Tizen MasterDetailPageRenderer.cs 32
  • V3095 The 'e.NewItems' object was used before it was verified against null. Check lines: 557, 567. Xamarin.Forms.Core Element.cs 557
  • V3095 The 'e.OldItems' object was used before it was verified against null. Check lines: 561, 574. Xamarin.Forms.Core Element.cs 561
  • V3095 The 'part' object was used before it was verified against null. Check lines: 135, 156. Xamarin.Forms.Core BindingExpression.cs 135

Предупреждение PVS-Studio: V3125 The 'e.NewItems' object was used after it was verified against null. Check lines: 999, 986. Xamarin.Forms.Core TemplatedItemsList.cs 999
CWE-476 NULL Pointer Dereference

void OnProxyCollectionChanged(....)
{
  ....
  if (e.NewStartingIndex >= 0 && e.NewItems != null)  // <=
    maxindex = Math.Max(maxindex, e.NewStartingIndex +
      e.NewItems.Count);
  ....
  for (int i = e.NewStartingIndex; i < _templatedObjects.Count; i++)
    SetIndex(_templatedObjects[i], i + e.NewItems.Count);  // <=
  ....
}

А здесь — обратная ситуация. Переменную e.NewItems проверяют на равенство null перед первым использованием. Однако при повторном использовании это сделать забывают.

Статистика

Как писал мой коллега Андрей Карпов в одной из предыдущих статей, анализатор PVS-Studio выявляет около 0.4 ошибки на 1000 строк C/C++ кода проекта Tizen. Давайте подсчитаем, что у нас получается для C# кода.

Всего было проверено около 567 000 строк кода на языке C#.

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

Получается, что анализатор обнаруживает 0.02 ошибки на 1000 строк кода. Или, другими словами, он находит 1 ошибку на 50 000 строк кода. Маловато.

Picture 11

Приходится признать, что данном случае анализатор не смог продемонстрировать свою полезность. Почему так получилось — сказать сложно. При проверке других открытых проектов анализатор часто демонстрировал хорошие результаты. Например, при проверке открытых компонентов Unity3D плотность обнаруживаемых ошибок составляла 0.5 ошибки на 1000 строк кода. Т.е. показатель был в 25 раз лучше.

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

Выводы

Итак, результат проверки оказался не таким, как я ожидал для такого объема кода. Скажу честно: я предполагал найти как минимум несколько сотен ошибок, но нашлось около пятнадцати.

Однако важно то, что анализатор PVS-Studio справился с задачей по анализу C# кода проекта Tizen. А значит, может быть полезен если не сейчас, то в дальнейшем, когда в Tizen будут появляться новые компоненты, написанные с использованием языка C#. Подтверждением потенциальной пользы является огромное количество ошибок, которые анализатор уже обнаружил в других открытых проектах (см. список статей).

Более того, как мы не устаём повторять, сценарий единичных проверок с использованием анализатора не оптимален – ошибки уже заложены в систему контроля версий, что, согласитесь, плохо. Гораздо эффективнее пользоваться статическим анализатором регулярно, что позволит исправлять ошибки на этапе написания кода, до момента попадания в систему контроля версий, ведь в таком случае стоимость и сложность их исправления куда ниже.

Picture 17

Загрузить и попробовать PVS-Studio: http://www.viva64.com/ru/pvs-studio/

Дополнительные ссылки

  1. Эксперимент по поиску ошибок в коде C# компонентов Tizen
  2. Microsoft открыла исходники Xamarin.Forms. Мы не могли упустить шанс проверить их с помощью PVS-Studio
  3. Команда PVS-Studio готова поработать над проектом Tizen (открытое письмо)
  4. Предоставляем анализатор PVS-Studio экспертам безопасности
  5. Как PVS-Studio может помочь в поиске уязвимостей?
  6. 27000 ошибок в операционной системе Tizen
  7. Поговорим о микрооптимизациях на примере кода Tizen

Автор: n0mo

Источник

* - обязательные к заполнению поля

Главная   |  Архив новостей  |   Android  |   Google  |   Apple  |   Microsoft  |   Информационная безопасность  |   Веб – разработка
Публикации RSS  |  Комментарии RSS
https://ajax.googleapis.com/ajax/libs/jquery/3.4.1/jquery.min.js