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

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

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

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

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

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

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


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

>
>
>
"Почему ещё не изобрели искусствен…

"Почему ещё не изобрели искусственный интеллект?" или проверка инструментария CNTK от Microsoft Research

02 Фев 2016

Microsoft выложила в открытый доступ исходный код инструментов, которые используются в компании для ускорения разработок в области искусственного интеллекта: набор Computational Network Toolkit теперь доступен на Github. Разработчикам пришлось создать собственное решение, так как имеющиеся инструменты работали слишком медленно. Давайте же взглянем на результаты проверки этого проекта статическим анализатором кода.

0372_CNTK_ru/image1.png

Введение

Computational Network Toolkit (CNTK) - набор инструментов для проектирования и тренировки сетей различного типа, которые можно использовать для распознавания образов, понимания речи, анализа текстов и многого другого.

PVS-Studio - это статический анализатор для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#. Инструмент PVS-Studio предназначен для разработчиков современных приложений и интегрируется в среды Visual Studio 2010-2015.

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

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

Эх, опечатки

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

0372_CNTK_ru/image2.png

V501 There are identical sub-expressions '!Input(0)->HasMBLayout()' to the left and to the right of the '||' operator. trainingnodes.h 1416

virtual void Validate(bool isFinalValidationPass) override
{
  ....
  if (isFinalValidationPass &&
      !(Input(0)->GetSampleMatrixNumRows() ==
        Input(2)->GetSampleMatrixNumRows() &&
       (Input(0)->GetMBLayout() ==
        Input(2)->GetMBLayout() ||
       !Input(0)->HasMBLayout() ||            // <=
       !Input(0)->HasMBLayout())))            // <=
  {
    LogicError(..., NodeName().c_str(),OperationName().c_str());
  }
   ....
}

Форматирование этого фрагмента сильно изменено для наглядности. Только после этого стало очевидным, что в условии присутствуют две одинаковые проверки "!Input(0)->HasMBLayout()". Скорее всего, в одном случае хотели использовать элемент с индексом '2'.

V501 There are identical sub-expressions to the left and to the right of the '-' operator: i0 - i0 ssematrix.h 564

void assignpatch(const ssematrixbase &patch,
                 const size_t i0,
                 const size_t i1,
                 const size_t j0,
                 const size_t j1)
{
  ....
  for (size_t j = j0; j < j1; j++)
  {
    const float *pcol = &patch(i0 - i0, j - j0);      // <=
    float *qcol = &us(i0, j);
    const size_t colbytes = (i1 - i0) * sizeof(*pcol);
    memcpy(qcol, pcol, colbytes);
  }
  ....
}

Из-за печатки выражение "i0-i0" всегда равно нулю. Возможно, тут хотели написать "i1-i0" или "j - i1", или ещё как-нибудь. Разработчикам необходимо обязательно проверить это место.

V596 The object was created but it is not being used. The 'throw' keyword could be missing: throw runtime_error(FOO); simplenetworkbuilder.cpp 1578

template <class ElemType>
ComputationNetworkPtr SimpleNetworkBuilder<ElemType>::
  BuildNetworkFromDbnFile(const std::wstring& dbnModelFileName)
{
  ....
  if (this->m_outputLayerSize >= 0)
    outputLayerSize = this->m_outputLayerSize;
  else if (m_layerSizes.size() > 0)
    m_layerSizes[m_layerSizes.size() - 1];
  else
    std::runtime_error("Output layer size must be...");     // <=
  ....
}

Ошибка заключается в том, что случайно забыто ключевое слово 'throw'. В результате данный код не генерирует исключение в случае ошибочной ситуации. Исправленный вариант кода:

....
else
  throw std::runtime_error("Output layer size must be...");
....

Работа с файлами

0372_CNTK_ru/image3.png

V739 EOF should not be compared with a value of the 'char' type. The 'c' should be of the 'int' type. fileutil.cpp 852

string fgetstring(FILE* f)
{
  string res;
  for (;;)
  {
    char c = (char) fgetc(f);        // <=
    if (c == EOF)                    // <=
      RuntimeError("error reading .... 0: %s", strerror(errno));
    if (c == 0)
      break;
    res.push_back(c);
  }
  return res;
}

Анализатор обнаружил, что константа EOF сравнивается с переменной типа 'char'. Это свидетельствует о том, что некоторые символы будут обрабатываться программой неверно.

Рассмотрим, как объявлен EOF:

#define EOF (-1)

Как видите, EOF есть ни что иное как '-1' типа 'int'. Функция fgetc () возвращает значения типа 'int'. А именно - она может вернуть число от 0 до 255 или -1 (EOF). Прочитанные значение помещаются в переменную типа 'char'. Из-за этого символ со значением 0xFF (255) превращается в -1 и интерпретируется точно также как конец файла (EOF).

Пользователи, использующие Extended ASCII Codes, могут столкнуться с ошибкой, когда один из символов их алфавита некорректно обрабатывается программой.

Например, последняя буква русского алфавита в кодировке Windows-1251 как раз имеет код 0xFF и воспримется программой как символ конца файла.

Исправленный фрагмент кода:

int c = fgetc(f);
if (c == EOF)
  RuntimeError(....);

V547 Expression 'val[0] == 0xEF' is always false. The value range of char type: [-128, 127]. file.cpp 462

bool File::IsUnicodeBOM(bool skip)
{
  ....
  else if (m_options & fileOptionsText)
  {
    char val[3];
    file.ReadString(val, 3);
    found = (val[0] == 0xEF && val[1] == 0xBB && val[2] == 0xBF);
  }
  // restore pointer if no BOM or we aren't skipping it
  if (!found || !skip)
  {
    SetPosition(pos);
  }
  ....
}

По умолчанию тип 'char' имеет диапазон значений равный [-127;127]. С помощью флага компиляции /J можно сказать компилятору, чтобы использовался диапазон [0;255]. Но для этого исходного файла такой флаг не указан, поэтому такой код никогда не определит, что файл содержит BOM.

Работа с памятью

0372_CNTK_ru/image4.png

V595 The 'm_rowIndices' pointer was utilized before it was verified against nullptr. Check lines: 171, 175. libsvmbinaryreader.cpp 171

template <class ElemType>
void SparseBinaryMatrix<ElemType>::ResizeArrays(size_t newNNz)
{
  ....
  if (m_nnz > 0)
  {
    memcpy(rowIndices, m_rowIndices, sizeof(int32_t)....);  // <=
    memcpy(values, this->m_values, sizeof(ElemType)....);   // <=
  }

  if (m_rowIndices != nullptr)
  {
    // free(m_rowIndices);
    CUDAPageLockedMemAllocator::Free(this->m_rowIndices, ....);
  }
  if (this->m_values != nullptr)
  {
    // free(this->m_values);
    CUDAPageLockedMemAllocator::Free(this->m_values, ....);
  }
  ....
}

Анализатор обнаружил потенциальное разыменование нулевого указателя. Если в коде присутствует сравнение указателя с нулём, но выше по коду этот указатель используется без проверки, то такой код является подозрительным и опасным.

Функции memcpy() копируют байты, расположенные по адресам 'm_rowIndices' и 'm_values', при этом выполняется разыменование этих указателей, а в приведённом примере кода они потенциально могут быть нулевыми.

V510 The 'sprintf_s' function is not expected to receive class-type variable as third actual argument. binaryfile.cpp 501

const std::wstring& GetName()
{
  return m_name;
}

Section* Section::ReadSection(....)
{
  ....
  char message[256];
  sprintf_s(message,"Invalid header in file %ls, in header %s\n",
              m_file->GetName(), section->GetName());       // <=
  RuntimeError(message);
  ....
}

В качестве фактических параметров функции sprint_s() могут выступать только POD типы. POD - это аббревиатура от "Plain Old Data", что можно перевести как "Простые данные в стиле Си".

"std::wstring" к POD-типам не относится. Вместо указателя на строку в стек попадёт содержимое объекта. Такой код приведет к формированию в буфере "абракадабры" или к аварийному завершению программы.

Исправленный вариант:

sprintf_s(message,"Invalid header in file %ls, in header %s\n",
          m_file->GetName().c_str(), section->GetName().c_str());

V630 The 'malloc' function is used to allocate memory for an array of objects which are classes containing constructors. latticeforwardbackward.cpp 912

void lattice::forwardbackwardalign()
{
  ....
  aligninfo *refinfo;
  unsigned short *refalign;

  refinfo = (aligninfo *) malloc(sizeof(aligninfo) * 1);    // <=
  refalign = (unsigned short *) malloc(sizeof(....) * framenum);

  array_ref<aligninfo> refunits(refinfo, 1);
  array_ref<unsigned short> refedgealignmentsj(....);
  ....
}

В этом фрагменте кода найдено некорректное выделение динамической памяти под структуру типа "aligninfo". Дело в том, что в определении структуры присутствуют конструкторы, а при таком способе выделения памяти не будет вызван конструктор. При освобождении памяти с помощью функции free() также не будет вызван деструктор.

Ниже приведён код описания типа "aligninfo":

struct aligninfo // phonetic alignment
{
  unsigned int unit : 19;   // triphone index
  unsigned int frames : 11; // duration in frames
  unsigned int unused : 1; // (for future use)
  unsigned int last : 1;   // set for last entry
  aligninfo(size_t punit, size_t pframes)
      : unit((unsigned int) punit),
        frames((unsigned int) pframes), unused(0), last(0)
  {
    checkoverflow(unit, punit, "aligninfo::unit");
    checkoverflow(frames, pframes, "aligninfo::frames");
  }
  aligninfo() // [v-hansu] initialize to impossible values
  {
#ifdef INITIAL_STRANGE
    unit = unsigned int(-1);
    frames = unsigned int(-1);
    unused = unsigned int(-1);
    last = unsigned int(-1);
#endif
  }
  template <class IDMAP>
  void updateunit(const IDMAP& idmap /*[unit] -> new unit*/)
  {
    const size_t mappedunit = idmap[unit];
    unit = (unsigned int) mappedunit;
    checkoverflow(unit, mappedunit, "aligninfo::unit");
  }
};

Исправленный вариант:

aligninfo *refinfo = new aligninfo();

И естественно для освобождения памяти потребуется вызывать оператор 'delete'.

V599 The virtual destructor is not present, although the 'IDataWriter' class contains virtual functions. datawriter.cpp 47

IDataWriter<ElemType>* m_dataWriter;
....
template <class ElemType>
void DataWriter<ElemType>::Destroy()
{
    delete m_dataWriter; // <= V599 warning
    m_dataWriter = NULL;
}

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

template <class ElemType>
class DATAWRITER_API IDataWriter
{
public:
    typedef std::string LabelType;
    typedef unsigned int LabelIdType;

    virtual void Init(....) = 0;
    virtual void Init(....) = 0;
    virtual void Destroy() = 0;
    virtual void GetSections(....) = 0;
    virtual bool SaveData(....) = 0;
    virtual void SaveMapping(....) = 0;
};

Это определение базового класса, как мы видим - он содержит виртуальные функции, но отсутствует виртуальный деструктор.

m_dataWriter = new HTKMLFWriter<ElemType>();

Таким образом выделяется память под объект производного класса "HTKMLFWriter". Найдём его описание:

template <class ElemType>
class HTKMLFWriter : public IDataWriter<ElemType>
{
private:
    std::vector<size_t> outputDims;
    std::vector<std::vector<std::wstring>> outputFiles;

    std::vector<size_t> udims;
    std::map<std::wstring, size_t> outputNameToIdMap;
    std::map<std::wstring, size_t> outputNameToDimMap;
    std::map<std::wstring, size_t> outputNameToTypeMap;
    unsigned int sampPeriod;
    size_t outputFileIndex;
    void Save(std::wstring& outputFile, ....);
    ElemType* m_tempArray;
    size_t m_tempArraySize;
    ....
};

Из-за пропущенного виртуального деструктора в базовом классе, этот объект не будет корректно разрушен. Не будут вызваны деструкторы для объектов outputDims, outputFiles и так далее. А вообще все последствия предсказать невозможно, ведь "неопределенное поведение" не зря так называется.

Разные ошибки

0372_CNTK_ru/image5.png

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. sequenceparser.h 338

enum SequenceFlags
{
    seqFlagNull = 0,
    seqFlagLineBreak = 1, // line break on the parsed line
    seqFlagEmptyLine = 2, // empty line
    seqFlagStartLabel = 4,
    seqFlagStopLabel = 8
};

long Parse(....)
{
  ....
  // sequence state machine variables
  bool m_beginSequence;
  bool m_endSequence;
  ....
  if (seqPos)
  {
    SequencePosition sequencePos(numbers->size(), labels->size(),
      m_beginSequence ? seqFlagStartLabel : 0 | m_endSequence ?
      seqFlagStopLabel : 0 | seqFlagLineBreak);
    // add a sequence element to the list
    seqPos->push_back(sequencePos);
    sequencePositionLast = sequencePos;
  }
  
  // end of sequence determines record separation
  if (m_endSequence)
      recordCount = (long) labels->size();
  ....
}

Приоритет тернарного оператора ':?' ниже приоритета побитового ИЛИ '|'. Рассмотрим фрагмент с ошибкой отдельно:

0 | m_endSequence ? seqFlagStopLabel : 0 | seqFlagLineBreak

В коде ожидалось выполнение побитовых операций только с заданными флагами, но из-за непредвиденного порядка выполнения операторов, сначала вычислится "0 | m_endSequence", а не "m_endSequence ? seqFlagStopLabel : 0 | seqFlagLineBreak".

Вообще это интересный случай. Несмотря на ошибку, код работает правильно. Побитое ИЛИ с 0 не оказывает никакого влияния. Тем не менее ошибку лучше поправить.

Ещё два таких места:

  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. sequenceparser.h 433
  • V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. sequenceparser.h 598

V530 The return value of function 'size' is required to be utilized. basics.h 428

// TODO: merge this with todouble(const char*) above
static inline double todouble(const std::string& s)
{
  s.size(); // just used to remove the unreferenced warning

  double value = 0.0;
  ....
}

В этом месте нет ошибки, о чём нам говорит оставленный комментарий, но этот пример я привёл не просто так:

Во-первых, для отключения предупреждения компилятора есть UNREFERENCED_PARAMETER macro, название которого однозначно даёт понять, что параметр функции не используется намеренно:

#define UNREFERENCED_PARAMETER(P) (P)

static inline double todouble(const std::string& s)
{
  UNREFERENCED_PARAMETER(s);
  ....
}

Во-вторых, я хотел подвести к другому предупреждению анализатора, которое, скорее всего, указывает на ошибку:

V530 The return value of function 'empty' is required to be utilized. utterancesourcemulti.h 340

template <class UTTREF>
std::vector<shiftedvector<....>>getclassids(const UTTREF &uttref)
{
  std::vector<shiftedvector<....>> allclassids;
  allclassids.empty();  // <=
  ....
}

Не имеет смысла не использовать результат функции empty(). Скорее всего тут хотели очистить вектор с помощью функции clear().

Похожее место:

  • V530 The return value of function 'empty' is required to be utilized. utterancesourcemulti.h 364

V688 The 'm_file' local variable possesses the same name as one of the class members, which can result in a confusion. sequencereader.cpp 552

template <class ElemType>
class SequenceReader : public IDataReader<ElemType>
{
protected:
  bool m_idx2clsRead;
  bool m_clsinfoRead;

  bool m_idx2probRead;
  std::wstring m_file;                               // <=
  ....
}

template <class ElemType>
template <class ConfigRecordType>
void SequenceReader<ElemType>::InitFromConfig(....)
{
  ....
  std::wstring m_file = readerConfig(L"file");       // <=
  if (m_traceLevel > 0)
  {
    fprintf(stderr, "....", m_file.c_str());

  }
  ....
}

Использование одноимённых переменных в классе, в функциях класса и параметрах класса является очень плохим стилем программирования. Вот в этом примере: объявление переменной "std::wstring m_file = readerConfig(L"file");" на самом деле должно было быть здесь или было добавлено временно для отладки, а потом забыли удалить?

Разработчикам необходимо проверить это и следующие места:

  • V688 The 'm_file' local variable possesses the same name as one of the class members, which can result in a confusion. sequencereader.cpp 1554
  • V688 The 'm_mbStartSample' function argument possesses the same name as one of the class members, which can result in a confusion. sequencereader.cpp 2062
  • V688 The 'm_file' local variable possesses the same name as one of the class members, which can result in a confusion. lusequencereader.cpp 417

Заключение

0372_CNTK_ru/image6.png

Computational Network Toolkit (CNTK) - оказался небольшим, но очень интересным проектом для проверки. Так как проект CNTK только недавно был открыт, то ждём интересных решений с его использованием, а также ждём открытие других проектов Microsoft.

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

Дата: 22 Дек 2018

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

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

Дата: 27 Июн 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 30 Янв 2019

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

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

Дата: 31 Июл 2017

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

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

Дата: 16 Окт 2017

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

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

Дата: 31 Май 2014

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

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

Дата: 19 Май 2017

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

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

Дата: 21 Ноя 2018

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

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

Дата: 17 Янв 2019

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

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

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

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

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