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

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

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

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

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

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

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


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

>
>
>
ChakraCore: проверка JavaScript-движка …

ChakraCore: проверка JavaScript-движка для Microsoft Edge

22 Янв 2016

В декабре 2015 года на конференции JSConf US разработчики объявили, что планируют открыть исходный код ключевых компонентов JavaScript-движка Chakra, работающего в Microsoft Edge. Недавно исходный код ChackraCore под MIT лицензией опубликовали в соответствующем репозитории на GitHub. В статье я расскажу, что удалось найти интересного в проекте с помощью статического анализатора PVS-Studio.

0370_ChakraCore_ru/image1.png

Введение

ChakraCore это базовая составляющая Chakra, высокопроизводительный движок JavaScript, который запускает приложения Microsoft Edge и Windows, написанные на HTML/CSS/JS. ChakraCore поддерживает JIT-компиляцию на JavaScript для x86/x64/ARM, сборку мусора и широкий спектр самых последних возможностей JavaScript.

PVS-Studio - это статический анализатор для выявления ошибок в исходном коде программ, написанных на языках С, C++ и C#. Инструмент PVS-Studio предназначен для разработчиков современных приложений и интегрируется в среды Visual Studio 2010-2015.

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

Разные ошибки

0370_ChakraCore_ru/image2.png

V501 There are identical sub-expressions 'this->propId == Js::PropertyIds::_superReferenceSymbol' to the left and to the right of the '||' operator. diagobjectmodel.cpp 123

IDiagObjectModelDisplay * ResolvedObject::CreateDisplay()
{
  ....
  if (this->isConst ||
    this->propId == Js::PropertyIds::_superReferenceSymbol ||
    this->propId == Js::PropertyIds::_superReferenceSymbol)
  {
      pOMDisplay->SetDefaultTypeAttribute(....);
  }
  ....
}

В условии присутствует две одинаковые проверки. Возможно, при написании кода, в меню IntelliSense случайно была выбрана такая же константа, например, вместо "Js::PropertyIds:: _superCtorReferenceSymbol".

V501 There are identical sub-expressions 'GetVarSymID(srcIndexOpnd->GetStackSym())' to the left and to the right of the '==' operator. globopt.cpp 20795

void GlobOpt::EmitMemop(....)
{
  ....
  IR::RegOpnd *srcBaseOpnd = nullptr;
  IR::RegOpnd *srcIndexOpnd = nullptr;
  IRType srcType;
  GetMemOpSrcInfo(...., srcBaseOpnd, srcIndexOpnd, srcType);
  Assert(GetVarSymID(srcIndexOpnd->GetStackSym()) ==        // <=
         GetVarSymID(srcIndexOpnd->GetStackSym()));         // <=
  ....
}

Ещё два одинаковых сравнения. Скорее всего, хотели сравнить "srcIndexOpnd->GetStackSym()" с " srcBaseOpnd ->GetStackSym()".

V517 The use of 'if (A) {...} else if (A) {...}' pattern was detected. There is a probability of logical error presence. Check lines: 3220, 3231. lower.cpp 3220

bool Lowerer::GenerateFastBrSrEq(....,
                                 IR::RegOpnd * srcReg1,
                                 IR::RegOpnd * srcReg2,
                                 ....)
{
  if (srcReg2 && IsConstRegOpnd(srcReg2))
  {
    ....
  }
  else if (srcReg1 && IsConstRegOpnd(srcReg1))
  {
    ....
  }
  else if (srcReg2 && (srcReg2->m_sym->m_isStrConst))
  {
    ....
  }
  else if (srcReg1 && (srcReg1->m_sym->m_isStrConst))       // <=
  {
    ....
  }
  else if (srcReg2 && (srcReg2->m_sym->m_isStrEmpty))
  {
    ....
  }
  else if (srcReg1 && (srcReg1->m_sym->m_isStrConst))       // <=
  {
    ....
  }

  return false;
}

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

Скорее всего, последние два условия хотели написать так:

....
else if (srcReg2 && (srcReg2->m_sym->m_isStrEmpty))
{
  ....
}
else if (srcReg1 && (srcReg1->m_sym-> m_isStrEmpty))       // <=
{
  ....
}

V713 The pointer scriptContext was utilized in the logical expression before it was verified against nullptr in the same logical expression. diaghelpermethodwrapper.cpp 214

template <bool doCheckParentInterpreterFrame>
void HandleHelperOrLibraryMethodWrapperException(....)
{
  ....
  if (!exceptionObject->IsDebuggerSkip() ||
    exceptionObject == scriptContext->GetThreadContext()->.... ||
    exceptionObject == scriptContext->GetThreadContext()->.... ||
    !scriptContext)    // <=
  {
    throw exceptionObject->CloneIfStaticExceptionObject(....);
  }
  ....
}

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

V570 The 'this->isInlined' variable is assigned to itself. functioncodegenjittimedata.h 625

void SetupRecursiveInlineeChain(
    Recycler *const recycler,
    const ProfileId profiledCallSiteId)
{
  if (!inlinees)
  {
    inlinees = RecyclerNewArrayZ(....);
  }
  inlinees[profiledCallSiteId] = this;
  inlineeCount++;
  this->isInlined = isInlined;   // <=
}

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

Ещё одно место присваивания переменной самой себе:

  • V570 The 'sym->m_isTaggableIntConst' variable is assigned to itself. linearscan.cpp 3170

V590 Consider inspecting the 'sub[i] != '-' && sub[i] == '/'' expression. The expression is excessive or contains a misprint. rl.cpp 1388

const char *
stristr
(
  const char * str,
  const char * sub
)
{
  ....
  for (i = 0; i < len; i++)
  {
    if (tolower(str[i]) != tolower(sub[i]))
    {
      if ((str[i] != '/' && str[i] != '-') ||
            (sub[i] != '-' && sub[i] == '/')) {              / <=
           // if the mismatch is not between '/' and '-'
           break;
      }
    }
  }
  ....
}

Анализатор обнаружил, что часть условного выражения (sub[i] != '-') не влияет на результат проверки. Чтобы в этом убедиться, построим таблицу истинности. Скорее всего здесь имеет место какая-то опечатка, но каким должен быть правильный код, я затрудняюсь ответить.

0370_ChakraCore_ru/image3.png

V603 The object was created but it is not being used. If you wish to call constructor, 'this->StringCopyInfo::StringCopyInfo(....)' should be used. stringcopyinfo.cpp 64

void StringCopyInfo::InstantiateForceInlinedMembers()
{
    AnalysisAssert(false);

    StringCopyInfo copyInfo;
    JavascriptString *const string = nullptr;
    wchar_t *const buffer = nullptr;

    (StringCopyInfo());                     // <=
    (StringCopyInfo(string, buffer));       // <=
    copyInfo.SourceString();
    copyInfo.DestinationBuffer();
}

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

Правильным вариантом было бы создать функцию инициализации и вызывать её из конструкторов и в этом месте.

V610 Undefined behavior. Check the shift operator '<<'. The left operand '-1' is negative. constants.h 39

class Constants
{
public:
  ....
  static const int Int31MinValue = -1 << 30;
  ....
};

Согласно современному стандарту языка C++, сдвиг отрицательного числа приводит к неопределённому поведению.

V557 Array overrun is possible. The value of 'i' index could reach 8. rl.cpp 2375

enum TestInfoKind::_TIK_COUNT = 9

const char * const TestInfoEnvLstFmt[] =
{
   " TESTFILE=\"%s\"",
   " BASELINE=\"%s\"",
   " CFLAGS=\"%s\"",
   " LFLAGS=\"%s\"",
   NULL,
   NULL,
   NULL,
   NULL    // <= TestInfoEnvLstFmt[7]
};

void
WriteEnvLst
(
   Test * pDir, TestList * pTestList
)
{
  ....
  // print the other TIK_*
  for(int i=0;i < _TIK_COUNT; i++) {
    if (variants->testInfo.data[i] && TestInfoEnvLstFmt[i]){// <=
       LstFilesOut->Add(TestInfoEnvLstFmt[i],               // <=
                        variants->testInfo.data[i]);
    }
    ....
  }
  ....
}

Анализатор обнаружил выход индекса за пределы массива. Дело в том, что цикл for() выполняет 9 итераций, а в массиве "TestInfoEnvLstFmt[]" всего 8 элементов.

Скорее всего забыли добавить ещё один NULL в конце:

const char * const TestInfoEnvLstFmt[] =
{
   " TESTFILE=\"%s\"",
   " BASELINE=\"%s\"",
   " CFLAGS=\"%s\"",
   " LFLAGS=\"%s\"",
   NULL,
   NULL,
   NULL,
   NULL    // <= TestInfoEnvLstFmt[7]
   NULL    // <= TestInfoEnvLstFmt[8]
};

Но может быть и пропустили какую-нибудь строку в середине массива!

Опасные указатели

0370_ChakraCore_ru/image4.png

Диагностика V595 находит участки кода, где выполняется разыменование указателя до его проверки на ноль. Обычно в проверяемых проектах всегда находится некоторое количество таких предупреждений. В нашей базе ошибок она занимает первое место по количеству найденных недочётов (см. примеры). Но дело в том, что диагностики V595 несколько скучны, чтобы выписывать много примеров из какого-то проекта. Также проверка и разыменование указателя могут находится довольно далеко в коде функции: в десятках или даже сотнях строк друг от друга, что усложняет описание ошибки в статье.

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

V595 The 'instrLd' pointer was utilized before it was verified against nullptr. Check lines: 1823, 1831. flowgraph.cpp 1823

IR::Instr *
FlowGraph::PeepTypedCm(IR::Instr *instr)
{
 ....
 if (instrLd && !instrLd->GetSrc1()->IsEqual(instr->GetDst()))
 {
   return nullptr;
 }
 
 if(instrLd2 && !instrLd2->GetSrc1()->IsEqual(instrLd->GetDst()))
 {
   return nullptr;
 }
 ....
}

Обратите внимание на указатель с именем "instrLd". В первом условии разыменование этого указателя выполняется в паре с проверкой на ноль, но во втором условии это сделать забыли, поэтому может возникнуть ситуация, когда произойдёт разыменование нулевого указателя.

V595 The 'src2Val' pointer was utilized before it was verified against nullptr. Check lines: 9717, 9725. globopt.cpp 9717

bool GlobOpt::TypeSpecializeIntBinary(....)
{
  ....
  bool isIntConstMissingItem = src2Val->GetValueInfo()->....

  if(isIntConstMissingItem)
  {
      isIntConstMissingItem = Js::SparseArraySegment<int>::....
  }

  if (!src2Val || !(src2Val->GetValueInfo()->IsLikelyInt()) ||
      isIntConstMissingItem)
  {
      return false;
  }
  ....
}

Указатель "src2Val" используется в начале функции, но потом разработчики активно начали проверять, является ли указатель равным нулю.

V595 The 'm_lastInstr' pointer was utilized before it was verified against nullptr. Check lines: 214, 228. irbuilderasmjs.cpp 214

void
IRBuilderAsmJs::AddInstr(IR::Instr * instr, uint32 offset)
{
  m_lastInstr->InsertAfter(instr);                  // <=
  if (offset != Js::Constants::NoByteCodeOffset)
  {
    ....
  }
  else if (m_lastInstr)                             // <=
  {
      instr->SetByteCodeOffset(m_lastInstr->GetByteCodeOffset());
  }
  m_lastInstr = instr;
  ....
}

Ещё один пример неаккуратного использования указателя, который потенциально может быть нулевым.

Список похожих мест:

  • V595 The 'arrayData' pointer was utilized before it was verified against nullptr. Check lines: 868, 870. immutablelist.h 868
  • V595 The 'pMembersList' pointer was utilized before it was verified against nullptr. Check lines: 2012, 2015. diagobjectmodel.cpp 2012
  • V595 The 'walkerRef' pointer was utilized before it was verified against nullptr. Check lines: 3191, 3193. diagobjectmodel.cpp 3191
  • V595 The 'block->loop' pointer was utilized before it was verified against nullptr. Check lines: 981, 1002. globopt.cpp 981
  • V595 The 'src2Val' pointer was utilized before it was verified against nullptr. Check lines: 12528, 12536. globopt.cpp 12528
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 1966, 1967. irbuilderasmjs.cpp 1966
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 2010, 2011. irbuilderasmjs.cpp 2010
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 2076, 2077. irbuilderasmjs.cpp 2076
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 3591, 3592. irbuilderasmjs.cpp 3591
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 4113, 4114. irbuilderasmjs.cpp 4113
  • V595 The 'symDst' pointer was utilized before it was verified against nullptr. Check lines: 4510, 4511. irbuilderasmjs.cpp 4510
  • V595 The 'm_lastInstr' pointer was utilized before it was verified against nullptr. Check lines: 1102, 1116. irbuilder.cpp 1102

В списке представлены наиболее простые и понятные примеры. Для изучения всех подобных мест разработчикам следует самим изучить отчёт анализатора.

V522 Dereferencing of the null pointer 'tempNumberTracker' might take place. backwardpass.cpp 578

void
BackwardPass::MergeSuccBlocksInfo(BasicBlock * block)
{
  TempNumberTracker * tempNumberTracker = nullptr; // <= line 346
  ....
  if (!block->isDead)
  {
      ....
      if(!IsCollectionPass())
      {
          ....
          if (this->DoMarkTempNumbers())
          {
              tempNumberTracker = JitAnew(....);   // <= line 413
          }
      ....
  ....
  if (blockSucc->tempNumberTracker != nullptr)
  {
      ....
      tempNumberTracker->MergeData(....);          // <= line 578
      if (deleteData)
      {
          blockSucc->tempNumberTracker = nullptr;
      }
  }
  ....
}

Пример другой диагностики, но тоже про указатели. Здесь представлен фрагмент функции MergeSuccBlocksInfo(), она довольно длинная - 707 строк. Но с помощью статического анализа удалось найти указатель "tempNumberTracker", инициализация которого потенциально может не выполнится из-за ряда условий. В результате при неудачном стечении обстоятельств произойдёт разыменование нулевого указателя.

Остановись! Проверь Assert!

0370_ChakraCore_ru/image5.png

Assert, размещённый в программе, указывает на то, что разработчик предполагает, что некоторое выражение является истинным для корректно работающей программы. Но можно ли доверять таким "успешным" проверкам?

V547 Expression 'srcIndex - src->left >= 0' is always true. Unsigned type value is always >= 0. sparsearraysegment.inl 355

class SparseArraySegmentBase
{
public:
    static const uint32 MaxLength;
    ....
    uint32 size;
    ....
}

template<typename T>
SparseArraySegment<T>* SparseArraySegment<T>::CopySegment(....,
  uint32 srcIndex, ....)
{
  ....
  AssertMsg(srcIndex - src->left >= 0,                    // <=
    "src->left > srcIndex resulting in \
     negative indexing of src->elements");
  js_memcpy_s(dst->elements + dstIndex - dst->left,
              sizeof(T) * inputLen,
              src->elements + srcIndex - src->left,
              sizeof(T) * inputLen);
  return dst;
}

Обратите внимание на сравнение "srcIndex - src->left >= 0". Разность двух беззнаковым чисел всегда будет больше или равна нулю. Далее эта формула используется в функции для работы с памятью. Результат может быть не таким, как ожидал программист.

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

void ByteCodeGenerator::AssignRegister(Symbol *sym)
{
  AssertMsg(sym->GetDecl() == nullptr ||
            sym->GetDecl()->nop != knopConstDecl ||      // <=
            sym->GetDecl()->nop != knopLetDecl, "...."); // <=
            
  if (sym->GetLocation() == Js::Constants::NoRegister)
  {
    sym->SetLocation(NextVarRegister());
  }
}

В этом Assert'е тестирование некоторых значений выполняется частично. Если предположение "sym->GetDecl() == nullptr" ложно, то следующие условия всегда истинны. В этом можно убедиться, построив таблицу истинности:

0370_ChakraCore_ru/image6.png

V547 Expression 'callSiteId >= 0' is always true. Unsigned type value is always >= 0. inline.cpp 1181

typedef uint16 ProfileId;

Func * Inline::BuildInlinee(Js::FunctionBody* funcBody, ....)
{
  ....
  Js::ProfileId callSiteId = static_cast<Js::ProfileId>(....);
  Assert(callSiteId >= 0);
  ....
}

В этом и ещё двух других местах анализатор обнаружил некорректное сравнение беззнакового числа с нулём:

  • V547 Expression 'callSiteId >= 0' is always true. Unsigned type value is always >= 0. inline.cpp 2627
  • V547 Expression 'callSiteId >= 0' is always true. Unsigned type value is always >= 0. inline.cpp 3657

Заключение

У Microsoft замечается позитивная тенденция открывать свои проекты под свободными лицензиями. Для нас это дополнительное тестирование анализатора на новых проектах, а также возможность продемонстрировать полезность и эффективность статического анализа на проектах такого крупного и известного производителя программного обеспечения.

Возможно, вам будет интересен список всех проверенных проектов, включающих и другие проекты от Microsoft, таких как .NET CoreCLR, .NET CoreFX и Microsoft Code Contracts.

Популярные статьи по теме
Характеристики анализатора PVS-Studio на примере EFL Core Libraries, 10-15% ложных срабатываний

Дата: 31 Июл 2017

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

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

Дата: 31 Май 2014

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

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

Дата: 27 Июн 2017

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

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

Дата: 22 Окт 2018

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

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

Дата: 17 Янв 2019

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

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

Дата: 21 Ноя 2018

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

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

Дата: 22 Дек 2018

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

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

Дата: 20 Мар 2017

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

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

Дата: 19 Май 2017

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

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

Дата: 14 Апр 2016

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

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

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

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

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