Виктория Ханиева

10 Апр 2020

Теги:

#StaticAnalysis

DeepCode: взгляд со стороны

10 Апр 2020

Автор: Виктория Ханиева

Не так давно DeepCode, статический анализатор, основанный на машинном обучении, стал поддерживать проверку C и C++ проектов. И теперь мы можем на практике взглянуть, чем отличаются результаты классического статического анализа и статического анализа, основанного на машинном обучении.

Мы уже упоминали DeepCode в своей статье "Использование машинного обучения в статическом анализе исходного кода программ". И в скором времени ими была опубликована статья "DeepCode adds AI-based static code analysis support for C and C++" с объявлением поддержки анализа проектов, написанных на C и C++.

Чтобы посмотреть на результаты анализа DeepCode, я решила проверить проект PhysX. У меня нет цели представить здесь разбор ошибок, найденных PVS-Studio или DeepCode в этом проекте, а просто хотелось взять какой-то сторонний проект и посмотреть на его примере, как будут работать анализаторы.

Запуск анализа DeepCode крайне прост. Анализ проектов, выложенных на GitHub, запускается автоматически. Хотя тут есть проблема, так как был проверен весь репозиторий подряд, хотя в нем содержались различные проекты, и предупреждения к ним в итоге оказались перемешаны. Также скорость анализа указывает на то, что код не компилируется и многие ошибки могут оказаться скрытыми от анализатора.

Общий результат работы DeepCode выглядит следующим образом:

Это не совсем отражает число срабатываний, так как они группируются в одно предупреждение, как я покажу дальше. Конечно, очевидно, что статический анализ С/С++ в DeepCode является нововведением, и работа над ним только началась, но, думаю, уже возможно сделать некоторые выводы.

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

Это срабатывание зацепило мой глаз, так как у PVS-Studio аналогичная диагностика и, к тому же, я рассматривала такое срабатывание в другой своей статье. Когда я встретила его в первый раз, мне казалось, что очень мало кто будет писать математическую константу "от руки".

И PVS-Studio тоже нашел такие неосторожные использования констант, но срабатываний такого рода было уже не 31, а 52. Практически сразу нашелся момент, где машинное обучение уступает классическому подходу. Если ошибочный код хоть как-то отходит от совсем типичной ошибки такого рода, найти её становится куда сложнее. Для классического же статического анализа нет необходимости, чтобы ошибка встречалась много-много раз - достаточно, чтобы разработчик диагностики придумал общий принцип появления ошибки.

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

Так как DeepCode срабатывал строго на константу вида 3.1415, я из интереса добавила в одном месте в коде к константе 4 знака после запятой (3.14159263), и срабатывание пропало.

PVS-Studio же срабатывает на различные варианты округления констант.

В том числе присутствует и срабатывание на изменённый участок:

V624 There is probably a misprint in '3.14159263' constant. Consider using the M_PI constant from <math.h>. Crab.cpp 219

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

Рассмотрим еще одно срабатывание. DeepCode выдал одно предупреждение на следующий код:

bool Shader::loadShaderCode(const char* fragmentShaderCode, ....)
{
  ....
  if (mFragmentShaderSource != NULL)
    free(mFragmentShaderSource);

  ....

  if (mFragmentShaderSource != NULL)
    free(mFragmentShaderSource);
  ....
}

У PVS-Studio к этому коду было больше претензий:

V809 Verifying that a pointer value is not NULL is not required. The 'if (mFragmentShaderSource != 0)' check can be removed. Shader.cpp 178
V809 Verifying that a pointer value is not NULL is not required. The 'if (mFragmentShaderSource != 0)' check can be removed. Shader.cpp 194
V774 The 'mFragmentShaderSource' pointer was used after the memory was released. Shader.cpp 194
V586 The 'free' function is called twice for deallocation of the same memory space. Inspect the first argument. Check lines: 179, 195. Shader.cpp 195

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

Первые два срабатывания относятся к излишней проверке указателя, так как для использования функции free() такая проверка не нужна. Третье срабатывание сигнализирует о том, что небезопасно использовать указатель после того, как он был освобожден. Даже если сам указатель не разыменовывается, а только проверяется, это всё равно подозрительно и часто свидетельствует об опечатке. Ну и последнее срабатывание как раз и указывает на ту же проблему, которую обнаружил DeepCode.

В статье DeepCode о начале поддержки С/C++ был скриншот, который, как нам показалось, содержит ложное срабатывание. Предупреждение говорит о возможном переполнении буфера, однако память для буфера выделяется с учетом длины home и длины строки, которая добавляется далее еще и + 1. Таким образом места в буфере хватит точно.

Проблема здесь может быть в отсутствии проверки, что память была успешно выделена. Возможно предупреждение DeepCode в этом случае говорит о чем-то ином: первое предложение предупреждения особо не помогает в понимании в чем ошибка и на что собственно ругается анализатор.

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

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

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

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

Приведу несколько интересных ошибок, найденных PVS-Studio.

Фрагмент N1

V773 The function was exited without releasing the 'line' pointer. A memory leak is possible. PxTkBmpLoader.cpp 166

bool BmpLoader::loadBmp(PxFileHandle f)
{
  ....
  int lineLen = ....;
  unsigned char* line = static_cast<unsigned char*>(malloc(lineLen));
  for(int i = info.Height-1; i >= 0; i--)
  {
    num = fread(line, 1, static_cast<size_t>(lineLen), f);
    if (num != static_cast<size_t>(lineLen)) { fclose(f); return false; }
    ....
  }
  free(line);
  return true;
}

Тут, в случае, если чтение из файла прервалось, возникает утечка памяти, так как перед возвращением из функции память не освобождается.

Фрагмент N2

V595 The 'gSphereActor' pointer was utilized before it was verified against nullptr. Check lines: 228, 230. SnippetContactReportCCD.cpp 228

void initScene()
{
  ....
  gSphereActor = gPhysics->createRigidDynamic(spherePose);
  gSphereActor->setRigidBodyFlag(PxRigidBodyFlag::eENABLE_CCD, true);

  if (!gSphereActor)
    return;
  ....
}

Указатель gSphereActor разыменовывается, но сразу после этого проверяется на nullptr, и идет выход из функции. То есть нулевой указатель здесь возможен, но разыменовывание все равно происходит. Ошибок такого рода нашлось 24 штуки в проекте PhysX.

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

Тут DeepCode, скорее всего, по какой-то причине указывает не на ту инициализацию. Вместо инициализации gCooking подсвечивается инициализация gFoundation, хотя этот указатель далее не подсвечивается.

Фрагмент N3

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 266, 268. AABox.cpp 266

bool AABox::initRT(int depthSamples, int coverageSamples)
{
  ....
  int query;
  ....
  if (....)  
  {
    ....
    if ( query < coverageSamples)
      ret = false;
    else if ( query > coverageSamples) 
      coverageSamples = query;
    ....
    if ( query < depthSamples)
      ret = false;
    else if ( query > depthSamples)
      depthSamples = query;
  }
  else {
    ....
    if ( query < depthSamples)
      ret = false;
    else if ( query < depthSamples) // <=
      depthSamples = query;
    ....
  }
  ....
}

Тут похоже закралась ошибка копипасты. Анализатор обнаружил одинаковое условие у if и у else. Из предыдущих if-else можно увидеть, что обычно проводится проверка ">" в if и "<" в else. Хотя паттерн выглядит крайне простым, среди предупреждений DeepCode я не нашла такого срабатывания, хотя, казалось бы, это очень простой для обнаружения паттерн.

Заключение

Конечно, DeepCode только объявили о поддержке C/C++ и это часть их продукта, которая только начала развиваться. Вы можете попробовать его и на своих проектах, в их сервисе реализовано удобное окошко обратной связи, в случае, если вам было выдано ложное предупреждение.

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

#StaticAnalysis