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

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

Бесплатная лицензия PVS-Studio для специалистов Microsoft MVP
* Нажимая на кнопку, вы даете согласие на обработку
своих персональных данных. См. Политику конфиденциальности

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

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

Ваше сообщение отправлено.

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


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

>
>
>
Проверка Firebird 3.0

Проверка Firebird 3.0

11 Май 2016

Не так давно вышла новая версия СУБД Firebird. Релиз стал одним из масштабных в истории проекта: была сильно переработана архитектура, добавлена поддержка многопоточности, улучшена производительность. Такое значительное обновление и послужило поводом для повторной проверки Firebird с помощью статического анализатора кода PVS-Studio.

0396_Firebird_ru/image1.png

Введение

Firebird - это кроссплатформенная свободная система управлениями базами данных. Проект написан на C++ и работает на Microsoft Windows, Linux, Mac OS X и многих Unix-like операционных системах. СУБД полностью бесплатна для использования и распространения. Подробнее о Firebird можно узнать на официальном сайте.

Ранее Firebird уже проверялся анализатором. С отчётом о предыдущей проверке можно ознакомиться в статье "Побочный результат: проверяем Firebird с помощью PVS-Studio". Для проверки был взят код с GitHub из ветви master. Сборка подробно описана в соответствующей статье на сайте проекта. Анализ исходных файлов производился в PVS-Studio Standalone версии 6.03 с помощью перехвата вызова компиляторов (Compiler Monitoring). Данная технология позволяет проверять проекты без интеграции в сборочную систему. Полученный отчёт можно просмотреть как в Standalone версии, так и в Visual Studio.

Опечатки

void advance_to_start()
{
  ....
  if (!isalpha(c) && c != '_' && c != '.' && c != '_')
    syntax_error(lineno, line, cptr);
  ....
}

Предупреждение PVS-Studio: V501 There are identical sub-expressions 'c != '_'' to the left and to the right of the '&&' operator. reader.c 1203

Анализатор обнаружил, что в логической операции присутствуют два одинаковых подвыражения c != '_'. В последнем условии допущена опечатка и переменная c должна сравниваться с другим символом. В других функциях используется проверка с символом '$', возможно здесь нужно использовать его:

if (!isalpha(c) && c != '_' && c != '.' && c != '$')

Ещё один пример ошибки от невнимательности:

int put_message(....)
{
  if (newlen <= MAX_UCHAR)
    {
    put(tdgbl, attribute);
    put(tdgbl, (UCHAR) newlen);
  }
  else if (newlen <= MAX_USHORT)
  {
    if (!attribute2)
      BURP_error(314, "");
    ....
  }
  else
    BURP_error(315, "");
  ....
}

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

  • V601 The string literal is implicitly cast to the bool type. Inspect the second argument. backup.cpp 6113
  • V601 The string literal is implicitly cast to the bool type. Inspect the second argument. backup.cpp 6120

Здесь неверно используется функция BURP_error. Она объявлена следующим образом:

void BURP_error(USHORT errcode, bool abort,
     const MsgFormat::SafeArg& arg = MsgFormat::SafeArg());

void BURP_error(USHORT errcode, bool abort, const char* str);

Второй аргумент имеет логический тип, а строка стоит третьим аргументом. В результате строковый литерал приводится к true. Вызов функции должен быть переписан как BURP_error(315, true, "") или BURP_error(315, false, "").

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

void IDX_create_index(....)
{
  ....
  index_fast_load ifl_data;
  ....
  if (!ifl_data.ifl_duplicates)
    scb->sort(tdbb);

  if (!ifl_data.ifl_duplicates)
    BTR_create(tdbb, creation, selectivity);

  ....
}

Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' operators situated alongside each other are identical. Check lines: 506, 509. idx.cpp 509

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

В следующем примере рассмотрим ситуацию, связанную с указателями.

static void string_to_datetime(....)
{
  ....

  const char* p = NULL;
  const char* const end = p + length;

  ....

  while (p < end)
  {
    if (*p != ' ' && *p != '\t' && p != 0)
    {
      CVT_conversion_error(desc, err);
      return;
    }
    ++p;
  }

  ....
}

Предупреждение PVS-Studio: V713 The pointer p was utilized in the logical expression before it was verified against nullptr in the same logical expression. cvt.cpp 702

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

Если посмотреть выше по коду, то можно найти аналогичный фрагмент:

while (++p < end)
{
  if (*p != ' ' && *p != '\t' && *p != 0)
    CVT_conversion_error(desc, err);
}

Для того, чтобы избежать подобной ошибки, для сравнения с нулём лучше использовать соответствующие литералы: '\0' для типа char, 0 для чисел и nullptr для указателей. Если взять это за правило, можно избежать множества подобных глупых ошибок.

Опасное использование memcmp

SSHORT TextType::compare(ULONG len1, const UCHAR* str1,
                         ULONG len2, const UCHAR* str2)
{
  ....

  SSHORT cmp = memcmp(str1, str2, MIN(len1, len2));

  if (cmp == 0)
    cmp = (len1 < len2 ? -1 : (len1 > len2 ? 1 : 0));

  return cmp;
}

Предупреждение PVS-Studio: V642 Saving the 'memcmp' function result inside the 'short' type variable is inappropriate. The significant bits could be lost breaking the program's logic. texttype.cpp 3

Функция memcmp возвращает следующие значения:

  • < 0, если str1 меньше str2
  • 0, если str1 равен str2
  • > 0, если str1 больше str2

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

Лишние проверки

void Trigger::compile(thread_db* tdbb)
{
  SET_TDBB(tdbb);

  Database* dbb = tdbb->getDatabase();
  Jrd::Attachment* const att = tdbb->getAttachment();

  if (extTrigger)
    return;

  if (!statement /*&& !compile_in_progress*/)
  {
    if (statement)
      return;

    ....
  }
}

Предупреждение PVS-Studio: V637 Two opposite conditions were encountered. The second condition is always false. Check lines: 778, 780. jrd.cpp 778

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

Ещё одним примером странного ветвления может послужить следующий фрагмент кода.

static void asgn_from( ref* reference, int column)
{
  TEXT variable[MAX_REF_SIZE];
  TEXT temp[MAX_REF_SIZE];

  for (; reference; reference = reference->ref_next)
  {
    const gpre_fld* field = reference->ref_field;
    ....

    if (!field || field->fld_dtype == dtype_text)
      ....
    else if (!field || field->fld_dtype == dtype_cstring)
      ....
    else
      ....
  }
}

Предупреждение PVS-Studio: V560 A part of conditional expression is always false: !field. int_cxx.cpp 217

Если field не нулевой указатель, то код никогда не достигнет условия в else if. Либо данная проверка лишняя, либо на её месте должно было быть другое сравнение. Неизвестно, противоречит ли данное условие логике приложения.

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

bool XnetServerEndPoint::server_init(USHORT flag)
{
  ....

  xnet_connect_mutex = CreateMutex(ISC_get_security_desc(),
                          FALSE, name_buffer);
  if (!xnet_connect_mutex ||
          (xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS))
  {
    system_error::raise(ERR_STR("CreateMutex"));
  }

  ....
}

Предупреждение PVS-Studio: V728 An excessive check can be simplified. The '||' operator is surrounded by opposite expressions '!xnet_connect_mutex' and 'xnet_connect_mutex'. xnet.cpp 2231

Проверку if (!xnet_connect_mutex || (xnet_connect_mutex && ERRNO == ERROR_ALREADY_EXISTS)) можно упростить до if (!xnet_connect_mutex || ERRNO == ERROR_ALREADY_EXISTS). Это можно легко доказать с помощью таблицы истинности.

Опасное сравнение беззнаковой переменной

static bool write_page(thread_db* tdbb, BufferDesc* bdb, ....)
{
  ....
  if (bdb->bdb_page.getPageNum() >= 0)
  ....
}

Предупреждение PVS-Studio: V547 Expression 'bdb->bdb_page.getPageNum() >= 0' is always true. Unsigned type value is always >= 0. cch.cpp 4827

Условие bdb->bdb_page.getPageNum() >= 0 всегда окажется верным, так как функция возвращает значение беззнакового типа. Возможно, программист неверно проверил значение. Принимая во внимания аналогичные сравнения в проекте, можно предположить, что код должен выглядеть следующим образом:

if (bdb->bdb_page.getPageNum() != 0)

Разыменовывание нулевых указателей

static bool initializeFastMutex(FAST_MUTEX* lpMutex, 
  LPSECURITY_ATTRIBUTES lpAttributes, BOOL bInitialState,
  LPCSTR lpName)
{
  if (pid == 0)
    pid = GetCurrentProcessId();
  
  LPCSTR name = lpName;

  if (strlen(lpName) + strlen(FAST_MUTEX_EVT_NAME) - 2
                                                   >= MAXPATHLEN)
  {
    SetLastError(ERROR_FILENAME_EXCED_RANGE);
    return false;
  }

  setupMutex(lpMutex);

  char sz[MAXPATHLEN]; 
  if (lpName)
  ....
}

Предупреждение PVS-Studio: V595 The 'lpName' pointer was utilized before it was verified against nullptr. Check lines: 2814, 2824. isc_sync.cpp 2814

Предупреждение V595 является самым часто встречаемым в проектах, проверенных PVS-Studio, и Firebird не стал исключением. Всего обнаружено более 30 мест с данным предупреждением.

В этом примере вызов strlen(lpName) идёт перед проверкой указателя на nullptr. Это приведёт к неопределённому поведению при попытке передать в функцию нулевой указатель. Разыменовывание указателя здесь спрятано в вызове strlen, что затрудняет обнаружение ошибки, если не пользоваться статическим анализатором.

Проверка на nullptr после new

rem_port* XnetServerEndPoint::get_server_port(....)
{
  ....
  XCC xcc = FB_NEW struct xcc(this);

  try {

    ....
  }
  catch (const Exception&)
  {
    if (port)
      cleanup_port(port);
    else if (xcc)
      cleanup_comm(xcc);

    throw;
  }

  return port;
}

Предупреждение PVS-Studio: V668 There is no sense in testing the 'xcc' pointer against null, as the memory was allocated using the 'new' operator. The exception will be generated in the case of memory allocation error. xnet.cpp 2533

Анализатор предупреждает, что оператор new не может вернуть nullptr - для проверки необходимо использовать блок try-catch или new (std::nothrow). Однако, в данном примере всё несколько сложнее. Для выделения памяти используется макрос FB_NEW. Он объявлен в файле alloc.h:

#ifdef USE_SYSTEM_NEW
#define OOM_EXCEPTION std::bad_alloc
#else
#define OOM_EXCEPTION Firebird::BadAlloc
#endif

#define FB_NEW new(__FILE__, __LINE__)

inline void* operator new(size_t s ALLOC_PARAMS)
throw (OOM_EXCEPTION)
{
  return MemoryPool::globalAlloc(s ALLOC_PASS_ARGS);
}

Сложно сказать, является ли этот конкретный пример ошибкой или нет, так как используется нестандартный аллокатор. Но из-за того, что в определении оператора указан throw (std::bad_alloc), подобная проверка выглядит подозрительно.

Опасное использование realloc

int mputchar(struct mstring *s, int ch)
{
  if (!s || !s->base) return ch;
  if (s->ptr == s->end) {
    int len = s->end - s->base;
    if ((s->base = realloc(s->base, len+len+TAIL))) {
      s->ptr = s->base + len;
      s->end = s->base + len+len+TAIL; }
    else {
      s->ptr = s->end = 0;
      return ch; } }
  *s->ptr++ = ch;
  return ch;
}

Предупреждение PVS-Studio: V701 realloc() possible leak: when realloc() fails in allocating memory, original pointer 's->base' is lost. Consider assigning realloc() to a temporary pointer. mstring.c 42

Выражение вида ptr = realloc(ptr, size) плохо тем, что в случае, когда realloc вернёт nullptr, указатель на память будет потерян. Чтобы этого избежать, следует сохранить результат realloc в временную переменную и после проверки на nullptr присвоить ptr её значение.

temp_ptr = realloc(ptr, new_size);
if (temp_ptr == nullptr) {
  //handle exception
} else {
  ptr = temp_ptr;
}

Неиспользованные значения enum в switch

template <typename CharType>
LikeEvaluator<CharType>::LikeEvaluator(....)
{
  ....
  PatternItem *item = patternItems.begin();
  ....
  switch (item->type)
  {
  case piSkipFixed:
  case piSkipMore:
    patternItems.grow(patternItems.getCount() + 1);
    item = patternItems.end() - 1;
    // Note: fall into
    case piNone:
      item->type = piEscapedString;
      item->str.data = const_cast<CharType*>
                        (pattern_str + pattern_pos - 2);
      item->str.length = 1;
      break;
    case piSearch:
      item->type = piEscapedString;
      // Note: fall into
    case piEscapedString:
      item->str.length++;
      break;
  }
  ....
}

Предупреждение PVS-Studio: V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 324

В конструкции switch были использованы не все значения enum, и отсутствует блок default. Возможно, что здесь забыли добавить обработку piDirectMatch. Список мест с аналогичным предупреждением:

  • V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch, piSkipMore. evl_string.h 351
  • V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 368
  • V719 The switch statement does not cover all values of the 'PatternItemType' enum: piDirectMatch. evl_string.h 387

Переполнение буфера

const int GDS_NAME_LEN = 32;
....
bool get_function(BurpGlobals* tdgbl)
{
  ....
  struct isc_844_struct {
    ....
    short isc_870; /* gds__null_flag */
    ....
    char  isc_874 [125]; /* RDB$PACKAGE_NAME */
    ....
  } isc_844;
 
  att_type attribute;
  TEXT    temp[GDS_NAME_LEN * 2];
  ....
  SSHORT prefixLen = 0;
  if (!/*X.RDB$PACKAGE_NAME.NULL*/
       isc_844.isc_870)
  {
    prefixLen = static_cast<SSHORT>(strlen(/*X.RDB$PACKAGE_NAME*/
                                           isc_844.isc_874));
    memcpy(temp, /*X.RDB$PACKAGE_NAME*/
                 isc_844.isc_874, prefixLen);
    temp[prefixLen++] = '.';
  }
  ....

}

Предупреждение PVS-Studio: V557 Array overrun is possible. The value of 'prefixLen ++' index could reach 124. restore.cpp 10040

Размер буфера isc_844.isc_874 равен 125, соответственно, максимальное возможное значение strlen(isc_844.isc_874) - 124. Размер temp - 64, что меньше этого значения. Запись по такому индексу может привести к переполнению буфера. Безопасней будет выделить больший объём памяти для переменной temp.

Сдвиг отрицательных чисел

static ISC_STATUS stuff_literal(gen_t* gen, SLONG literal)
{
  ....

  if (literal >= -32768 && literal <= 32767)
    return stuff_args(gen, 3, isc_sdl_short_integer, literal, 
                      literal >> 8);

  ....

}

Предупреждение PVS-Studio: V610 Unspecified behavior. Check the shift operator '>>'. The left operand is negative ('literal' = [-32768..32767]). array.cpp 848

Код содержит сдвиг вправо отрицательного числа. В стандарте C++ указано, что такое действие имеет неуточняемое поведение, значит на разных компиляторах и платформах может выдавать разный результат. Лучше переписать его так:

if (literal >= -32768 && literal <= 32767)
  return stuff_args(gen, 3, isc_sdl_short_integer, literal, 
                    (ULONG)literal >> 8);

Ещё одно место, где сработало предупреждение:

V610 Unspecified behavior. Check the shift operator '>>'. The left operand is negative ('i64value' = [-2147483648..2147483647]). exprnodes.cpp 6382

Переопределение переменной

THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
{
  int exit_code = -1;
  try
  {
    Service* svc = (Service*)arg;
    RefPtr<SvcMutex> ref(svc->svc_existence);
    int exit_code = svc->svc_service_run->serv_thd(svc);

    svc->started();
    svc->svc_sem_full.release();
    svc->finish(SVC_finished);
  }
  catch (const Exception& ex)
  {
    // Not much we can do here
    iscLogException("Exception in Service::run():", ex);
  }

  return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
}

Предупреждение PVS-Studio: V561 It's probably better to assign value to 'exit_code' variable than to declare it anew. Previous declaration: svc.cpp, line 1893. svc.cpp 1898

В этом примере вместо присвоения переменная exit_code переопределяется. Это скрывает из области видимости предыдущую переменную, и в итоге функция возвращает неверное значение, всегда равное -1.

Корректный код:

THREAD_ENTRY_DECLARE Service::run(THREAD_ENTRY_PARAM arg)
{
  int exit_code = -1;
  try
  {
    Service* svc = (Service*)arg;
    RefPtr<SvcMutex> ref(svc->svc_existence);
    exit_code = svc->svc_service_run->serv_thd(svc);

    svc->started();
    svc->svc_sem_full.release();
    svc->finish(SVC_finished);
  }
  catch (const Exception& ex)
  {
    // Not much we can do here
    iscLogException("Exception in Service::run():", ex);
  }

  return (THREAD_ENTRY_RETURN)(IPTR) exit_code;
}

Заключение

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

Популярные статьи по теме
Эффект последней строки

Дата: 31 Май 2014

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

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

Дата: 31 Июл 2017

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

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

Дата: 21 Ноя 2018

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

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

Дата: 30 Янв 2019

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

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

Дата: 20 Мар 2017

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

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

Дата: 22 Дек 2018

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

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

Дата: 14 Апр 2016

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

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

Дата: 27 Июн 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 17 Янв 2019

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

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

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

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