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

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

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

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

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

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

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


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

>
>
>
Проверка операционной системы Haiku (се…

Проверка операционной системы Haiku (семейство BeOS) с помощью PVS-Studio. Часть 1

22 Апр 2015

Операционные системы являются одними из самых сложных и крупных проектов в мире программного обеспечения, а значит идеально подходят для демонстрации применения методики статического анализа кода. После проверки Linux Kernel, я вдохновился проанализировать и другие открытые операционные системы.

0317_Haiku_1_ru/image1.png

Введение

Haiku — свободная операционная система для персональных компьютеров, которая нацелена на двоичную совместимость с операционной системой BeOS. Haiku воплощает в себе основные идеи BeOS. Это модульная система, архитектурно решённая как гибридное ядро: микроядерная архитектура, способная динамически подгружать необходимые модули.

Проект для проверки был предложен пользователем, знакомым с продуктом PVS-Studio и нашей работе по проверке open-source проектов. После сравнительно недавней проверки Linux Kernel, я догадывался, с какими проблемами мне придётся столкнуться и описал их в ответном письме. Неожиданно мне предложили содействие в сборке операционной системы и интеграции анализатора. Дополнительно на официальном сайте была доступна очень обширная документация и я решил попробовать.

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

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

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

Предупреждения #1, #2:

V501 There are identical sub-expressions to the left and to the right of the '<' operator: lJack->m_jackType < lJack->m_jackType MediaJack.cpp 783

int __CORTEX_NAMESPACE__ compareTypeAndID(....)
{
  int retValue = 0;
  ....
  if (lJack && rJack)
  {
    if (lJack->m_jackType < lJack->m_jackType)           // <=
    {
      return -1;
    }
    if (lJack->m_jackType == lJack->m_jackType)          // <=
    {
      if (lJack->m_index < rJack->m_index)
      {
        return -1;
      }
      else
      {
        return 1;
      }
    }
    else if (lJack->m_jackType > rJack->m_jackType)
    {
      retValue = 1;
    }
  }
  return retValue;
}

На эту функцию анализатор выдал целых два предупреждения. В обоих случаях хорошо заметна опечатка (когда уже анализатор "тыкнул пальцем", конечно) в именах lJack и rjack.

V575 The 'strchr' function processes value '2112800'. Inspect the second argument. CommandActuators.cpp 1517

extern char    *strchr(const char *string, int character);

SendMessageCommandActuator::
SendMessageCommandActuator(int32 argc, char** argv)
  :
  CommandActuator(argc, argv),
  fSignature((argc > 1) ? argv[1] : "")
{
  ....
  const char* arg = argv[i];
  BString argString(arg);
  const char* equals = strchr(arg, ' = ');  // <=
  ....
}

Функция strchr() возвращает указатель на первое вхождение указанного символа в указанной строке. Символ преобразуется в int, в данном случае ' = ' будет представлен как число 2112800. Скорее всего хотели искать одиночный символ '=', а его код будет 61.

Если хотели найти подстроку " = ", то явно используется не та функция и её следует заменить на вызов strstr().

Предупреждения #3, #4:

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '-' operator. AbstractLayout.cpp 244

bool IsVisible(bool ancestorsVisible) const
{
  int16 showLevel = BView::Private(view).ShowLevel();
  return (showLevel - (ancestorsVisible) ? 0 : 1) <= 0;
}

К сожалению, в данном случае взятие в скобочки переменной 'ancestorsVisible' никак не влияет на порядок вычислений в этом выражении. Поэтому первой по приоритету будет выполняться операция вычитания (из int16 вычитается bool), только потом выполняется тернарный оператор '?:'.

Правильный код:

bool IsVisible(bool ancestorsVisible) const
{
  int16 showLevel = BView::Private(view).ShowLevel();
  return (showLevel - (ancestorsVisible ? 0 : 1) ) <= 0;
}

V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '&&' operator. fnmatch.c 58

#define FOLD(c) \
  ((flags & FNM_CASEFOLD) && ISUPPER ((unsigned char) (c)) \
  ? tolower ((unsigned char) (c)) \
  : (c))

Также я советую авторам проверить порядок выполнения операций в этом макросе и расставить скобки для наглядности.

Предупреждения #5, #6

V562 It's odd to compare 0 or 1 with a value of 0. cmp.c 300

#ifndef same_file
# define same_file(s, t) \
    ((((s)->st_ino == (t)->st_ino) \
     && ((s)->st_dev == (t)->st_dev)) \
     || same_special_file (s, t))
#endif

int
main (int argc, char **argv)
{
  ....
  if (0 < same_file (&stat_buf[0], &stat_buf[1])           // <=
      && same_file_attributes (&stat_buf[0], &stat_buf[1])
      && file_position (0) == file_position (1))
    return EXIT_SUCCESS;
  ....
}

На первый взгляд обычное условие, но "same_file" является макросом, преобразующимся в логическое выражение, как и 'same_file_attributes', в итоге получили странное сравнение "0 < value_of_boolean_type". В языке Си нет типа 'bool'. Выражение справа будет иметь тип 'int', но по своей сути она всегда "истина" или "ложь", поэтому мы и назвали его 'boolean'. И сравнение "0 < boolean_expr" весьма подозрительно.

Аналогичное использование макроса:

  • V562 It's odd to compare 0 or 1 with a value of 0. cmp.c 313

V562 It's odd to compare a bool type value with a value of 18: 0x12 == IsProfessionalSpdif(). CEchoGals_mixer.cpp 533

#define ECHOSTATUS_DSP_DEAD 0x12                          // <=

virtual BOOL IsProfessionalSpdif()                        // <=
{ 
  ....
  return( (....) ? TRUE : FALSE ); 
}

ECHOSTATUS CEchoGals::ProcessMixerFunction
(
  PMIXER_FUNCTION  pMixerFunction,
  INT32 &          iRtnDataSz
)
{
  ....
  case MXF_GET_PROF_SPDIF :
      if ( ECHOSTATUS_DSP_DEAD == IsProfessionalSpdif() ) // <=
      {
        Status = ECHOSTATUS_DSP_DEAD;        
      }
      else
      {
        pMixerFunction->Data.bProfSpdif = IsProfessionalSpdif();
      }
  ....
}

Ещё одно некорректное сравнение с использованием макросов. Функция IsProfessionalSpdif() возвращает TRUE или FALSE, а мы сравниваем возвращаемый результат с числом 0x12.

Предупреждения #7, #8

V583 The '?:' operator, regardless of its conditional expression, always returns one and the same value. impactv.c 520

void Radeon_CalcImpacTVRegisters(....)
{
  ....
  values->tv_hstart =
    internal_encoder ? 
    values->tv_hdisp + 1 - params->mode888 + 12 :
    values->tv_hdisp + 1 - params->mode888 + 12;
  ....
}

Независимо от значения переменной 'internal_encoder', тернарный оператор возвращает одинаковые значения. Необходимо проверить это место на наличие опечаток.

V523 The 'then' statement is equivalent to the 'else' statement. mkntfs.c 1132

static int insert_positioned_attr_in_mft_record(....)
{
  ....
  if (flags & ATTR_COMPRESSION_MASK) {
    hdr_size = 72;
    /* FIXME: This compression stuff is all wrong. .... */
    /* now. (AIA) */
    if (val_len)
      mpa_size = 0; /* get_size_for_compressed_....; */
    else
      mpa_size = 0;
  } else {
  ....
  }
  ....
}

Анализатор напоминает, что необходимо "пофиксить" отложенные места.

Ещё такое место:

  • V523 The 'then' statement is equivalent to the 'else' statement. mkntfs.c 1334

Предупреждения #9, #10

V503 This is a nonsensical comparison: pointer <= 0. Header.cpp 900

extern
char *strstr(const char *string, const char *searchString);

void
TTextControl::MessageReceived(BMessage *msg)
{
  ....
  while (node.GetNextAttrName(buffer) == B_OK) {
    if (strstr(buffer, "email") <= 0)
      continue;
  ....
}

Функция strstr() возвращает указатель на первое вхождение строки "email" в строке 'buffer', если такого соответствия не найдено, то возвращается NULL. Следовательно, в данном случае необходимо с NULL и сравнивать.

Возможное решение:

void
TTextControl::MessageReceived(BMessage *msg)
{
  ....
  while (node.GetNextAttrName(buffer) == B_OK) {
    if (strstr(buffer, "email") == NULL)
      continue;
  ....
}

V512 A call of the 'memcmp' function will lead to underflow of the buffer '"Private-key-format: v"'. dst_api.c 858

dst_s_read_private_key_file(....)
{
  ....
  if (memcmp(in_buff, "Private-key-format: v", 20) != 0)
    goto fail;
  ....
}

Длина сравниваемой строки не совпадает с указанным количеством сравниваемых символов. В строке "Private-key-format: v" 21 символ.

Предупреждения #11, #12

V547 Expression '* r && * r == ' ' && * r == '\t'' is always false. Probably the '||' operator should be used here. selection.c 546

static int
selection_rel(....)
{
  char *r, *rname;
  ....
  while (*r && *r == ' ' && *r == '\t')
    r++;
  ....
}

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

Возможный корректный вариант:

static int
selection_rel(....)
{
  char *r, *rname;
  ....
  while (*r == ' ' || *r == '\t')
    r++;
  ....
}

V590 Consider inspecting the 'path[i] == '/' && path[i] != '\0'' expression. The expression is excessive or contains a misprint. storage_support.cpp 309

status_t
parse_first_path_component(const char *path, int32& length,
               int32& nextComponent)
{
  ....
  for (; path[i] == '/' && path[i] != '\0'; i++);  // <=
  if (path[i] == '\0')  // this covers "" as well
    nextComponent = 0;
  else
    nextComponent = i;
  ....
}

Здесь всё хорошо, но одна проверка является лишней и её стоит удалить. Для сохранения логики работы, достаточно оставить: "for (; path[i] == '/'; i++);".

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

  • V590 Consider inspecting this expression. The expression is excessive or contains a misprint. PoseView.cpp 5773
  • V590 Consider inspecting this expression. The expression is excessive or contains a misprint. Tracker.cpp 1728
  • V590 Consider inspecting the '* ptr == ';' && * ptr != '\0'' expression. The expression is excessive or contains a misprint. pc.c 316

Предупреждение #13, #14

V547 Expression is always true. Probably the '&&' operator should be used here. StatusView.cpp 1397

void
TDragRegion::Draw(BRect)
{
  ....
  if (fDragLocation != kDontDrawDragRegion ||
      fDragLocation != kNoDragRegion)
    DrawDragRegion();
}

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

V547 Expression 'reservedBase < 0' is always false. Unsigned type value is never < 0. agp_gart.cpp 1172

/* address types */
typedef  unsigned long int  __haiku_addr_t;   // <=
typedef __haiku_addr_t    addr_t;             // <=

static status_t
bind_aperture(...., addr_t reservedBase, ....)
{
  ....
  if (status < B_OK) {
    if (reservedBase < 0)                     // <=
      aperture->DeleteMemory(memory);

    return status;
  }
  ....
}

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

Предупреждения #15, #16

V713 The pointer lp was utilized in the logical expression before it was verified against nullptr in the same logical expression. util.c 311

char *
bittok2str(register const struct tok *lp, ....)
{
  ....
  while (lp->s != NULL && lp != NULL) {
    ....
  }
  ....
}

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

while (lp != NULL && lp->s != NULL) {

V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. VideoProducer.cpp 766

int32
VideoProducer::_FrameGeneratorThread()
{
  ....
  err = B_OK;
  // Send the buffer on down to the consumer
  if (wasCached || (err = SendBuffer(buffer, fOutput.source,
      fOutput.destination) != B_OK)) {
        ....
      }
  ....
}

Анализатор обнаружил потенциальную ошибку в выражении, которое, скорее всего, работает не так, как задумывал программист. Планировалось выполнить присваивание "err = SendBuffer()", а результат сравнить с константой 'B_OK', но приоритет оператора '!=' выше, чем у '=', поэтому в переменную 'err' записывается результат логической операции.

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

  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. if_age.c 590
  • V593 Consider reviewing the expression of the 'A = B != C' kind. The expression is calculated as following: 'A = (B != C)'. if_alc.c 954
  • V593 Consider reviewing the expression of the 'A = B >= C' kind. The expression is calculated as following: 'A = (B >= C)'. RAW.cpp 2601

Предупреждения #17, #18

V547 Expression 'nogscale >= 0' is always true. Unsigned type value is always >= 0. tvp3026.c 212

status_t mil2_dac_init (void)
{
  uint32   rfhcnt, nogscale, memconfig;
  ....
  for (nogscale = 1; nogscale >= 0; nogscale--) {           // <=
    int max = -1 + 33.2 * mclk / (nogscale? 1: 4);
    for (rfhcnt = 15; rfhcnt > 0; rfhcnt--) {
      int value = (rfhcnt & 0x0e) * 256 + (rfhcnt & 0x01) * 64;
      LOG(2,("mil2_dac_init factor %d, rfhcnt %2d: %d ?<= %d\n",
        nogscale, rfhcnt, value, max));
      if (value <= max) goto rfhcnt_found;
    }
  }
  ....
}

Скорее всего из-за оператора перехода 'goto' никогда не замечали, что один из циклов "вечный", т.к. при проверке "nogscale >= 0" декремент беззнаковой переменной можно делать бесконечно.

V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. if_ae.c 1670

#define  AE_IDLE_TIMEOUT 100

static void
ae_stop_rxmac(ae_softc_t *sc)
{
  ....
  /*
   * Wait for IDLE state.
   */
  for (i = 0; i < AE_IDLE_TIMEOUT; i--) {
    val = AE_READ_4(sc, AE_IDLE_REG);
    if ((val & (AE_IDLE_RXMAC | AE_IDLE_DMAWRITE)) == 0)
      break;
    DELAY(100);
  }
  ....
}

Почему-то значение счётчика в этом цикле изменяется не в ту сторону, логичнее делать инкремент переменной 'i', чтобы ожидание длилось максимум 100 итераций, а не в миллионы раз больше.

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

  • V621 Consider inspecting the 'for' operator. It's possible that the loop will be executed incorrectly or won't be executed at all. if_ae.c 1706

Предупреждение #19, #20

V646 Consider inspecting the application's logic. It's possible that 'else' keyword is missing. Filter.cpp 760

uchar
Scaler::Limit(intType value)
{
  if (value < 0) {
    value = 0;
  } if (value > 255) {
    value = 255;
  }
  return value;
}

В этой функции нет серьёзной ошибки, но код плохо оформлен. Необходимо добавить ключевое слово 'else', либо разместить условия на одном уровне.

V640 The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. strftime.c 1263

#define DO_NUMBER(d, v) \
    digits = width == -1 ? d : width; \
    number_value = v; goto do_number

size_t
my_strftime (s, maxsize, format, tp extra_args)
{
  ....
  if (modifier == L_('O'))
    goto bad_format;
  else
    DO_NUMBER (1, tp->tm_year + TM_YEAR_BASE);
  ....
}

Макросы и так доставляют неудобства при отладке, так ещё и являются источником вот таких ошибок: макрос 'DO_NUMBER' раскрывается в несколько строк, но только первая из них будет частью условного оператора, последующие же операторы будут выполняться независимо от условия.

Аналогично неправильно макрос используется здесь:

  • V640 The code's operational logic does not correspond with its formatting. The second statement will always be executed. It is possible that curly brackets are missing. strftime.c 1267

Заключение

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

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

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

Дата: 22 Окт 2018

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

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

Дата: 22 Дек 2018

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

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

Дата: 31 Июл 2017

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

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

Дата: 27 Июн 2017

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

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

Дата: 17 Янв 2019

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

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

Дата: 30 Янв 2019

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

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

Дата: 31 Май 2014

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

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

Дата: 19 Май 2017

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

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

Дата: 14 Апр 2016

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

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

Дата: 20 Мар 2017

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

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

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

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

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