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

Коллекция "Awesome header-only C++ libraries"

История написания этой статьи началась с выхода подкаста Cppcast "Cross Platform Mobile Telephony". Из него я узнал о существовании списка "awesome-hpp" в котором перечислено большое количество открытых C++ библиотек, состоящих только из заголовочных файлов.

Этот список заинтересовал меня по двум причинам. Во-первых, это возможность пополнить базу проектов для тестирования нашего анализатора PVS-Studio на современном коде. Многие проекты написаны на C++11, C++14 и C++17. Во-вторых, это возможность написать статью о проверке этих проектов.

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

Зачем проводить анализ

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

Код известных открытых библиотек хорошо отлажен, и вероятность встретить там ошибку намного меньше, чем в аналогичном коде, написанном самостоятельно. Беда в том, что как раз далеко не все библиотеки широко используются и отлажены. И вот здесь и встаёт вопрос оценки их качества.

Чтобы стало понятнее, давайте рассмотрим пример. Возьмём библиотеку JSONCONS.

JSONCONS is a C++, header-only library for constructing JSON and JSON-like data formats such as CBOR.

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

static constexpr uint64_t basic_type_bits = sizeof(uint64_t) * 8;
....
uint64_t* data() 
{
  return is_dynamic() ? dynamic_stor_.data_ : short_stor_.values_;
}
....
basic_bigint& operator<<=( uint64_t k )
{
  size_type q = (size_type)(k / basic_type_bits);
  if ( q ) // Increase common_stor_.length_ by q:
  {
    resize(length() + q);
    for (size_type i = length(); i-- > 0; )
      data()[i] = ( i < q ? 0 : data()[i - q]);
    k %= basic_type_bits;
  }
  if ( k )  // 0 < k < basic_type_bits:
  {
    uint64_t k1 = basic_type_bits - k;
    uint64_t mask = (1 << k) - 1;             // <=
    resize( length() + 1 );
    for (size_type i = length(); i-- > 0; )
    {
      data()[i] <<= k;
      if ( i > 0 )
        data()[i] |= (data()[i-1] >> k1) & mask;
      }
  }
  reduce();
  return *this;
}

Предупреждение анализатора PVS-Studio: V629 Consider inspecting the '1 << k' expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. bigint.hpp 744

Как я понимаю, функция работает с большими числами, которые хранятся как массив 64-битных элементов. Для работы с определёнными битами нужно сформировать 64-битную маску:

uint64_t mask = (1 << k) - 1;

Вот только эта маска формируется некорректно. Поскольку числовой литерал 1 имеет тип int, то при сдвиге его более чем на 31 бит мы получим неопределённое поведение.

Из стандарта:
shift-expression << additive-expression
…
2. The value of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are zero-filled. If E1 has an unsigned type, the value of the result is E1 * 2^E2, reduced modulo one more than the maximum value representable in the result type. Otherwise, if E1 has a signed type and non-negative value, and E1*2^E2 is representable in the result type, then that is the resulting value; otherwise, the behavior is undefined.

Значение переменной mask может получиться каким угодно. Да-да знаю, теоретически из-за UB вообще может произойти что угодно. Но на практике, скорее всего, речь идёт о некорректном результате выражения.

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

А ещё одну такую же ошибку можно видеть в operator>>=.

Риторический вопрос. Стоит ли доверять этой библиотеке?

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

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

template<typename Scalar>
vector<Scalar> operator*(double scalar, const vector<Scalar>& v) {
  vector<Scalar> scaledVector(v);
  scaledVector *= scalar;
  return v;
}

Предупреждение анализатора PVS-Studio: V1001 The 'scaledVector' variable is assigned but is not used by the end of the function. vector.hpp 124

Из-за опечатки возвращается не новый контейнер scaledVector, а исходный вектор. Та же самая ошибка и в операторе деления. Facepalm.

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

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

Как проводить анализ

Хорошо, мы хотим понять качество кода библиотек, но как это сделать? Да, сделать это непросто. Нельзя просто взять и посмотреть код. Вернее, посмотреть-то можно, но это даст мало информации. И тем более такой просмотр вряд ли поможет оценить плотность ошибок в проекте.

Вернёмся к уже упомянутой ранее математической библиотеке Universal. Попробуйте найти ошибку в коде вот этой функции. Собственно, видя сопровождающий комментарий, я не могу пройти мимо этого места :).

// subtract module using SUBTRACTOR: CURRENTLY BROKEN FOR UNKNOWN REASON

template<size_t fbits, size_t abits>
void module_subtract_BROKEN(const value<fbits>& lhs, const value<fbits>& rhs,
                            value<abits + 1>& result) {
  if (lhs.isinf() || rhs.isinf()) {
    result.setinf();
    return;
  }
  int lhs_scale = lhs.scale(),
      rhs_scale = rhs.scale(),
      scale_of_result = std::max(lhs_scale, rhs_scale);

  // align the fractions
  bitblock<abits> r1 = lhs.template nshift<abits>(lhs_scale-scale_of_result+3);
  bitblock<abits> r2 = rhs.template nshift<abits>(rhs_scale-scale_of_result+3);
  bool r1_sign = lhs.sign(), r2_sign = rhs.sign();

  if (r1_sign) r1 = twos_complement(r1);
  if (r1_sign) r2 = twos_complement(r2);

  if (_trace_value_sub) {
    std::cout << (r1_sign ? "sign -1" : "sign  1") << " scale "
      << std::setw(3) << scale_of_result << " r1       " << r1 << std::endl;
    std::cout << (r2_sign ? "sign -1" : "sign  1") << " scale "
      << std::setw(3) << scale_of_result << " r2       " << r2 << std::endl;
  }

  bitblock<abits + 1> difference;
  const bool borrow = subtract_unsigned(r1, r2, difference);

  if (_trace_value_sub) std::cout << (r1_sign ? "sign -1" : "sign  1")
    << " borrow" << std::setw(3) << (borrow ? 1 : 0) << " diff    "
    << difference << std::endl;

  long shift = 0;
  if (borrow) {   // we have a negative value result
    difference = twos_complement(difference);
  }
  // find hidden bit
  for (int i = abits - 1; i >= 0 && difference[i]; i--) {
    shift++;
  }
  assert(shift >= -1);

  if (shift >= long(abits)) {            // we have actual 0 
    difference.reset();
    result.set(false, 0, difference, true, false, false);
    return;
  }

  scale_of_result -= shift;
  const int hpos = abits - 1 - shift;         // position of the hidden bit
  difference <<= abits - hpos + 1;
  if (_trace_value_sub) std::cout << (borrow ? "sign -1" : "sign  1")
    << " scale " << std::setw(3) << scale_of_result << " result  "
    << difference << std::endl;
  result.set(borrow, scale_of_result, difference, false, false, false);
}

Уверен, несмотря на то, что я подсказал про наличие ошибки в этом коде, найти её непросто.

Если не нашли, то вот она. Предупреждение PVS-Studio: V581 The conditional expressions of the 'if' statements situated alongside each other are identical. Check lines: 789, 790. value.hpp 790

if (r1_sign) r1 = twos_complement(r1);
if (r1_sign) r2 = twos_complement(r2);

Классическая опечатка. Во втором условии должна проверяться переменная r2_sign.

В общем, о "ручном" обзоре кода можно забыть. Да, такой путь возможен, но неоправданно трудоёмок.

Что я предлагаю? Очень просто. Используйте статический анализ кода.

Проверьте библиотеки, которые вы собираетесь использовать. Начните смотреть отчёты и достаточно быстро вам всё станет понятно.

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

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

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

Что использовать

Существует множество инструментов статического анализа кода. Я естественно предлагаю использовать анализатор PVS-Studio. Он отлично подходит как для разовой оценки качества кода, так и для регулярного поиска и исправления ошибок.

Вы можете проверить код проектов на языке C, C++, C# и Java. Продукт является проприетарным. Однако, бесплатной триальной лицензии будет более чем достаточно для оценки качества нескольких открытых библиотек.

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

• студентов;
• открытых проектов;
• закрытых проектов (требуется добавление специальных комментариев в код);
• Microsoft MVP.

Заключение

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

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

• Как внедрить статический анализатор кода в legacy проект и не демотивировать команду.
• Причины внедрить в процесс разработки статический анализатор кода PVS-Studio.

Спасибо за внимание, и желаю поменьше багов как в вашем коде, так и в коде используемых библиотек :).

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Andrey Karpov. Why it is important to apply static analysis for open libraries that you add to your project.