Linux-версия PVS-Studio не могла не проверить CodeLite

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

О проекте

CodeLite — это бесплатная кросс-платформенная среда разработки C, C++, PHP и Node.js с открытым исходным кодом, в которой используется набор инструментов wxWidgets. Чтобы соответствовать духу программного обеспечения с открытым исходным кодом, CodeLite компилируется и отлаживается исключительно с помощью бесплатных инструментов (MinGW и GDB).

Возможности CodeLite: управление проектами, завершение кода (ctags + clang), рефакторинг кода, подсветка синтаксиса, интеграция с Subversion и Git, интеграция с Cscope, интеграция с UnitTest++, интерактивный отладчик, построенный на основе GDB, и мощный редактор исходного кода (на основе Scintilla) .

Codelite распространяется под лицензией GNU General Public License версии 2 или более поздней. Это бесплатно. Codelite, будучи хорошо разработанным и отлаженным, может использоваться в качестве платформы для разработки.

Современные версии CodeLite также поддерживают проекты на PHP и Node.js.

Исходный код CodeLite доступен на GitHub

Результаты анализа

Для проверки я использовал PVS-Studio для Linux. Кратко расскажу о рабочем процессе.

Прежде чем приступить к работе, я прочитал инструкцию по запуску и использованию PVS-Studio для Linux. Анализатор можно использовать двумя способами: интегрировать в систему сборки (считается лучшим способом) или использовать как утилиту pvs-studio-analyzer. Чтобы быстро сделать проверку и приступить к анализу ошибок, я решил воспользоваться вторым способом.

Итак, поехали.

Сначала я скачал исходный код проекта.

Затем я создал простой конфиг-файл — PVS-Studio.cfg — где прописал следующее:

exclude-path = /usr/include/
lic-file = /path/to/PVS-Studio.lic
output-file = /path/to/PVS-Studio.log

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

$ mkdir codelite/build
$ cd build
$ cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ../

После того, как проект был успешно собран, я приступил к анализу:

$ pvs-studio-analyzer analyze --cfg /path/to/PVS-Studio.cfg -j4

В итоге я получил файл PVS-Studio.log по указанному пути в PVS-Studio.cfg. Чтобы получить от него полезную информацию, я воспользовался утилитой plog-converter, входящей в состав дистрибутива PVS-Studio.

Для просмотра отчета анализатора я запустил plog-converter следующим образом:

$ plog-converter -a GA:1,2 -t tasklist -o /path/to/codelite.tasks 
/path/to/PVS-Studio.log

После этой команды я получил codelite.tasks в указанном каталоге, который я открыл с помощью Qt Creator.

Обработка указателя

Предупреждение V595 Указатель pResult был использован до того, как он был проверен на соответствие nullptr. Проверить строки: 522, 526. SqliteDatabaseLayer.cpp 522

bool CodeBlocksImporter::isSupportedWorkspace()
{
  ....
  wxXmlNode* root = codeBlocksProject.GetRoot();
  wxString nodeName = root->GetName();                // <=
  
  if(root &&                                          // <=
    (nodeName == wxT("CodeBlocks_workspace_file") || 
     nodeName == wxT("CodeBlocks_project_file")))
      return true;
  }
  return false;
}

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

Похожие предупреждения анализатора:

• V595 Указатель pResult использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 522, 526. SqliteDatabaseLayer.cpp 522
• V595 Указатель «ms_instance» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 24, 25. php_parser_thread.cpp 24

Предупреждение V512 Вызов функции memset приведет к недополнению буфера EndTimestampListHandles. md5.cpp 243

class MD5
{
  ....
  // assumes char is 1 word long
  typedef unsigned      char uint1; 
  // next, the private data:
  ....
  uint1 buffer[64];   // input buffer
  ....
  static void memset(uint1 *start, uint1 val, uint4 length);
  ....
};
void MD5::finalize ()
{
  ....
  // Zeroize sensitive information
  memset (buffer, 0, sizeof(*buffer));        // <=
  finalized=1;
}

Здесь ошибка связана с неправильным значением третьего аргумента, передаваемого в функцию memset. Оператор sizeof(*buffer) возвращает не фактический размер буфера, а размер первого элемента, что является ошибкой. В этом конкретном примере в memset будет передан только 1 байт вместо 64.

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

Предупреждение V668 Нет смысла проверять указатель буфер на нуль, так как память была выделена с помощью оператора новый. Исключение будет сгенерировано в случае ошибки выделения памяти. ShapeDataObject.cpp 65

wxString wxSFShapeDataObject::SerializeSelectedShapes(....)
{
  ....
  char *buffer = new char [outstream.GetSize()];
  if(buffer)        //<=
  {
    memset(buffer, 0, outstream.GetSize());
    outstream.CopyTo(buffer, outstream.GetSize()-1);
    wxString output(buffer, wxConvUTF8);
    delete [] buffer;
    return output;
  }
  else
    return wxT(....);
}

Здесь у нас есть бессмысленная проверка указателя. Согласно стандартам языка C++, при выделении памяти через new не имеет смысла проверять указатель на нуль, так как может быть выдано исключение std::bad_alloc() в случае, если память выделить не получится. В таких случаях следует использовать блок try… catch для обработки таких критических ситуаций. Если вы хотите избежать использования исключений, тогда есть new, который не генерирует исключений. Например:

char *buffer = new char (std::nothrow) [outstream.GetSize()];

Конечно, использование try..catch или std::nothrow не являются примерами изящных решений и представлены здесь только как варианты быстрых и грубых исправлений.

Были обнаружены и другие подобные ситуации (здесь приведены только некоторые из сообщений, всего их 19):

• V668 Проверять указатель pResultSet на null нет смысла, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. SqliteDatabaseLayer.cpp 199
• V668 Проверять указатель pReturnStatement на null нет смысла, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. SqliteDatabaseLayer.cpp 223
• V668 Проверять указатель m_proc на null нет смысла, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. async_executable_cmd.cpp 182
• и так далее…

Это невнимание…

Предупреждение V519 Переменной m_commentEndLine два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 175, 176. PhpLexerAPI.h 176

struct WXDLLIMPEXP_CL phpLexerUserData {
    ....
    int m_commentStartLine;
    int m_commentEndLine;
    ....
    void ClearComment()
    {
        m_comment.clear();
        m_commentEndLine = wxNOT_FOUND;     //<=
        m_commentEndLine = wxNOT_FOUND;
    }
};

Явная ошибка копирования-вставки. В классе phpLexerUserData помимо переменной commentEndLine есть переменная commentStartLine. По сути, метод ClearComment должен быть таким:

void ClearComment()
{
  m_comment.clear();
  m_commentStartLine = wxNOT_FOUND;
  m_commentEndLine = wxNOT_FOUND;
}

Такая же ошибка была обнаружена еще в нескольких местах:

• V519 Переменной m_commentEndLine два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 171, 172. CxxLexerAPI.h 172
• V519 Переменной m_commentEndLine два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 143, 144. JSLexerAPI.h 144

Предупреждение V547 Выражение type.Lower() == Array всегда ложно. NodeJSOuptutParser.h 61

struct NodeJSHandle {
  wxString type;
  ....
  bool IsString() const {return type.Lower() == "string";}
  bool IsArray() const {return type.Lower() == "Array"; }  //<=
};

Метод IsArray всегда будет возвращать false из-за небольшой опечатки. Чтобы исправить это, нам нужно просто заменить «Массив» на «массив», и все будет работать так, как должно.

Предупреждение V517 Обнаружено использование шаблона if (A) {…} else if (A) {…}. Существует вероятность наличия логической ошибки. Проверьте строки: 383, 386. MainFrame.cpp 383

void MainFrame::OnSignal(wxCommandEvent& e)
{
  if(m_process) {
    int sigid = e.GetId();
    if(sigid == ID_SIGHUP)
        wxKill(m_process->GetPid(), wxSIGHUP);
    else if(sigid == ID_SIGINT)
        wxKill(m_process->GetPid(), wxSIGINT);
    else if(sigid == ID_SIGKILL)
        wxKill(m_process->GetPid(), wxSIGKILL);
    else if(sigid == ID_SIGKILL)        // <=
        wxKill(m_process->GetPid(), wxSIGTERM);        
  }
}

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

void MainFrame::OnSignal(wxCommandEvent& e)
{
    ....
    else if(sigid == ID_SIGKILL)
        wxKill(m_process->GetPid(), wxSIGKILL);
    else if(sigid == ID_SIGTERM)        
        wxKill(m_process->GetPid(), wxSIGTERM);        
  }
}

Еще одно предупреждение анализатора:

• V517 Обнаружено использование шаблона «if (A) {…} else if (A) {…}». Существует вероятность наличия логической ошибки. Строки проверки: 212, 222. new_quick_watch_dlg.cpp 212

Предупреждение V530 Необходимо использовать возвращаемое значение функции пусто. act_network.cpp 56

StringTokenizer::StringTokenizer(const wxString& str,
                const wxString& strDelimiter,
                const bool &bAllowEmptyTokens /* false */)
{
  ....
  wxString token;
  while( nEnd != -1 )
  {
    if( nEnd != nStart)
      token = str.substr(nStart, nEnd-nStart);
    else
      token.empty();        // <=
    if(!token.empty())
      m_tokensArr.push_back(token);
    ....
  }
}

Функция empty() не изменяет объект, она только возвращает логический результат. Другими словами, ветка else ничего не делает. Вместо token.empty() программист должен был написать token.Empty(), который обнуляет строку, или, возможно, что-то еще.

Ой! Что-то было забыто

Предупреждение V729 Тело функции содержит метку find_rule, которая не используется никакими операторами goto. include_finder.cpp 716

....
#define YY_DECL int yylex YY_PROTO(( void ))
....
YY_DECL
  {
    ....
    yy_find_action:
      yy_current_state = *--yy_state_ptr;
      yy_lp = yy_accept[yy_current_state];
      /* we branch to this label when backing up */
    find_rule:         //<= 
    
    for ( ; ; ) /* until we find what rule we matched */
    ....
  }

Здесь ошибка в том, что среди большого количества строк кода есть метка find_rule, на которую не ссылается ни один из операторов goto. Это могло произойти из-за рефакторинга кода или, возможно, из-за чего-то еще. Пока эта одинокая этикетка не несет смысловой нагрузки, она лишь намекает, что где-то что-то забыли.

Такое предупреждение было найдено еще в нескольких местах:

• Тело функции V729 содержит метку «find_rule», которая не используется никакими операторами «goto». comment_parser.cpp 672
• Тело функции V729 содержит метку «find_rule», которая не используется никакими операторами «goto». cpp_expr_lexer.cpp 1090
• Тело функции V729 содержит метку «find_rule», которая не используется никакими операторами «goto». cpp_lexer.cpp 1138

Предупреждения V523 Оператор then эквивалентен оператору else. art_metro.cpp 402

void wxRibbonMetroArtProvider::DrawTab(
                 wxDC& dc,
                 wxWindow* WXUNUSED(wnd),
                 const wxRibbonPageTabInfo& tab)
{
    ....
    if (tab.active)
      dc.SetPen(m_tab_border_pen);
    else
      // TODO: introduce hover border pen colour
      dc.SetPen(m_tab_border_pen);              // <=
     
    ....
 }

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

Похожие предупреждения анализатора:

• V523 Оператор "then" эквивалентен оператору "else". art_metro.cpp 402
• V523 Оператор "then" эквивалентен оператору "else". php_workspace_view.cpp 948

Предупреждение V560 Часть условного выражения всегда ложна: 0. entry.c 397

extern void openTagFile (void)
{
  ....
  boolean fileExists;
  setDefaultTagFileName ();
  TagFile.name = eStrdup (Option.tagFileName);
  fileExists = doesFileExist (TagFile.name);
  /* allways override old files */
  if (fileExists  &&  /*! isTagFile (TagFile.name)*/ 0) //<= 
    error (FATAL,
      "\"%s\" doesn't look like a tag file; ....",
        TagFile.name);
  if (Option.etags)
   {
  ....
}

Здесь мы видим, что условие (fileExists && /*! isTagFile (TagFile.name)*/ 0) всегда ложно из-за 0. Возможно, так и должно было быть, но, скорее всего, это ошибка. Это могло попасть в код, когда программист занимался отладкой и изменил условие, но потом, закончив работу, забыл вернуть условие обратно.

Лишнее сравнение

Предупреждение V728 Излишняя проверка может быть упрощена. Оператор || окружен противоположными выражениями !found и found. editor_config.cpp 120

bool EditorConfig::Load()
  {
  ....
  if(userSettingsLoaded) {
      if(!found || (found && version != this->m_version)) { //<=
          if(DoLoadDefaultSettings() == false) {
              return false;
          }
      }
  }
  ....
}

Ошибки здесь нет, но такое условие плохо читается. Его можно сократить до:

if(!found && version != this->m_version)

Предупреждение V571 Периодическая проверка. Условие isInStatement уже проверено в строке 2292. ASBeautifier.cpp 2293

void ASBeautifier::parseCurrentLine(const string& line)
{
....
    if(isInStatement && !inStatementIndentStack->empty()) {
      if(prevNonSpaceCh == '=' &&
         isInStatement && !inStatementIndentStack->empty()) //<=
          inStatementIndentStack->back() = 0;
    }
  }
....
}

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

Вывод

В проекте CodeLite примерно 600 тысяч строк кода, написанного на C и C++. Конечно, были и некоторые ошибки, допущенные из-за невнимательности и обращения с указателями, как это происходит в большинстве проектов. Всего анализатор выдал 360 предупреждений первого и второго уровня. Около 40 из них — это те, которые нужно пересмотреть и, скорее всего, исправить.

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

Если вы хотите проверить свой проект, или любой интересующий вас проект, с помощью PVS-Studio для Linux, его можно скачать здесь.