Главная
Блог разработчиков phpBB
 
+ 17 предустановленных модов
+ SEO-оптимизация форума
+ авторизация через соц. сети
+ защита от спама

Как мы пытаемся продать PVS-Studio в Google либо очередные ошибки в Chromium

Anna | 24.06.2014 | нет комментариев

PVS-Studio интегрируется в Ninja для проверки Chomium.

Когда мы пишем статьи про проверки каких-либо планов с поддержкой PVS-Studio, то, как правило, у нас прибавляется заказчиков. Здесь все Добросовестно. Программисты не любят рекламу, но с охотой отзываются на увлекательные материалы, которые легко проверить. Следственно мы не рекламируем свой инструмент, а легко показываем, что он может. Впрочем, правда мы проверили код Chromium теснее три раза и трижды находили в нем ошибки, ордера с почтой в google.com в моей почте до сих пор нет. От того что мне увлекательно, что я делаю не так, и отчего Google пока не использует PVS-Studio, я решил написать очередную статью.

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

Кстати эта статья опубликована и на английском языке. Если захотите переслать ее англоязычным коллегам – пожалуйста, дайте им ссылку вот на это.

Хотите узнать, отчего разрабатывать Chromium трудно и вдалеке не всякий инструмент для программистов может быть использован в плане Chromium? Тогда читаем…

Разработчики Chromium не любят Visual Studio, не применяют Makefile, но при этом у них фантастически добротный код. Как же так?

Разработка планов типа Chromium Исключительно трудна. На самом деле, мне даже немножко несуразно писать «планов типа Chromium», так как других планов сходственного яруса я не знаю. Нет, ясно, что есть ядро Linux, есть среда Visual Studio и много еще крупных и больших планов. Но лично мне довелось пока «постоять рядом» только с Chromium. И то, что я видел – дюже увлекательно любому программисту, так как там есть чему поучиться.

Скажем, при разработке Chromium не дюже-то энергично, оказывается, применяют Visual Studio. Повод примитивна – Chromium содержит большое число планов, и IDE от Microsoft искренне говоря «не тянет» такое число. «Ага!» — сказали грозные линуксоиды… «Еще бы!!!» Но при разработке Chromium не применяют и линуксовые Makefile. Ровно по той же самой причине – типовой GNU make «не тянет» такое число планов и работает слишком длинно.

Что же применяют разработчики Chromium? Это сборочная система GYP (Generate Your Projects). С ее поддержкой дозволено генерировать либо файлы .vcxproj (MSBuild/Visual C ), либо файлы системы Ninja (это такой крепко упрощенный и стремительный makefile). Следственно для того, Дабы регулярно применять какой-нибудь статический обзор, нужно интегрировать его именно в эту сборочную систему. Чем мы и занялись для того, Дабы продать PVS-Studio в Google.

План Chromium знаменателен тем, что размер его начального кода на языках C/C , рассматривая сторонние библиотеки (англ. third party library), превышает 500МБ, а всякое метаморфоза кода проверяется десятками тысяч механических тестов параллельно на сотнях тестовых компьютеров разных архитектур и конфигураций. Дозволено подметить и высокую скорость разработки в данном плане: в 2012-м году в репозиторий Chromium было сделано больше 48 тысяч ревизий больше чем 900 уникальными авторами, что соответствует в среднем одной ревизии всякие 11 минут и одной ревизии в неделю от всякого энергичного участника плана.

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

Правда PVS-Studio и не является open source планом, в эластичности нам не откажешь. Следственно мы захотели показать на примере Chromium, что можем встроиться в его сборочную систему и проверять такой огромный план без задач.

Как встроить PVS-Studio в сборочную систему Chromium для регулярных проверок?

Всеобщие данные о тезисах работы PVS-Studio

В дистрибутиве PVS-Studio дозволено выделить2 основных компонента: command-line анализатор PVS-Studio.exe и IDE плагин, интегрирующий данный command-line анализатор в одну из поддерживаемых сред разработки (Microsoft Visual Studio и Embarcadero RAD Studio).

Command-line анализатор по тезису своей работы дюже схож с компилятором — он вызывается для всякого из проверяемых файлов с параметрами, включающими в себя, в том числе, и подлинные параметры компиляции этого файла. После этого анализатор вызывает нужный ему внешний препроцессор (в соответствии с компилятором, используемым при сборке проверяемого файла) и изготавливает непринужденный обзор полученного препроцессированного временного файла (i-файла), т.е. файла, в котором раскрыты все include и define директивы.

Но применение анализатора PVS-Studio.exe не ограничивается IDE плагинами. Как теснее было сказано выше, command-line анализатор, дюже близок в своём применении непринужденно к компилятору. Соответственно, его вызов дозволено встроить, при необходимости, и напрямую в сборочную систему, наравне с самим компилятором. Скажем, если ваш план собирается в IDE Eclipse с поддержкой gcc, вы можете встроить проверку PVS-Studio в ваши makefile’ы.

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

Проверка плана Chromium статическим анализатором PVS-Studio.exe

Как написано выше, Chromium разрабатывается с поддержкой сборочной системы GYP (Generate Your Projects), разрешающей получить нативные проектные файлы для разных ОС и компиляторов. Т.к. на данный момент анализатор PVS-Studio поддерживает только ОС Windows, разглядим допустимые варианты сборки Chromium с поддержкой компилятора Visual C 10. Данный компилятор (cl.exe) поставляется в составе интегрированной среды разработки Visual Studio, а также может быть установлен отдельно из бесплатного пакета Windows SDK.

Применение проектных файлов MSBuild

Применяемая в Chromium система GYP разрешает получить проектные файлы MSBuild (vcxproj) для сборки Chromium с поддержкой компилятора Visual C (cl.exe). Сборочная система MSBuild является частью пакета .NET Framework, являющегося одним из стандартных компонентов ОС семейства Windows.

Особенно простым методом проверки плана Chromium анализатором PVS-Studio будет применение «родного» для него IDE плагина среды Visual Studio. Проектные файлы MSBuild дозволено открыть для проверки в среде разработки Visual Studio. При этом IDE плагин PVS-Studio механически соберёт всю нужную информацию о всяком из файлов плана и запустит для них статический анализатор PVS-Studio.exe. Подметим, что бесплатная версия среды разработки Visual Studio Express Edition не поддерживает работу с IDE плагинами.

Сборку и проверку проектных файлов vcxproj дозволено также произвести напрямую, без применения среды Visual Studio, с поддержкой сборочной системы MSBuild (а вернее, применяя command-line утилиту MSBuild.exe). Для проверки планов статическим анализатором в таком режиме нужно будет напрямую интегрировать в всякий расчетный файл вызов command-line анализатора PVS-Studio.exe (дозволено также и импортировать во все проектные файлы всеобщий props файл, содержащий сходственный вызов анализатора).

Стоит подметить, что правда MSBuild и разрешает напрямую из своих сборочных скриптов (которыми, в том числе, и являются проектные файлы vcxproj) вызывать исполняемые файлы, вызов таких сборочных инструментов, как компилятор и компоновщик в стандартных планах осуществляется с поддержкой особых подключаемых модулей-обёрток (называемых в терминологии MSBuild сборочными задачами – Build Tasks). Дистрибутив PVS-Studio содержит сходственный модуль сборочной задачи для сценариев MSBuild, а также использующий его Props (property sheet) файл, тот, что дозволено напрямую импортировать в типовые планы vcxproj для интеграции в них статического обзора.

Применение проектных файлов Ninja

Сборка Chromium под Windows с поддержкой компилятора cl.exe допустима и с поддержкой скриптов сборочной системы Ninja, которые также дозволено получить генератором планов GYP.

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

В случае с файлами системы Ninja сходственный метод интеграции оказывается затруднённым тем, что собираемые файлы жёстко прописываются в авто-сгенерированных файлах *.ninja как зависимости для obj файлов. В связи с этим, для интеграции обзора на данном шаге сборки нужно будет модифицировать соответствующие этому шагу правила сборки (описанные в всеобщем файле build.ninja): это сс и cxx, т.к. именно эти правила применяются при обходе всех начальных файлов.

На данный момент нам не удалось напрямую добавить в правила cc и cxx вызов PVS-Studio.exe. Правила сборки системы Ninja разрешают применять для определения команды к выполнению только одну переменную command. При этом, в соответствии с документацией, переменная command может содержать и несколько команд, разделённых символами &&.Впрочем, если добавить к присутствующему правилу, как скажем:

command = ninja -t msvc -e $arch -- $cc /nologo /showIncludes /FC 
@$out.rsp /c $in /Fo$out /Fd$pdbname

вызов PVS-Studio.exe:

PVS = "PVS-Studio.exe"
...
command = ninja -t msvc -e $arch -- $cc /nologo /showIncludes /FC 
@$out.rsp /c $in /Fo$out /Fd$pdbname && $PVS –cfg "c:\test.cfg"

то такой вызов понимается, как часть доводов для процесса ninja, и, соответственно, передаётся при вызове –t msvs как довод в cl.exe ($cc). Подобно, если разместить вызов $PVS в предисловие строки, то остальные параметры позже && передаются в качестве доводов теснее в PVS-Studio.exe.

Безусловно, для обхода данного ограничения дозволено написать программу-обёртку, которая по очереди вначале вызовет ninja, а после этого PVS-Studio.exe, и прописать вызов этой обёртки в предисловие переменной command для волнующих нас сборочных правил (cc и cxx). Это мы и сделали для того, Дабы проверить Chromium с поддержкой PVS-Studio таким образом.

Особенности применения Command-line анализатора PVS-Studio.exe при прямой интеграции статического обзора в сценарии сборочной системы.

Основное, что нужно помнить при применении PVS-Studio.exe в режиме прямой интеграции со сборочной системой (т.е. без IDE плагина) — это надобность произвести препроцессирование всякого проверяемого начального файла перед непосредственным выполнением обзора. PVS-Studio.exe должен в качестве одного из параметров своего запуска получить флаг cl-params, позже которого нужно передать все «подлинные» параметры компиляции данного файла. PVS-Studio.exe сам запустит внешний препроцессор (скажем, cl.exe), добавив к данным параметрам флаги, руководящие препроцессором (флаг /P в случае cl.exe).

Впрочем, из-за отличий в поведении препроцессора и компилятора допустима и обстановка, в которой флагов компиляции может оказаться неудовлетворительно для правильного препроцесирования начального C/C файла. В частности, препроцессирование может оказаться немыслимым, если в переданных препроцессору include путях отсутствует путь до директории, содержащей заголовочный файл, применяемый как precompiled заголовок. В такой обстановки компиляция будет проходить удачно (безусловно, если теснее был сгенерирован указанный компилятору pch файл), но препроцессор будет завершать работу с оплошностью «cannot open include file».

IDE плагин PVS-Studio для разрешения данной задачи в случае применения в файле precompiled заголовка сканирует все файлы плана, содержащего проверяемый файл, и добавляет в include пути директорию файла, используемого для генерации требуемого pch (которых в плане может быть и несколько). Для правильной же работы PVS-Studio.exe в режиме прямой интеграции нужно обеспечить корректность работы препроцессора, также передавая данный путь в числе других параметров компиляции. Это дозволено сделать, добавив в перечень передаваемых анализатору доводов компилятора ещё один –I (/I) параметр с соответствующей директорией.

План Chromium содержит несколько сотен сходственных файлов, т.е. файлов, использующих precompiled заголовки, к которым при их компиляции в Includes не передаётся путь до папки, содержащей сами h файлы, из которых эти заголовки были получены. Для правильной проверки этих файлов анализатором PVS-Studio в режиме прямой интеграции (т.е. без применения плагина) нужно модифицировать сборочную систему описанным выше образом перед запуском обзора.

Но дозволено поступить проще. Скажем, мы легко отключили precompiled headers при сборке Chromium для интеграции PVS-Studio в сборочную систему.

Что делать с полученным в итоге интеграции логом проверки?

В итоге такой интеграции будет получен отчет в так называемом «сыром» (raw) виде. Его дозволено открыть в нашей же утилите PVS-Studio Standalone (про которую я писал тут). И начать трудиться в полновесной среде с навигацией и прочими удобствами.

Резюме по интеграции PVS-Studio в сборочную систему Chromium

Выходит, как же выглядит интеграция PVS-Studio в сборочную систему Chromium?

  1. Отключаем precompiled headers.
  2. Генерируем планы Ninja.
  3. В планах Ninja вызывается особая утилита PVS-Studio Wrapper (не идет в дистрибутиве PVS-Studio), которая теснее вызывает PVS-Studio.
  4. Итог проверки – сырой лог – открываем с поддержкой PVS-Studio Standalone и трудимся с обнаруженными ошибками.

А сейчас переходим ко 2-й части статьи – примерам найденных ошибок.

Примеры найденных ошибок

Целью следующий проверки Chromium был не поиск ошибок, а осваивание новой системы сборки. Вернее встраивание PVS-Studio в нее. Следственно Андрей Карпов просмотрел диагностические сообщения дюже и дюже поверхностно. Тем не менее, он всё равно кое-что углядел и прислал мне несколько фрагментов кода, снабдив их комментариями. То, что даже стремительный просмотр разрешает обнаружить ошибки не изумительно для планов такого размера как Chromium. Как бы они не были отлично написаны, в них неизменно есть ошибки. Тем больше план Chromium стремительно прогрессирует и обрастает новыми функциями и библиотеками.

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

Да, хочется напомнить, что это вдалеке не первая проверка Chromium:

Именно следственно, трудно уговорить Андрея уделить просмотру ошибок огромнее времени. Во-первых, теснее не так увлекательно. Во-вторых, полновесно проанализировать отчёт могут только разработчики Chromium. Делать обзор в одиночку, тем больше, не будучи приятелем с планом и библиотеками, неподъемный труд. Да и вообще, план дозволено (и необходимо) проверять всякий день, а не раз в полгода. Но это теснее на совести сообщества, которое работает над становлением плана Chromium и отдельных библиотек.

Не там поставленная скобка (параноики, могут усмотреть в этом закладку :)

static SECStatus
ssl3_SendEncryptedExtensions(sslSocket *ss)
{
  static const unsigned char P256_SPKI_PREFIX[] = {
    0x30, 0x59, 0x30, 0x13, 0x06, 0x07, 0x2a, 0x86,
    0x48, 0xce, 0x3d, 0x02, 0x01, 0x06, 0x08, 0x2a,
    0x86, 0x48, 0xce, 0x3d, 0x03, 0x01, 0x07, 0x03,
    0x42, 0x00, 0x04
  };
  ....
  if (.... ||
      memcmp(spki->data, P256_SPKI_PREFIX,
             sizeof(P256_SPKI_PREFIX) != 0))
  {
    PORT_SetError(SSL_ERROR_INVALID_CHANNEL_ID_KEY);
    rv = SECFailure;
    goto loser;
  }
  ....
}

Предупреждение PVS-Studio (библиотека Network Security Services): V526 The ‘memcmp’ function returns 0 if corresponding buffers are equal. Consider examining the condition for mistakes. ssl3con.c 10533

Из-за ненормально поставленной скобки функция memcmp() сопоставляет один байт.

Выражение «sizeof(P256_SPKI_PREFIX) != 0» неизменно правдиво. То-есть значение этого выражения равно 1.

Верная проверка должна быть такой:

if (.... ||
    memcmp(spki->data, P256_SPKI_PREFIX,
           sizeof(P256_SPKI_PREFIX)) != 0)

Переменая ‘i’ схожа на 1.

void SkCanvasStack::pushCanvas(....) {
  ....
  for (int i = fList.count() - 1; i > 0; --i) {
    SkIRect localBounds = canvasBounds;
    localBounds.offset(origin - fCanvasData[i-1].origin);

    fCanvasData[i-1].requiredClip.op(localBounds,
                                     SkRegion::kDifference_Op);
    fList[i-i]->clipRegion(fCanvasData[i-1].requiredClip);
  }
  ....
}

Слабо подметить подозрительное? :) Анализатору нет.

Предупреждение PVS-Studio (библиотека Skia Graphics Engine): V501 There are identical sub-expressions to the left and to the right of the ‘-’ operator: i — i SkCanvasStack.cpp 38

В качестве индекса несколько раз применяется выражение [i — 1]. А в одном месте, написано [i-i]. Это дюже схоже на опечатку. Скорее каждого, тут тоже должна вычитаться единица.

«Одноразовый» цикл

Code*Code::FindFirstCode() {
  ASSERT(is_inline_cache_stub());
  DisallowHeapAllocation no_allocation;
  int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
  for (RelocIterator it(this, mask); !it.done(); it.next())
  {
    RelocInfo* info = it.rinfo();
    return
      Code::GetCodeFromTargetAddress(info->target_address());
  }
  return NULL;
}

Предупреждение PVS-Studio (Chromium): V612 An unconditional ‘return’ within a loop. objects.cc 10326

Цикл будет закончен сразу позже первой итерации. Скорее каждого, тут позабыли условие. Допустимо, код правилен. Но всё равно он крайне подозрителен и на него стоит обратить внимание.

А вот ещё схожий цикл:

int SymbolTable::Symbolize() {
  ....
  if (socketpair(AF_UNIX, SOCK_STREAM,
                 0, child_fds[i]) == -1)
  {
    for (int j = 0; j < i; j  ) {
      close(child_fds[j][0]);
      close(child_fds[j][1]);
      PrintError("Cannot create a socket pair");
      return 0;
    }
  }
  ....
}

Предупреждение PVS-Studio (библиотека tcmalloc): V612 An unconditional ‘return’ within a loop. symbolize.cc 154

Кажется, тут фигурная скобка закрывается не там, где нужно. По каждой видимости, код должен был выглядеть так:

if (socketpair(AF_UNIX, SOCK_STREAM,
               0, child_fds[i]) == -1)
{
  for (int j = 0; j < i; j  ) {
    close(child_fds[j][0]);
    close(child_fds[j][1]);
  }
  PrintError("Cannot create a socket pair");
  return 0;
}

Предисловие и конец — одно и тоже

class CONTENT_EXPORT EventPacket {
  ....
  InputEvents::const_iterator begin() const
    { return events_.end(); }
  InputEvents::const_iterator end() const
    { return events_.end(); }
  ....
protected:
  InputEvents events_;
  ....
};

Предупреждение PVS-Studio (Chromium): V524 It is odd that the body of ‘end’ function is fully equivalent to the body of ‘begin’ function. event_packet.h 36

Функции beign() и end() возвращают одно и тоже. Вероятно, функция begin() должна выглядеть так:

InputEvents::const_iterator begin() const
  { return events_.begin(); }

Неустойчивая функция rdtsc()

__inline__ unsigned long long int rdtsc()
{
#ifdef __x86_64__
  unsigned int a, d;
  __asm__ volatile ("rdtsc" : "=a" (a), "=d" (d));
  return (unsigned long)a | ((unsigned long)d << 32);
#elif defined(__i386__)
  unsigned long long int x;
  __asm__ volatile ("rdtsc" : "=A" (x));
  return x;
#else
#define NO_CYCLE_COUNTER
  return 0;
#endif
}

Предупреждение PVS-Studio (библиотека SMHasher): V629 Consider inspecting the ‘(unsigned long) d << 32′ expression. Bit shifting of the 32-bit value with a subsequent expansion to the 64-bit type. Platform.h 78

Эта функция Отчасти работает. Впрочем, она может подвести, если тип long окажется 32-битным. Вот тут “(unsigned long)d << 32″ произойдёт переполнение. Дабы этого избежать, код нужно модифицировать дальнейшим образом:

return (unsigned long long)a |
       ((unsigned long long)d << 32);

Эпохальный и страшный break

Ключевое слово ‘break’ непрерывно забывают при применении оператора выбора. Позабыть его может кто желательно и когда желательно. Не теряйте неусыпность.

1-й пример:

static v8::Handle<v8::Value>
toV8Object(....)
{
  switch (extension->name()) {
    ....
    case WebGLExtension::WebGLCompressedTextureATCName:
      extensionObject = toV8(....);
      referenceName = "webGLCompressedTextureATCName";
    case WebGLExtension::WebGLCompressedTexturePVRTCName:
      extensionObject = toV8(....);
      referenceName = "webGLCompressedTexturePVRTCName";
      break;
  }
  ....
}

Предупреждения PVS-Studio (библиотека WebKit):

  • V519 The ‘extensionObject’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 222, 225. V8WebGLRenderingContextCustom.cpp 225
  • V519 The ‘referenceName’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 223, 226. V8WebGLRenderingContextCustom.cpp 226

Обговаривать тут нечего. Позабыли ‘break’. Точка.

2-й пример:

bool ScriptDebugServer::executeSkipPauseRequest(....)
{
  const char* v8MethodName;
  switch (request)
  {
    case ScriptDebugListener::NoSkip:
      return false;
    case ScriptDebugListener::Continue:
      return true;
    case ScriptDebugListener::StepInto:
      v8MethodName = stepIntoV8MethodName;
    case ScriptDebugListener::StepOut:
      v8MethodName = stepOutV8MethodName;
  }
  ....
}

Предупреждение PVS-Studio (библиотека WebKit): V519 The ‘v8MethodName’ variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 412, 414. ScriptDebugServer.cpp 414

Андрей Карпов прислал ещё несколько фрагментов кода. Но они не дюже увлекательные, так что я их пропущу.

Неинтересным я считаю вот такие ляпы:

int linux_get_device_address (....,
  uint8_t *busnum, uint8_t *devaddr,
  ....)
{
  ....
  *busnum = __read_sysfs_attr(ctx, sys_name, "busnum");
  if (0 > *busnum)
    return *busnum;
  ....
}

Предупреждение PVS-Studio (библиотека LibUSB): V547 Expression ’0 > * busnum’ is always false. Unsigned type value is never < 0. linux_usbfs.c 620

Указатель ‘busnum’ ссылается на беззнаковую переменную, имеющую тип uint8_t. Это значит, что условие (0 > *busnum) никогда не выполнится.

То есть самая настоящая оплошность. Но тоскливая. Так что я заканчиваю с изложением ошибок.

Завершение, либо обращение к разработчикам Chromium

Вы видите, что PVS-Studio регулярно находит ошибки в коде Chromium. Вы видите, что сейчас PVS-Studio легко встроить в сборочную систему. Мы готовы различно вам в этом помогать, если что-то не будет получаться. Дело за вами – давайте совместно будем повышать качество Chromium. Внедряйте PVS-Studio в своем плане!

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

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

 

Источник: programmingmaster.ru

 

Оставить комментарий
Форум phpBB, русская поддержка форума phpBB
Рейтинг@Mail.ru 2008 - 2017 © BB3x.ru - русская поддержка форума phpBB