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

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

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

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

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

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

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


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

>
>
>
Новогодняя проверка .NET Core Libraries…

Новогодняя проверка .NET Core Libraries (CoreFX)

28 Дек 2015

Около года назад Microsoft выложила в открытый доступ исходный код таких проектов, как CoreCLR и CoreFX. Последний проект до недавнего времени не был нам интересен, потому что написан на языке C#, а не C++. Но с выходом новой версии PVS-Studio 6.00, поддерживающей проекты и на языке программирования C#, я решил вернуться к CoreFX и написать статью.

0365_CoreFX_ru/image1.png

Введение

.NET Core это модульная реализация библиотек и среды выполнения, которая включает подмножество .NET Framework. .NET Core состоит из набора библиотек, называемых "CoreFX" и небольшой оптимизированной рабочей среды "CoreCLR".

.NET Core распространяется с открытым исходным кодом, который доступен на GitHub:

Это крупные продукты от Microsoft, содержащие качественный исходный код, но подозрительные участки кода всё равно можно найти.

О проверке CoreCLR можно прочитать в статье "PVS-Studio: 25 подозрительных фрагментов кода из CoreCLR".

Проект CoreFX, о котором подойдёт речь в статье, проверялся с помощью статического анализатора PVS-Studio 6.00, который теперь поддерживает и C#!

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

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

Наиболее опасные найденные места

V3027 The variable 'start.BaseMapping' was utilized in the logical expression before it was verified against null in the same logical expression. Mappings.cs 598

internal void SetSequence()
{
  if (TypeDesc.IsRoot)
      return;

  StructMapping start = this;

  // find first mapping that does not have the sequence set
  while (!start.BaseMapping.IsSequence &&          // <=
          start.BaseMapping != null    &&          // <=???
         !start.BaseMapping.TypeDesc.IsRoot)
      start = start.BaseMapping;
  ....
}

В коде присутствует серьёзная логическая ошибка! В теле цикла объект с именем 'start' изменяется на каждой итерации, и цикл выполняется, пока объект находится в определённом состоянии. НО проверка условия "start.BaseMapping != null" выполняется после обращения к "start.BaseMapping.IsSequence", а это может привести к разыменованию нулевой ссылки.

V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'comparand', 'comparedCredentialKey'. CredentialCache.cs 4007

public override bool Equals(object comparand)
{
  CredentialHostKey comparedCredentialKey =
                                  comparand as CredentialHostKey;

  if (comparand == null)
  {
    // This covers also the compared == null case
    return false;
  }

  bool equals = string.Equals(AuthenticationType,
        comparedCredentialKey.AuthenticationType, ....
  ....
}

В функцию может прийти объект любого типа или null. Если придёт null, этот случай будет обработан корректно. Если это будет объект такого типа, который не удастся преобразовать к типу "CredentialHostKey", то произойдёт ошибка при обращении к "comparedCredentialKey.AuthenticationType", т.к. переменная "comparedCredentialKey" может быть равна null.

Скорее всего, хотели написать так:

CredentialHostKey comparedCredentialKey =
                                  comparand as CredentialHostKey;
if (comparedCredentialKey == null)
{
  return false;
}

Аналогичное место в коде:

  • V3019 Possibly an incorrect variable is compared to null after type conversion using 'as' keyword. Check variables 'comparand', 'comparedCredentialKey'. CredentialCache.cs 497

V3008 The 'HResult' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 169, 166. WebSocketException.cs 169

private void SetErrorCodeOnError(int nativeError)
{
    if (!Succeeded(nativeError))
    {
        HResult = nativeError;
    }

    HResult = nativeError;  // <=???
}

Почему-то независимо от условия, переменная "HResult" всегда принимает одно и то же значение. Скорее всего функция должна быть реализована другим образом.

V3008 The 'ResPrec' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 1735, 1731. SQLDecimal.cs 1735

public static SqlDecimal operator /(SqlDecimal x, SqlDecimal y)
{
  int ResPrec;
  ....
  ResPrec = ResScale + x.m_bPrec + y.m_bPrec + 1;     // <=
  MinScale = Math.Min(ResScale, s_cNumeDivScaleMin);

  ResInteger = Math.Min(ResInteger, s_NUMERIC_MAX_PRECISION);
  ResPrec = ResInteger + ResScale;                    // <=

  if (ResPrec > s_NUMERIC_MAX_PRECISION)
      ResPrec = s_NUMERIC_MAX_PRECISION;
  ....
}

Очень подозрительно, что значение переменной "ResPrec" вычисляется по некой формуле, но потом просто перетирается другим значением.

V3020 An unconditional 'return' within a loop. Enumerable.cs 517

public override bool MoveNext()
{
  switch (state)
  {
    case 1:
      _enumerator = _source.GetEnumerator();
      state = 2;
      goto case 2;
    case 2:
      while (_enumerator.MoveNext())
      {
        current = _selector(_enumerator.Current);
        return true;
      }
      Dispose();
      break;
  }
  return false;
}

Странно, в теле цикла "while" выполняется выход из функции без какого-либо условия. Возможно, в коде присутствует ошибка.

Ещё один похожий цикл:

  • V3020 An unconditional 'return' within a loop. JsonDataContract.cs 128

V3008 The 'prefix' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 953, 952. XmlSerializationWriter.cs 953

protected void WriteAttribute(string localName, string ns, ....)
{
  ....
  string prefix = localName.Substring(0, colon);
  prefix = _w.LookupPrefix(ns);
  _w.WriteStartAttribute(prefix,
                         localName.Substring(colon + 1), ns);
  ....
}

В переменную 'prefix' сохраняется подстрока из 'localName' длинной "colon", потом это значение перетирается другим. Дальше по коду видно, что используется оставшаяся подстрока из 'localName', а первая часть была потеряна. Очень сомнительный код.

V3030 Recurring check. The 'baseTableRowCounts == null' condition was already verified in line 68. MetadataAggregator.cs 70

private MetadataAggregator(....)
{
  ....
  if (baseTableRowCounts == null)                           // <=
  {
    if (baseReader == null)
    {
      throw new ArgumentNullException("deltaReaders");
    }

    if (baseReader.GetTableRowCount(TableIndex.EncMap) != 0)
    {
      throw new ArgumentException("....", "baseReader");
    }

    CalculateBaseCounts(baseReader, out baseTableRowCounts, // <=
                                    out baseHeapSizes);
  }
  else
  {
    if (baseTableRowCounts == null)                      // <=???
    {
      throw new ArgumentNullException("baseTableRowCounts");
    }

    ....
  }
  ....
}

Анализатор обнаружил условие, которое уже проверялось. Если посмотреть на фрагмент кода, то последняя проверка в 'else' - "baseTableRowCounts == null" не имеет смысла. Зато выше по коду можно увидеть, что если переменная "baseTableRowCounts" равна null, то её значение пытаются изменить вызовом функции CalculateBaseCounts(). Вот после этой функции, скорее всего, и не хватает дополнительной проверки "baseTableRowCounts == null". Т.е. скорее всего код должен выглядеть так:

private MetadataAggregator(....)
{
  ....
  if (baseTableRowCounts == null)
  {
    if (baseReader == null)
    {
      throw new ArgumentNullException("deltaReaders");
    }

    if (baseReader.GetTableRowCount(TableIndex.EncMap) != 0)
    {
      throw new ArgumentException("....", "baseReader");
    }

    CalculateBaseCounts(baseReader, out baseTableRowCounts,
                                    out baseHeapSizes);
    if (baseTableRowCounts == null)
    {
      throw new ArgumentNullException("baseTableRowCounts");
    }

  }
  else
  {
    ....
  }
  ....
}

Остальные предупреждения

V3022 Expression 'readercount >= 0' is always true. Unsigned type value is always >= 0. ReaderWriterLockSlim.cs 977

private void ExitAndWakeUpAppropriateWaitersPreferringWriters()
{
  ....
  uint readercount = GetNumReaders();
  ....
  
  if (readercount == 1 && _numWriteUpgradeWaiters > 0)
  {
    ....
  }
  else if (readercount == 0 && _numWriteWaiters > 0)
  {
    ExitMyLock();
    _writeEvent.Set();
  }
  else if (readercount >= 0)
  {
    ....
  }
  else
    ExitMyLock();
  ....
}

Переменная "readercount" имеет беззнаковый тип, поэтому условие "readercount >= 0" не имеет смысла. Скорее всего, раньше она была знакового типа, но тогда для функции ExitMyLOck() в последнем 'else' был хоть какой-то шанс выполниться. Теперь этот код никогда не получает управления. Необходимо переписать это место.

V3014 It is likely that a wrong variable is being incremented inside the 'for' operator. Consider reviewing 'i'. RegexCharClass.cs 1094

private void Canonicalize()
{
  ....
  for (i = 1, j = 0; ; i++)
  {
    for (last = _rangelist[j]._last; ; i++)
    {
      if (i == _rangelist.Count || last == LastChar)
      {
        done = true;
        break;
      }

      if ((CurrentRange = _rangelist[i])._first > last + 1)
        break;

      if (last < CurrentRange._last)
        last = CurrentRange._last;
    }

    _rangelist[j] = new SingleRange(_rangelist[j]._first, last);

    j++;

    if (done)
      break;

    if (j < i)
      _rangelist[j] = _rangelist[i];
  }
  _rangelist.RemoveRange(j, _rangelist.Count - j);
  ....
}

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

V3004 The 'then' statement is equivalent to the 'else' statement. XmlSerializationWriterILGen.cs 1213

private void WriteMember(...., TypeDesc memberTypeDesc, ....)
{
  ....
  if (memberTypeDesc.IsArray)
  {
    LocalBuilder localI = ilg.DeclareOrGetLocal(...., iVar);
    ilg.For(localI, 0, ilg.GetLocal(aVar));
  }
  else
  {
    LocalBuilder localI = ilg.DeclareOrGetLocal(...., iVar);
    ilg.For(localI, 0, ilg.GetLocal(aVar));
  }
  ....
}

Условие, которое ни на что не влияет, т.к. всегда будет выполнятся один код. Классический copy-paste.

V3004 The 'then' statement is equivalent to the 'else' statement. SqlUtil.cs 93

internal static void ContinueTask(....)
{
  ....
  if (connectionToDoom != null || connectionToAbort != null)
  {
    try
    {
      onSuccess();
    }
    catch (Exception e)
    {
      completion.SetException(e);
    }
  }
  else
  { // no connection to doom - reliability section not required
    try
    {
      onSuccess();
    }
    catch (Exception e)
    {
      completion.SetException(e);
    }
  }
  ....
}

Тут тоже что-то много одинакового кода в условии, хотя в комментарии написано, что ситуации разные.

Заключение

Вот проверен ещё один проект от Microsoft. Для такого объёма проект имеет довольно качественный код, но программисты всё равно могут допускать ошибки. Статья является обзорной и содержит далеко не все предупреждения анализатора, которые были получены в отчёте.

Качественному коду способствуют два очень важных обстоятельства:

  • Регулярный статический анализ проекта, а не разовый;
  • Просмотр предупреждений анализатора авторами соответствующих фрагментов кода.

Надеемся, вам понравилась эта статья. Обещаем и дальше радовать наших читателей проверками интересных открытых проектов на языках C/C++ и C#.

Спасибо за внимание. И безбажного вам кода в Новом Году!

Популярные статьи по теме
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

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

Дата: 27 Июн 2017

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

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

Дата: 14 Апр 2016

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

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

Дата: 20 Мар 2017

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

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

Дата: 16 Окт 2017

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

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

Дата: 30 Янв 2019

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

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

Дата: 22 Дек 2018

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

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

Дата: 31 Май 2014

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

Я изучил множество ошибок, возникающих в результате копирования кода. И утверждаю, что чаще всего ошибки допускают в последнем фрагменте однотипного кода. Ранее я не встречал в книгах описания этого …
PVS-Studio для Java

Дата: 17 Янв 2019

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

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

Дата: 21 Ноя 2018

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

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

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

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

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