Поиск ошибок в коде проекта LLVM с помощью PVS-Studio

Около двух месяцев назад я написал статью об анализе GCC с помощью PVS-Studio. Идея статьи заключалась в следующем: предупреждения GCC - это здорово, но их недостаточно. Для анализа кода необходимо использовать специализированные инструменты, например, PVS-Studio. В доказательство своих слов я показал ошибки, что PVS-Studio смог найти код GCC. Ряд читателей заметили, что качество кода GCC и его диагностика невысоки; в то время как компилятор Clang обновлен, качественен и свеж. В общем Clang классный! Что ж, пора проверить проект LLVM с помощью PVS-Studio.

Проверка LLVM с помощью Linux-версии PVS-Studio

Думаю, мало кто не знает, что такое LLVM. Тем не менее, я сохраню традицию давать краткое описание протестированного проекта.

LLVM (Low Level Virtual Machine) - универсальная система анализа, трансформации и оптимизации программ, реализующая виртуальную машину с RISC-инструкциями. Его можно использовать как оптимизирующий компилятор байтового кода в машинный код для различных архитектур или для его интерпретации и JIT-компиляции (для некоторых платформ). В рамках проекта LLVM разработчики создали интерфейс Clang для C, C ++ и Objective-C, переведя исходный код в байтовый код LLVM и позволив использовать LLVM в качестве полноценного компилятора.

Официальный сайт: http://llvm.org/

Мы проверили ревизию 282481. Код проверялся на версии PVS-Studio, работающей под Linux. Поскольку PVS-Studio для Linux - новый продукт, я расскажу подробнее о процессе анализа. Я уверен, что это продемонстрирует, что использовать наш анализатор в Linux действительно несложно, и вы должны без колебаний попробовать его на своем проекте.

Версия анализатора для Linux доступна для скачивания на этой странице: http://www.viva64.com/ru/pvs-studio-download-linux/

Предыдущие проекты проверялись универсальным механизмом, отслеживающим запуск компилятора. На этот раз мы воспользуемся для анализа информацией, которую PVS-Studio берет из JSON Database Compilation. Подробности читайте в разделе Как запустить PVS-Studio на Linux.

В LLVM 3.9 мы полностью отказались от использования autoconf в пользу Cmake, и это был хороший повод попробовать поддержку JSON Compilation Database. Что это? Это формат, используемый утилитами Clang. Он хранит список вызовов компилятора следующим образом:

[
  {
    "directory": "/home/user/llvm/build",
    "command": "/usr/bin/c++ .... file.cc",
    "file": "file.cc"
  },
  ....
]

Получить такой файл для CMake-проектов очень просто - достаточно просто сгенерировать проект с дополнительной опцией:

cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On ../llvm

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

make -j8

Теперь все готово к анализу. Он начинается с единственной строки:

pvs-studio-analyzer analyze -l ~/PVS-Studio.lic -o PVS-Studio.log -j

Вы можете получить compile_commands.json с помощью утилиты Bear для проектов, не использующих CMake. Но для сложных систем сборки, активно использующих переменные среды или кросс-компиляцию, команды не всегда предоставляют подробную информацию о единице перевода.

Примечание N1. Как работать с отчетом PVS-Studio в Linux.

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

Результаты анализа

Кстати, это не первая проверка LLVM. Статья вдохновлена предыдущими проверками:

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

Хватит разговоров, перейдем к самому интересному. Давайте посмотрим на подозрительные фрагменты кода LLVM, обнаруженные PVS-Studio.

Небитовые поля

Итак, в коде есть такое перечисление:

enum Type {
  ST_Unknown, // Type not specified
  ST_Data,
  ST_Debug,
  ST_File,
  ST_Function,
  ST_Other
};

Это, если можно так выразиться, «классическое перечисление». Каждому имени в перечислении присваивается целочисленное значение, которое соответствует определенному месту в порядке значений в перечислении:

ST_Unknown = 0
ST_Data = 1
ST_Debug = 2
ST_File = 3
ST_Function = 4
ST_Other = 5

Еще раз подчеркну, что это просто перечисление, а не набор масок. Если бы константы могли быть объединены, они были бы степенью двойки.

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

void MachODebugMapParser::loadMainBinarySymbols(....)
{
  ....
  SymbolRef::Type Type = *TypeOrErr;
  if ((Type & SymbolRef::ST_Debug) ||
      (Type & SymbolRef::ST_Unknown))
    continue;
  ....
}

Предупреждение PVS-Studio: V616 Именованная константа SymbolRef :: ST_Unknown со значением 0 используется в побитовой операции. MachODebugMapParser.cpp 448

Напомним по памяти, что константа ST_Unknown равна нулю. Следовательно, вы можете сократить выражение:

if (Type & SymbolRef::ST_Debug)

Ясно, что здесь что-то не так. Видимо программист, писавший этот код, решил, что работает с перечислением, состоящим из флагов. То есть он ожидал, что каждой константе будет соответствовать тот или иной бит. Но это не так. Думаю, правильная проверка должна быть такой:

if ((Type == SymbolRef::ST_Debug) || (Type == SymbolRef::ST_Unknown))

Я думаю, здесь следовало использовать enum class, чтобы избежать таких ошибок. В этом случае некорректное выражение просто не будет скомпилировано.

Циклы с одной итерацией

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

Parser::TPResult Parser::TryParseProtocolQualifiers() {
  assert(Tok.is(tok::less) && "Expected '<' for qualifier list");
  ConsumeToken();
  do {
    if (Tok.isNot(tok::identifier))
      return TPResult::Error;
    ConsumeToken();
    
    if (Tok.is(tok::comma)) {
      ConsumeToken();
      continue;
    }
    
    if (Tok.is(tok::greater)) {
      ConsumeToken();
      return TPResult::Ambiguous;
    }
  } while (false);
  
  return TPResult::Error;
}

Предупреждение PVS-Studio: V696 Оператор continue завершает цикл do {…} while (FALSE), потому что условие всегда ложно. Проверить строки: 1642, 1649. ParseTentative.cpp 1642

Разработчики LLVM, конечно же, смогут понять, есть ли здесь ошибка или нет. Я должен играть в детектива. Глядя на код, я думал в следующем направлении: функция должна прочитать открывающую скобку ‘‹ ’, затем она считывает идентификаторы и запятые в цикле. Если запятой нет, мы ожидали закрывающую скобку. Если что-то пойдет не так, функция вернет код ошибки. Полагаю, должен был быть следующий алгоритм работы функции (псевдокод):

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

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

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

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

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

static bool checkMachOAndArchFlags(....) {
  ....
  unsigned i;
  for (i = 0; i < ArchFlags.size(); ++i) {
    if (ArchFlags[i] == T.getArchName())
      ArchFound = true;
    break;
  }
  ....
}

Предупреждение PVS-Studio: V612 Безусловный разрыв внутри цикла. MachODump.cpp 1206

Обратите внимание на оператор break. Это прервет цикл после первой итерации. Я думаю, что оператор break должен относиться к условию, поэтому правильный код будет выглядеть так:

for (i = 0; i < ArchFlags.size(); ++i) {
  if (ArchFlags[i] == T.getArchName())
  {
    ArchFound = true;
    break;
  }
}

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

V612 Безусловный «возврат» внутри цикла. R600OptimizeVectorRegisters.cpp 54
V612 Безусловный «разрыв» внутри цикла. llvm-size.cpp 525

|| и операторы && перепутаны

static bool containsNoDependence(CharMatrix &DepMatrix,
                                 unsigned Row,
                                 unsigned Column) {
  for (unsigned i = 0; i < Column; ++i) {
    if (DepMatrix[Row][i] != '=' || DepMatrix[Row][i] != 'S' ||
        DepMatrix[Row][i] != 'I')
      return false;
  }
  return true;
}

Предупреждение PVS-Studio: выражение V547 всегда истинно. Вероятно, здесь следует использовать оператор &&. LoopInterchange.cpp 208

Выражение не имеет смысла. Я упрощу код, чтобы выделить суть ошибки:

if (X != '=' || X != 'S' || X != 'I')

Переменная X никогда ничему не будет равна. В результате условие всегда выполняется. Скорее всего, вместо операторов «||» следовало использовать «&&», тогда выражение имело бы смысл.

Функция возвращает ссылку на локальный объект

SingleLinkedListIterator<T> &operator++(int) {
  SingleLinkedListIterator res = *this;
  ++*this;
  return res;
}

Предупреждение PVS-Studio: Функция V558 возвращает ссылку на временный локальный объект: res. LiveInterval.h 679

Функция представляет собой традиционную реализацию постфиксного приращения:

Текущее состояние сохраняется во временном объекте;
Изменяется текущее состояние объекта;
Возвращается старое состояние объекта.

Ошибка в том, что функция возвращает ссылку. Эта ссылка недействительна, потому что временный объект res уничтожается при выходе из функции.

Чтобы исправить это, вам нужно вернуть значение, а не ссылку:

SingleLinkedListIterator<T> operator++(int) { .... }

Повторное присвоение

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

HexagonMCAsmInfo::HexagonMCAsmInfo(const Triple &TT) {
  Data16bitsDirective = "\t.half\t";
  Data32bitsDirective = "\t.word\t";
  Data64bitsDirective = nullptr;
  ZeroDirective = "\t.skip\t";                            // <=
  CommentString = "//";
  LCOMMDirectiveAlignmentType = LCOMM::ByteAlignment;
  InlineAsmStart = "# InlineAsm Start";
  InlineAsmEnd = "# InlineAsm End";
  ZeroDirective = "\t.space\t";                           // <=
  AscizDirective = "\t.string\t";
  SupportsDebugInformation = true;
  MinInstAlignment = 4;
  UsesELFSectionDirectiveForBSS  = true;
  ExceptionsType = ExceptionHandling::DwarfCFI;
}

Предупреждение PVS-Studio: V519 Переменной ZeroDirective дважды подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 25, 31. HexagonMCAsmInfo.cpp 31

Переменная ZeroDirective представляет собой простой указатель типа const char *. Вначале он указывает на строку «\ t.skip \ t», но в дальнейшем ему присваивается адрес строки «\ t.space \ t». Это странно и бессмысленно. Есть большая вероятность, что одно из присвоений должно изменить совершенно другую переменную.

Давайте посмотрим на другой случай повторяющегося задания.

template <class ELFT>
void GNUStyle<ELFT>::printFileHeaders(const ELFO *Obj) {
  ....
  Str = printEnum(e->e_ident[ELF::EI_OSABI], makeArrayRef(ElfOSABI));
  printFields(OS, "OS/ABI:", Str);
  Str = "0x" + to_hexString(e->e_version);                  // <=
  Str = to_hexString(e->e_ident[ELF::EI_ABIVERSION]);       // <=
  printFields(OS, "ABI Version:", Str);
  Str = printEnum(e->e_type, makeArrayRef(ElfObjectFileType));
  printFields(OS, "Type:", Str);
  ....
}

Предупреждение PVS-Studio: V519 Переменной Str два раза подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 2407, 2408. ELFDumper.cpp 2408

Видимо мы имеем дело с опечаткой. Вместо переназначения программисту пришлось связать две строки с помощью оператора + =. Тогда правильный код может быть таким:

Str = "0x" + to_hexString(e->e_version);
Str += to_hexString(e->e_ident[ELF::EI_ABIVERSION]);

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

V519 Переменной дважды подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 55, 57. coff2yaml.cpp 57
V519 Переменной «O» два раза подряд присваиваются значения. Возможно, это ошибка. Проверьте строки: 394, 395. llvm-pdbdump.cpp 395
V519 Переменной servAddr.sin_family два раза подряд присваиваются значения. Возможно, это ошибка. Проверить строки: 63, 64. server.cpp 64

Подозрительное обращение с умными указателями

Expected<std::unique_ptr<PDBFile>>
PDBFileBuilder::build(
  std::unique_ptr<msf::WritableStream> PdbFileBuffer)
{
  ....
  auto File = llvm::make_unique<PDBFile>(
    std::move(PdbFileBuffer), Allocator);
  File->ContainerLayout = *ExpectedLayout;
  if (Info) {
    auto ExpectedInfo = Info->build(*File, *PdbFileBuffer);
  ....
}

Предупреждение PVS-Studio: V522 Возможно разыменование нулевого указателя PdbFileBuffer. PDBFileBuilder.cpp 106

Код мне непонятен, так как я не изучал, что такое llvm :: make_unique и как он работает в целом. Тем не менее и меня, и анализатора смущает тот факт, что на первый взгляд владение объектом из умного указателя PdbFileBuffer переходит в File. После этого происходит разыменование нулевого указателя PdbFileBuffer, который уже содержит nullptr. Конкретно этот фрагмент выглядит странно:

.... llvm::make_unique<PDBFile>(::move(PdbFileBuffer), Allocator);
....
.... Info->build(*File, *PdbFileBuffer);

Если это ошибка, то ее нужно исправить еще в 3-х фрагментах в том же файле:

V522 Может иметь место разыменование нулевого указателя «PdbFileBuffer». PDBFileBuilder.cpp 113
V522 Может иметь место разыменование нулевого указателя «PdbFileBuffer». PDBFileBuilder.cpp 120
V522 Может иметь место разыменование нулевого указателя «PdbFileBuffer». PDBFileBuilder.cpp 127

Опечатка в условии

static bool areExclusiveRanges(BinaryOperatorKind OpcodeLHS,
                               const APSInt &ValueLHS,
                               BinaryOperatorKind OpcodeRHS,
                               const APSInt &ValueRHS) {
  ....
  // Handle cases where the constants are different.
  if ((OpcodeLHS == BO_EQ ||
       OpcodeLHS == BO_LE ||                 // <=
       OpcodeLHS == BO_LE)                   // <=
      &&
      (OpcodeRHS == BO_EQ ||
       OpcodeRHS == BO_GT ||
       OpcodeRHS == BO_GE))
    return true;
  ....
}

Предупреждение PVS-Studio: V501 Слева и справа от оператора || есть идентичные подвыражения OpcodeLHS == BO_LE. RedundantExpressionCheck.cpp 174

Это классическая опечатка. Переменная OpcodeLHS дважды сравнивается с константой BO_LE. Мне кажется, что одну из констант BO_LE следует заменить на BO_LT. Как видите, названия констант очень похожи, и их легко перепутать.

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

std::pair<Function *, Function *>
llvm::createSanitizerCtorAndInitFunctions(
    ....
    ArrayRef<Type *> InitArgTypes, ArrayRef<Value *> InitArgs,
    ....)
{
  assert(!InitName.empty() && "Expected init function name");
  assert(InitArgTypes.size() == InitArgTypes.size() &&
    "Sanitizer's init function expects "
    "different number of arguments");
  ....

}

Предупреждение PVS-Studio: V501 Слева и справа от оператора «==» есть идентичные подвыражения «InitArgTypes.size ()». ModuleUtils.cpp 107

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

Для нас важно, чтобы макросы assert () использовались в функции createSanitizerCtorAndInitFunctions () для проверки правильности входных данных. Жаль, что второй макрос assert () бесполезен из-за опечатки.

К счастью, здесь очень помогает статический анализатор, так как он замечает, что размер массива сравнивается с самим собой. В результате мы можем исправить эту проверку, а правильное условие в assert () может помочь предотвратить другие ошибки в будущем.

Видимо, в условии нужно сравнивать размеры массивов InitArgTypes и InitArgs:

assert(InitArgTypes.size() == InitArgs.size() &&
  "Sanitizer's init function expects "
  "different number of arguments");

Путаница между release () и reset ()

В классе std :: unique_ptr есть две функции с похожими именами: release и reset. Мои наблюдения показывают, что их иногда путают. Видимо вот что здесь произошло:

std::unique_ptr<DiagnosticConsumer> takeClient()
  { return std::move(Owner); }
VerifyDiagnosticConsumer::~VerifyDiagnosticConsumer() {
  ....
  SrcManager = nullptr;
  CheckDiagnostics();
  Diags.takeClient().release();
}

Предупреждение PVS-Studio: V530 Необходимо использовать возвращаемое значение функции release. VerifyDiagnosticConsumer.cpp 46

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

Резервные условия

bool ARMDAGToDAGISel::tryT1IndexedLoad(SDNode *N) {
  LoadSDNode *LD = cast<LoadSDNode>(N);
  EVT LoadedVT = LD->getMemoryVT();
  ISD::MemIndexedMode AM = LD->getAddressingMode();
  if (AM == ISD::UNINDEXED ||
      LD->getExtensionType() != ISD::NON_EXTLOAD ||
      AM != ISD::POST_INC ||
      LoadedVT.getSimpleVT().SimpleTy != MVT::i32)
    return false;
  ....
}

Предупреждение PVS-Studio: V590 Рассмотрите возможность проверки этого выражения. Выражение является чрезмерным или содержит опечатку. ARMISelDAGToDAG.cpp 1565

Условие длинное, поэтому выделю самую важную часть:

AM == ISD::UNINDEXED || AM != ISD::POST_INC

Это условие является избыточным, и его можно упростить до:

AM != ISD::POST_INC

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

V590 Рассмотрите возможность проверки этого выражения. Выражение является чрезмерным или содержит опечатку. ASTReader.cpp 4178
V590 Рассмотрите возможность проверки этого выражения. Выражение является чрезмерным или содержит опечатку. BracesAroundStatementsCheck.cpp 46

Мои любимые предупреждения V595

Указатели в C и C ++ - бесконечная головная боль программистов. Вы проверяете их на соответствие нулю, а затем где-то снова происходит разыменование нулевого указателя! Диагностика V595 обнаруживает ситуации, когда проверка на нуль выполняется слишком поздно. Перед этой проверкой указатель уже используется. Это одна из самых типичных ошибок, которые мы находим в коде различных приложений (пруф). Однако, говоря в поддержку C / C ++, скажу, что ситуация с C # не намного лучше. Несмотря на то, что указатели C # теперь называются ссылками, такие ошибки никуда не делись (доказательство).

Вернемся к коду LLVM и рассмотрим простой вариант ошибки:

bool PPCDarwinAsmPrinter::doFinalization(Module &M) {
  ....
  MachineModuleInfoMachO &MMIMacho =
      MMI->getObjFileInfo<MachineModuleInfoMachO>();
  if (MAI->doesSupportExceptionHandling() && MMI) {
  ....
}

Предупреждение PVS-Studio: V595 Указатель MMI использовался до проверки на nullptr. Проверьте строки: 1357, 1359. PPCAsmPrinter.cpp 1357

Дело простое, и все достаточно очевидно. Проверка (… && MMI) сообщает нам, что указатель MMI может иметь значение NULL. В этом случае программа не сможет пройти эту проверку во время выполнения. Он будет прекращен раньше из-за разыменования нулевого указателя.

Посмотрим еще на один фрагмент кода:

void Sema::CodeCompleteObjCProtocolReferences(
  ArrayRef<IdentifierLocPair> Protocols)
{
  ResultBuilder 
    Results(*this, CodeCompleter->getAllocator(),
            CodeCompleter->getCodeCompletionTUInfo(),
            CodeCompletionContext::CCC_ObjCProtocolName);
  
  if (CodeCompleter && CodeCompleter->includeGlobals()) {
    Results.EnterNewScope();
  ....
}

Предупреждение PVS-Studio: V595 Указатель CodeCompleter использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 5952, 5955. SemaCodeComplete.cpp 5952

Сначала разыменовывается указатель CodeCompleter, а затем выполняется проверка указателя на нуль. Этот же код был обнаружен еще трижды в том же файле:

V595 Указатель CodeCompleter использовался до того, как он был проверен против nullptr. Проверить строки: 5980, 5983. SemaCodeComplete.cpp 5980
V595 Указатель CodeCompleter использовался до того, как он был проверен против nullptr. Проверить строки: 7455, 7458. SemaCodeComplete.cpp 7455
V595 Указатель CodeCompleter использовался до того, как он был проверен против nullptr. Проверить строки: 7483, 7486. SemaCodeComplete.cpp 7483

Это были простые случаи, но иногда код бывает более сложным, и сложно сказать, насколько он опасен. Поэтому я предлагаю разработчикам проверить следующие фрагменты кода LLVM:

V595 Указатель «Получатель» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 2543, 2560. SemaExprObjC.cpp 2543
V595 Указатель «S» использовался до проверки на nullptr. Проверьте строки: 1267, 1296. SemaLookup.cpp 1267
V595 Указатель TargetDecl использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 4037, 4046. CGExpr.cpp 4037
V595 Указатель CurrentToken использовался до проверки на nullptr. Проверить строки: 705, 708. TokenAnnotator.cpp 705
V595 Указатель «FT» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 540, 554. Expr.cpp 540
V595 Указатель «II» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 448, 450. IdentifierTable.cpp 448
V595 Указатель «MF» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 268, 274. X86RegisterInfo.cpp 268
V595 "Внешний" указатель использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 40, 45. HeaderSearch.cpp 40
V595 Указатель «TLI» использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 4239, 4244. CodeGenPrepare.cpp 4239
V595 Указатель SU- ›getNode () использовался до того, как он был проверен на соответствие nullptr. Проверить строки: 292, 297. ResourcePriorityQueue.cpp 292
V595 Указатель «BO0» использовался до того, как он был проверен на соответствие nullptr. Проверьте строки: 2835, 2861. InstCombineCompares.cpp 2835
V595 Указатель «Ret» использовался до того, как он был проверен на соответствие nullptr. Проверьте строки: 2090, 2092. ObjCARCOpts.cpp 2090

Странный код

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

static bool print_class_ro64_t(....) {
  ....
  const char *r;
  uint32_t offset, xoffset, left;
  ....
  r = get_pointer_64(p, offset, left, S, info);
  if (r == nullptr || left < sizeof(struct class_ro64_t))
    return false;
  memset(&cro, '\0', sizeof(struct class_ro64_t));
  if (left < sizeof(struct class_ro64_t)) {
    memcpy(&cro, r, left);
    outs() << "   (class_ro_t entends past the .......)\n";
  } else
    memcpy(&cro, r, sizeof(struct class_ro64_t));
  ....
}

Предупреждение PVS-Studio: V649 Есть два оператора if с одинаковыми условными выражениями. Первый оператор if содержит возврат функции. Это означает, что второе утверждение если бессмысленно. Проверьте строки: 4410, 4413. MachODump.cpp 4413

Обратите внимание на чек:

if (.... || left < sizeof(struct class_ro64_t))
  return false;

Если значение в переменной left меньше размера класса, функция завершится. Оказывается, такой выбор поведения не имеет смысла:

if (left < sizeof(struct class_ro64_t)) {
  memcpy(&cro, r, left);
  outs() << "   (class_ro_t entends past the .......)\n";
} else
  memcpy(&cro, r, sizeof(struct class_ro64_t));

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

Это место тоже требует доработки:

V649 Есть два оператора if с одинаковыми условными выражениями. Первый оператор «if» содержит возврат функции. Это означает, что второе утверждение «если» бессмысленно. Проверьте строки: 4612, 4615. MachODump.cpp 4615

Пара небольших заметок

Класс SequenceNumberManager объявлен внутри класса шаблона RPC. У него есть такой оператор присваивания хода:

SequenceNumberManager &operator=(SequenceNumberManager &&Other) {
  NextSequenceNumber = std::move(Other.NextSequenceNumber);
  FreeSequenceNumbers = std::move(Other.FreeSequenceNumbers);
}

Предупреждение PVS-Studio: непустая функция V591 должна возвращать значение. RPCUtils.h 719

Как видите, о возврате в конце концов забыли:

return *this;

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

Было несколько странных фрагментов кода, где значение указателя, возвращаемого оператором new, сверяется с нулевым значением. Этот код не имеет смысла, потому что, если вы не можете выделить память, будет выброшено исключение std :: bad_alloc. Вот одно из таких мест:

LLVMDisasmContextRef LLVMCreateDisasmCPUFeatures(....) {
  ....
  // Set up the MCContext for creating symbols and MCExpr's.
  MCContext *Ctx = new MCContext(MAI, MRI, nullptr);
  if (!Ctx)
    return nullptr;
  ....
}

Предупреждение PVS-Studio: V668 Нет смысла проверять указатель Ctx на ноль, так как память была выделена с помощью оператора new. Исключение будет сгенерировано в случае ошибки выделения памяти. Disassembler.cpp 76

Еще два предупреждения:

V668 Нет смысла проверять указатель «DC» на ноль, так как память была выделена с помощью оператора «new». Исключение будет сгенерировано в случае ошибки выделения памяти. Disassembler.cpp 103
V668 Нет смысла проверять указатель «JITCodeEntry» на нуль, так как память была выделена с помощью оператора «новый». Исключение будет сгенерировано в случае ошибки выделения памяти. GDBRegistrationListener.cpp 180

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

Вывод

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

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

Предлагаю скачать и опробовать PVS-Studio на коде вашего проекта.

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