# Примеры ошибок, которые может обнаружить PVS\-Studio в коде LLVM 15\.0

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

![1003_LLVM_15_ru/image1.png](https://import.viva64.com/docx/blog/1003_LLVM_15_ru/image1.png)

## Назначение статьи

Иногда программисты задаются вопросом, а есть ли смысл пробовать какие\-то дополнительные инструменты контроля качества кода, помимо тех, которые предоставляют компиляторы/среды разработки\. Есть ли преимущества у таких инструментов перед "условным \-Wall"? Стоит ли внедрять в процесс разработки дополнительный инструмент, такой как PVS\-Studio?

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

Ответ: да\. Есть смысл использовать дополнительные инструменты статического анализа кода\. Эти инструменты тоже развиваются и учатся выполнять всё более глубокий анализ кода\. Сторонние инструменты решают узкую задачу: поиск ошибок и потенциальных уязвимостей\. А следовательно они решают эту задачу лучше, чем компиляторы, для которых это только одна из функций\.

Однако это теоретический ответ\. А как дела обстоят на практике?

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

* 2021 — [LLVM 13\.0](https://pvs-studio.ru/ru/blog/posts/cpp/0871/);
* 2020 — [GCC 10](https://pvs-studio.ru/ru/blog/posts/cpp/0727/);
* 2020 — [LLVM 11](https://pvs-studio.ru/ru/blog/posts/cpp/0771/);
* 2019 — [Roslyn](https://pvs-studio.ru/ru/blog/posts/csharp/0622/);
* 2019 — [LLVM 8](https://pvs-studio.ru/ru/blog/posts/cpp/0629/)\.

Проверку LLVM 14 мы пропустили, но решили, что стоит написать заметку, посвященную проверке LLVM 15\.

[**Справка из Wikipedia**](https://ru.wikipedia.org/wiki/LLVM)**\.** LLVM — проект программной инфраструктуры для создания компиляторов и сопутствующих им утилит\. Состоит из набора компиляторов из языков высокого уровня, системы оптимизации, интерпретации и компиляции в машинный код\. Написан на C, C\+\+ и ассемблере\. Сайт проекта: [llvm\.org\.](https://llvm.org/)

При разработке проекта LLVM для предотвращения ошибок используются [диагностические возможности](https://clang.llvm.org/diagnostics.html) компилятора Clang\. LLVM проверяется с помощью [Clang Static Analyzer](https://clang-analyzer.llvm.org/) и [clang\-tidy](https://clang.llvm.org/extra/clang-tidy/)\. Более того, проект LLVM дополнительно проверяется с помощью проприетарного анализатора Coverity\.

Найти даже несколько ошибок в LLVM с помощью статического анализатора — это уже большое достижение\. И анализатор PVS\-Studio на это способен\!

## Анализ проекта, предупреждения, ложные срабатывания

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

Это не является правильным сценарием работы с анализатором\. Надо понимать, что моей целью было только написать эту статью :\)\. И с этой задачей я успешно справился\. О том, как правильно начать использовать анализатор, я писал в статье "[Как внедрить статический анализатор кода в legacy проект и не демотивировать команду](https://pvs-studio.ru/ru/blog/posts/0743/)"\.

Итак, в отчёте встретились [ложноположительные срабатывания](https://pvs-studio.ru/ru/blog/terms/6461/)\. С этими всё понятно, любой статический анализатор, к сожалению, их выдаёт\. Интересен другой момент\. На проекте LLVM я увидел очень большое количество предупреждений, которые нельзя назвать ложными\. Анализатор прав\. Но и полезными эти сообщения назвать сложно\. Давайте чуть подробнее остановимся на этой теме\.

Очень много неоднозначных предупреждений связано с условиями, которые всегда истинны или ложны\. Из\-за этого я даже не захотел изучать предупреждения [V547 \- Expression is always true/false](https://pvs-studio.ru/ru/docs/warnings/v547/)\. Это утомительно и неинтересно\. Давайте на паре примеров посмотрим, что я имею в виду\.

```cpp
int L = -1, V = -1, T = -1;
if (L != -1 && V != -1 && T != -1 && Name.empty()) {  // <=
  if (!Strict) {
    Buffer.append("HANGUL SYLLABLE ");
    if (L != -1)                                      // <=
      Buffer.append(HangulSyllables[L][0]);
    if (V != -1)                                      // <=
      Buffer.append(HangulSyllables[V][1]);
    if (T != -1)                                      // <=
      Buffer.append(HangulSyllables[T][2]);
  }
```

Здесь анализатор выдаёт сразу 3 предупреждения:

* V547 \[CWE\-571\] Expression 'L \!\= \- 1' is always true\. UnicodeNameToCodepoint\.cpp 306
* V547 \[CWE\-571\] Expression 'V \!\= \- 1' is always true\. UnicodeNameToCodepoint\.cpp 308
* V547 \[CWE\-571\] Expression 'T \!\= \- 1' is always true\. UnicodeNameToCodepoint\.cpp 310

И действительно, переменные _L_, _V_, _T_ проверяются дважды\. Повторные проверки избыточны, но и ошибкой это назвать нельзя\.

Другой пример:

```cpp
while (top_reader_sp) {
  if (!top_reader_sp)
    break;
  top_reader_sp->Run();
```

Предупреждение: V547 \[CWE\-570\] Expression '\!top\_reader\_sp' is always false\. Debugger\.cpp 1042

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

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

## 42 ошибки в коде LLVM

Почему 42? Потому, что "42 — ответ на главный вопрос жизни, вселенной и всего такого" :\)\. Выписывать больше примеров ошибок мне было утомительно и неинтересно\. Да и всё равно читать здоровый отчёт будет никому неинтересно\. Ну разве что разработчикам LLVM\.\.\. Но им будет полезней проверить проект, настроить анализатор PVS\-Studio и самостоятельно изучить предупреждения\.

**Фрагмент N1**

Классическая ошибка, похожая на множество тех, что я рассматривал в статье "[Зло живёт в функциях сравнения](https://pvs-studio.ru/ru/blog/posts/cpp/0509/)"\. Конечно, перед нами не совсем функция сравнения\. Однако суть в том же\. Есть два объекта, над которыми выполняется ряд проверок\. Эта функция скучна, и её не интересно внимательно проверять на code review\. И именно поэтому в ней есть эта ошибка, которую, к счастью, может заметить статический анализатор кода\. Анализатор никогда не устаёт и не ленится :\)\.

```cpp
static bool isCopyCompatibleType(LLT SrcTy, LLT DstTy) {
  if (SrcTy == DstTy)
    return true;

  if (SrcTy.getSizeInBits() != DstTy.getSizeInBits())
    return false;

  SrcTy = SrcTy.getScalarType();
  DstTy = DstTy.getScalarType();

  return (SrcTy.isPointer() && DstTy.isScalar()) ||
         (DstTy.isScalar() && SrcTy.isPointer());
}
```

Предупреждение PVS\-Studio: V501 \[CWE\-570\] There are identical sub\-expressions '\(SrcTy\.isPointer\(\) && DstTy\.isScalar\(\)\)' to the left and to the right of the '\|\|' operator\. CallLowering\.cpp 1198

Ошибка здесь:

```cpp
(SrcTy.isPointer() && DstTy.isScalar()) ||
(DstTy.isScalar()  && SrcTy.isPointer())
```

Программист одновременно поменял во второй строчке _Src_ на _Dst_ и _Pointer_ на _Scalar_\. В результате получается, что два раза проверяется одно и то же\! Код выше эквивалентен:

```cpp
(SrcTy.isPointer() && DstTy.isScalar()) ||
(SrcTy.isPointer() && DstTy.isScalar())
```

Правильный вариант:

```cpp
(SrcTy.isPointer() && DstTy.isScalar()) ||
(SrcTy.isScalar()  && DstTy.isPointer())
```

**Фрагмент N2**

И сразу ещё одна родственная ошибка\.

```cpp
static ValueKnowledge meet(const ValueKnowledge &lhs,
                           const ValueKnowledge &rhs) {
  ValueKnowledge result = getPessimisticValueState();
  result.hasError = true;

  if (!rhs || !rhs || lhs.dtype != rhs.dtype)   // <=
    return result;

  result.hasError = false;
  result.dtype = lhs.dtype;
  ....
}
```

Предупреждение PVS\-Studio: V501 \[CWE\-570\] There are identical sub\-expressions to the left and to the right of the '\|\|' operator: \!rhs \|\|\!rhs ShapeUtils\.h 141

Дважды проверяется объект _rhs_, а _lhs_ не проверяется\. Обыкновенная спешка при написании кода с отсутствием последующей внимательной проверки на этапе code review\.

**Фрагмент N3**

```cpp
std::optional<parser::MessageFixedText> Scope::SetImportKind(ImportKind kind) {
  ....
  } else if (kind != *importKind_ &&
      (kind != ImportKind::Only || kind != ImportKind::Only)) {
    return 
      "Every IMPORT must have ONLY specifier if one of them does"_err_en_US;
  } else {
  ....
}
```

Предупреждение PVS\-Studio: V501 \[CWE\-570\] There are identical sub\-expressions 'kind \!\= ImportKind::Only' to the left and to the right of the '\|\|' operator\. scope\.cpp 275

Нет смысла два раза выполнять одну и ту же проверку\. Перед нами явная опечатка\. Должна быть использована ещё какая\-то именованная константа, помимо _ImportKind::Only_\.

Примечание\. Если бы выражение было оформлено в виде "столбика", а не в одну строчку, то, возможно, ошибку было бы легче заметить\. См\. [главу 13\. Выравнивайте однотипный код "таблицей"](https://pvs-studio.ru/ru/blog/posts/cpp/0391/) :\) Я бы писал этот код так:

```cpp
} else if (kind != *importKind_ &&
    (   kind != ImportKind::Only
     || kind != ImportKind::Only)) {
  return ....;
```

**Фрагмент N4**

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

```cpp
bool Merger::maybeZero(unsigned e) const {
  if (tensorExps[e].kind == kInvariant) {
    if (auto c = tensorExps[e].val.getDefiningOp<complex::ConstantOp>()) {
      ArrayAttr arrayAttr = c.getValue();
      return arrayAttr[0].cast<FloatAttr>().getValue().isZero() &&
             arrayAttr[0].cast<FloatAttr>().getValue().isZero();
    }
    if (auto c = tensorExps[e].val.getDefiningOp<arith::ConstantIntOp>())
      return c.value() == 0;
    if (auto c = tensorExps[e].val.getDefiningOp<arith::ConstantFloatOp>())
      return c.value().isZero();
  }
  return true;
}
```

Предупреждение PVS\-Studio: V501 \[CWE\-571\] There are identical sub\-expressions 'arrayAttr\[0\]\.cast < FloatAttr \> \(\)\.getValue\(\)\.isZero\(\)' to the left and to the right of the '&&' operator\. Merger\.cpp 812

Подозрительно это место:

```cpp
return arrayAttr[0].cast<FloatAttr>().getValue().isZero() &&
       arrayAttr[0].cast<FloatAttr>().getValue().isZero();
```

**Фрагмент N5**

И последняя ошибка, выявленная диагностикой V501\.

```cpp
template <int KIND>
Expr<Type<TypeCategory::Logical, KIND>> FoldIntrinsicFunction(....)
{
  ....
  } else if (name == "__builtin_ieee_support_datatype" ||
      name == "__builtin_ieee_support_denormal" ||
      name == "__builtin_ieee_support_divide" ||     // <=
      name == "__builtin_ieee_support_divide" ||     // <=
      name == "__builtin_ieee_support_inf" ||
      name == "__builtin_ieee_support_io" ||
      name == "__builtin_ieee_support_nan" ||
      name == "__builtin_ieee_support_sqrt" ||
      name == "__builtin_ieee_support_standard" ||
      name == "__builtin_ieee_support_subnormal" ||
      name == "__builtin_ieee_support_underflow_control") {
    return Expr<T>{true};
  }
  ....
}
```

Предупреждение PVS\-Studio: V501 \[CWE\-570\] There are identical sub\-expressions 'name \=\= "\_\_builtin\_ieee\_support\_divide"' to the left and to the right of the '\|\|' operator\. fold\-logical\.cpp 218

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

**Фрагмент N6**

Для начала обратите внимание, что _bmap_ — это указатель:

```cpp
struct isl_coalesce_info {
  isl_basic_map *bmap;
  ....
};
```

А теперь ошибочный код, выполняющий бессмысленную проверку этого члена класса:

```cpp
static isl_stat tab_insert_divs(struct isl_coalesce_info *info, ....)
{
  ....
  if (any) {
    if (isl_tab_rollback(info->tab, snap) < 0)
      return isl_stat_error;
    info->bmap = isl_basic_map_cow(info->bmap);
    info->bmap = isl_basic_map_free_inequality(info->bmap, 2 * n);

    if (info->bmap < 0)                 // <=
      return isl_stat_error;

    return fix_constant_divs(info, n, expanded);
  }
  ....
}
```

Предупреждение PVS\-Studio: V503 \[CWE\-697, CERT\-EXP08\-C\] This is a nonsensical comparison: pointer < 0\. isl\_coalesce\.c 3181

Нет практического смысла выполнять проверку "указатель < 0"\. Скорее всего, перед нами опечатка или код не дописан\. Например, с 0 должен сравниваться какой\-то из членов структуры _isl\_basic\_map_\. 

**Фрагмент N7**

Встретилась достаточно редкая и интересная ошибка\. По крайней мере, до этого момента в нашей коллекции, было всего [два таких бага](https://pvs-studio.ru/ru/blog/examples/v504/), найденных нашей командой в открытых проектах\.

```cpp
void FunctionLoweringInfo::ComputePHILiveOutRegInfo(const PHINode *PN) {
  ....
  Register DestReg = It->second;
  if (DestReg == 0)
    return
  assert(Register::isVirtualRegister(DestReg) &&
         "Expected a virtual reg");
  LiveOutRegInfo.grow(DestReg);
  ....
}
```

Предупреждение PVS\-Studio: V504 \[CWE\-841\] It is highly probable that the semicolon ';' is missing after 'return' keyword\. FunctionLoweringInfo\.cpp 454

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

Кажется, что _assert_ выполняется в том случае, если не выполняется условие _DestReg \=\= 0_\. Но на самом деле, assert выполнится только в том случае, если _DestReg \=\= 0_\.

Нестрашна ошибка только из\-за везения\. Собственно, _assert_ не оказывает заметного влияния на выполнение программы\. А в release\-версии он вообще превращается в ничто\.

**Фрагмент N8, N9**

Не изучал историю этого кода, но, скорее всего, это "калька с _malloc_"\. Т\.е\. когда\-то здесь использовалась функция _malloc_, а затем её грубо заменили на _new_ и _shared\_ptr_\.

```cpp
std::shared_ptr<uint8_t>
RenderScriptRuntime::GetAllocationData(....) {
  ....
  const uint32_t size = *alloc->size.get();
  std::shared_ptr<uint8_t> buffer(new uint8_t[size]);
  if (!buffer) {
    LLDB_LOGF(log, "%s - couldn't allocate a %" PRIu32 " byte buffer",
              __FUNCTION__, size);
    return nullptr;
  }
  ....
  return buffer;
}
```

Предупреждение PVS\-Studio: V554 \[CWE\-762, CERT\-MEM51\-CPP\] Incorrect use of shared\_ptr\. The memory allocated with 'new \[\]' will be cleaned using 'delete'\. RenderScriptRuntime\.cpp 2371

Умный указатель _std::shared\_ptr_ используется некорректно:

```cpp
std::shared_ptr<uint8_t> buffer(new uint8_t[size]);
```

Память будет освобождаться с помощью _delete_, а не _delete \[\]_, что приводит к неопределённому поведению\. Подробности см\. в статье "[Почему в С\+\+ массивы нужно удалять через delete\[\]](https://pvs-studio.ru/ru/blog/posts/cpp/0973/)"\. Правильный вариант:

```cpp
std::shared_ptr<uint8_t []> buffer(new uint8_t[size]);
```

Поясняю, почему я предполагаю, что раньше здесь использовался _malloc_\. Обратите внимание на последующую проверку указателя на равенство _nullptr_\. Она не имеет смысла, так как в случае ошибки выделения памяти будет брошено исключение _std::bad\_alloc_\. Проверка и вывод отладочного сообщения — атавизм\.

Ещё точно такая же "калька с _malloc_": V554 \[CWE\-762, CERT\-MEM51\-CPP\] Incorrect use of shared\_ptr\. The memory allocated with 'new \[\]' will be cleaned using 'delete'\. RenderScriptRuntime\.cpp 2698

**Фрагмент N10**

Ой, я давно не находил столь примечательную и забавную ошибку\! Однозначно жемчужина среди опечаток\!

![1003_LLVM_15_ru/image2.png](https://import.viva64.com/docx/blog/1003_LLVM_15_ru/image2.png)

```cpp
OwningMemRef &operator=(const OwningMemRef &&other) {
  freeFunc = other.freeFunc;
  descriptor = other.descriptor;
  other.freeFunc = nullptr;
  memset(0, &other.descriptor, sizeof(other.descriptor));
}
```

Предупреждение PVS\-Studio: V575 \[CWE\-628, CERT\-EXP37\-C\] The null pointer is passed into 'memset' function\. Inspect the first argument\. MemRefUtils\.h 194

Я описал разнообразные способы, как можно ошибиться, используя функцию _memset,_ в статье "[Самая опасная функция в мире С/С\+\+](https://pvs-studio.ru/ru/blog/posts/cpp/0360/)"\. Но это что\-то новенькое\!

Перепутан первый и второй аргумент функции\. Запись произойдёт по нулевому указателю\.

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

В общем ничего страшного, но тем не менее это ошибка, которую лучше поправить заранее\.

**Фрагмент N11\-N17**

Самым распространённым среди обнаруживаемых нашей командой в открытых проектах дефектов является разыменование указателя до его проверки\. По крайней мере больше всего в базе ошибок [выписано](https://pvs-studio.ru/ru/docs/warnings/v595/) случаев именно такого типа\. Исходные коды LLVM не исключение\.

```cpp
void LibCallSimplifier::classifyArgUse(....) {
  CallInst *CI = dyn_cast<CallInst>(Val);
  Module *M = CI->getModule();

  if (!CI || CI->use_empty())
    return;
  ....
}
```

Предупреждение PVS\-Studio: V595 \[CWE\-476, CERT\-EXP12\-C\] The 'CI' pointer was utilized before it was verified against nullptr\. Check lines: 2515, 2517\. SimplifyLibCalls\.cpp 2515

Указатель _CI_ в начале разыменовывается при вызове функции: _CI\-\>getModule\(\)_\. И только затем проверяется на равенство _nullptr_\.

Ещё один пример:

```cpp
void Module::FindFunctions(....) {
  ....
  for (size_t i = 0; i < num_matches; ++i) {
    sc.symbol = symtab->SymbolAtIndex(symbol_indexes[i]);
    SymbolType sym_type = sc.symbol->GetType();
    if (sc.symbol && (sym_type == eSymbolTypeCode ||
                      sym_type == eSymbolTypeResolver))
      sc_list.Append(sc);
  }
  ....
}
```

Предупреждение PVS\-Studio: V595 \[CWE\-476, CERT\-EXP12\-C\] The 'sc\.symbol' pointer was utilized before it was verified against nullptr\. Check lines: 877, 878\. Module\.cpp 877

Чтобы сократить выражение, программист решил предварительно сохранить некий тип символа в переменной _sym\_type_:

```cpp
SymbolType sym_type = sc.symbol->GetType();
```

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

```cpp
if (sc.symbol && (sym_type == eSymbolTypeCode ||
                  sym_type == eSymbolTypeResolver))
```

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

<details>
   <summary>Но как минимум есть ещё пяток таких ошибок\\\.</summary>

* V595 \[CWE\-476, CERT\-EXP12\-C\] The 'sc\.symbol' pointer was utilized before it was verified against nullptr\. Check lines: 899, 900\. Module\.cpp 899
* V595 \[CWE\-476, CERT\-EXP12\-C\] The 'process' pointer was utilized before it was verified against nullptr\. Check lines: 159, 184\. IRExecutionUnit\.cpp 159
* V595 \[CWE\-476, CERT\-EXP12\-C\] The 'localVarCst' pointer was utilized before it was verified against nullptr\. Check lines: 77, 96\. AffineStructures\.cpp 77
* V595 \[CWE\-476, CERT\-EXP12\-C\] The 'Class' pointer was utilized before it was verified against nullptr\. Check lines: 58, 60\. Transforms\.cpp 58
* V595 \[CWE\-476, CERT\-EXP12\-C\] The 'CurPPLexer' pointer was utilized before it was verified against nullptr\. Check lines: 739, 743\. PPDirectives\.cpp 739
* Возможно, ошибок больше\. Я не всматривался\. Их неинтересно изучать и тем более — писать про них, так как они однотипные\.


</details>
**Фрагмент N18**

```cpp
Optional<SmallVector<ReassociationIndices>>
mlir::getReassociationIndicesForCollapse(ArrayRef<int64_t> sourceShape,
                                         ArrayRef<int64_t> targetShape) {
  unsigned sourceDim = 0;
  ....
  int64_t currTargetShape = targetShape[targetDim];
  while (sourceShape[sourceDim] != ShapedType::kDynamicSize &&
         prodOfCollapsedDims * sourceShape[sourceDim] < currTargetShape &&
         sourceDim < sourceShape.size()) {
    prodOfCollapsedDims *= sourceShape[sourceDim];
    currIndices.push_back(sourceDim++);
  }
  ....
}
```

Предупреждение PVS\-Studio: V781 \[CWE\-20, CERT\-API00\-C\] The value of the 'sourceDim' index is checked after it was used\. Perhaps there is a mistake in program logic\. ReshapeOpsUtils\.cpp 49

Потенциальный выход за границу массива\. Взглянем внимательно на это условие цикла:

```cpp
sourceShape[sourceDim] != ShapedType::kDynamicSize && .... &&
sourceDim < sourceShape.size()
```

Проверка индекса выполняется до обращения к элементу массива\. Более логичным и безопасным выглядит такой вариант кода:

```cpp
sourceDim < sourceShape.size() &&
sourceShape[sourceDim] != ShapedType::kDynamicSize && ....
```

**Фрагмент N19\-N36**

Неожиданно много опасных разыменований указателей встретилось в конструкторах классов\. Указатель смело разыменовывается при инициализации членов класса, а затем вдруг проверяется на равенство _nullptr_ в теле конструктора\. Пример:

```cpp
typedef std::shared_ptr<lldb_private::ValueObject> ValueObjectSP;

lldb_private::formatters::LibCxxMapIteratorSyntheticFrontEnd::
    LibCxxMapIteratorSyntheticFrontEnd(lldb::ValueObjectSP valobj_sp)
    : SyntheticChildrenFrontEnd(*valobj_sp), m_pair_ptr(), m_pair_sp() {
  if (valobj_sp)
    Update();
}
```

Предупреждение PVS\-Studio: V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 211, 212\. LibCxx\.cpp 211

<details>
   <summary>Схожий код можно увидеть здесь\\\.\\\.\\\.</summary>

* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 381, 382\. LibCxx\.cpp 381
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 551, 552\. LibCxx\.cpp 551
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 635, 636\. LibCxx\.cpp 635
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 49, 50\. LibCxxInitializerList\.cpp 49
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 70, 71\. LibCxxSpan\.cpp 70
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 311, 312\. LibCxxList\.cpp 311
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 207, 208\. LibCxxMap\.cpp 207
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 71, 72\. LibCxxVector\.cpp 71
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 174, 176\. LibCxxVector\.cpp 174
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 57, 59\. LibCxxUnorderedMap\.cpp 57
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 80, 82\. LibStdcpp\.cpp 80
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 183, 185\. LibStdcpp\.cpp 183
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 354, 355\. LibStdcpp\.cpp 354
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 464, 465\. NSArray\.cpp 464
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 608, 610\. NSArray\.cpp 608
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 404, 405\. NSSet\.cpp 404
* V664 \[CWE\-476, CERT\-EXP34\-C\] The 'valobj\_sp' pointer is being dereferenced on the initialization list before it is verified against null inside the body of the constructor function\. Check lines: 671, 673\. NSSet\.cpp 671


</details>
Этот код не обязательно приводит к проблемам\. Возможно, в конструкторы просто никогда не передаются нулевые указатели\. Однако хорошим и правильным код всё равно назвать нельзя\.

Въедливый читатель может возразить, что, даже если передать нулевой указатель, это не обязательно будет проблемой\. В конструкторе _SyntheticChildrenFrontEnd_ сохраняется ссылка на объект\. Если передать _nullptr_, то сохранится ссылка на несуществующий объект\. Однако если никак далее не "трогать" эту ссылку, то и проблемы не будет\. Некрасиво, но не критично\.

Нет, на такое допущение закладываться нельзя\. Разыменование нулевого указателя — это неопределённое поведение\. И невозможно предсказать, как оно проявит себя\. Например, компилятор видит, что указатель разыменовывается\. Значит, он точно не нулевой\. Следовательно, в целях оптимизации проверку "_if \(valobj\_sp\)_" можно удалить\. Подробнее эту тему я рассматривал в статье "[Разыменовывание нулевого указателя приводит к неопределённому поведению](https://pvs-studio.ru/ru/blog/posts/cpp/0306/)"\. И вот здесь тоже про это говорится "[What Every C Programmer Should Know About Undefined Behavior \#2/3](http://blog.llvm.org/2011/05/what-every-c-programmer-should-know_14.html)"\.

**Фрагмент N37**

```cpp
uint64_t NullabilityPayload = 0;

static constexpr const unsigned NullabilityKindMask = 0x3;

void addTypeInfo(unsigned index, NullabilityKind kind) {
  ....
  NullabilityPayload &=
    ~(NullabilityKindMask << (index * NullabilityKindSize));
  ....
}
```

Предупреждение PVS\-Studio: V784 \[CWE\-197, CERT\-INT31\-C\] The size of the bit mask is less than the size of the first operand\. This will cause the loss of higher bits\. Types\.h 529

Старшие биты в переменной _NullabilityPayload_ будут потеряны\. Сейчас поясню, как это произойдёт\.

Переменная _NullabilityKindMask_ представлена 32\-битным типом _unsigned_\. Сколько не сдвигай биты в этой переменной, результат всё равно будет иметь тип _unsigned_\. Оператор побитового НЕ \(\~\) также не изменит тип выражения\.

И только в момент выполнения оператора _&\=_ тип _unsigned_ будет расширен до типа _uint64\_t_\. Так как тип был беззнаковый, то старшие биты 64\-битного типа будут всегда нулевыми\. В результате выполнится операция:

```cpp
NullabilityPayload &= 0x00000000xxxxxxxx;
```

Что приведёт к сбросу старших битов в _NullabilityPayload_\. Правильный вариант кода:

```cpp
NullabilityPayload &=
  ~(static_cast<uint64_t>(NullabilityKindMask)
     << (index * NullabilityKindSize));
```

Другой вариант исправить код — это сразу сделать константу _NullabilityKindMask_ 64\-битной:

```cpp
static constexpr const uint64_t NullabilityKindMask = 0x3;
....
NullabilityPayload &=
  ~(NullabilityKindMask << (index * NullabilityKindSize));
```

**Фрагмент N38**

Бывает интересно заметить ошибку в тестах и понять, что он тестирует не совсем то, что задумывалось\. Это, кстати, хороший пример, когда статический анализ дополняет юнит\-тесты и методологию [TDD](https://ru.wikipedia.org/wiki/%D0%A0%D0%B0%D0%B7%D1%80%D0%B0%D0%B1%D0%BE%D1%82%D0%BA%D0%B0_%D1%87%D0%B5%D1%80%D0%B5%D0%B7_%D1%82%D0%B5%D1%81%D1%82%D0%B8%D1%80%D0%BE%D0%B2%D0%B0%D0%BD%D0%B8%D0%B5)\. Ведь никто не пишет тесты на тесты :\)\. К счастью, статический анализатор помогает находить ошибки не только в основном коде, но и в тестах\.

```cpp
TEST(RegisterContextMinidump, ConvertMinidumpContext_x86_64) {
  MinidumpContext_x86_64 Context;
  ....
  Context.rax = 0x0001020304050607;
  Context.rbx = 0x08090a0b0c0d0e0f;
  ....
  Context.eflags = 0x88898a8b;
  Context.cs = 0x8c8d;
  Context.fs = 0x8e8f;
  Context.gs = 0x9091;
  Context.ss = 0x9293;
  Context.ds = 0x9495;
  Context.ss = 0x9697;
  llvm::ArrayRef<uint8_t> ContextRef(reinterpret_cast<uint8_t *>(&Context),
                                     sizeof(Context));
  ....
}
```

Предупреждение PVS\-Studio: V519 \[CWE\-563, CERT\-MSC13\-C\] The 'Context\.ss' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 110, 112\. RegisterContextMinidumpTest\.cpp 112

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

```cpp
Context.ss = 0x9293;
....
Context.ss = 0x9697;
```

Опечатка\. В одном месте, по всей видимости, следовало написать _Context\.es_\.

**Фрагмент N39, N40**

В начале статьи я говорил, что не стал разбираться с некоторыми предупреждениями, такими как V547\. Но немного я всё\-таки их посмотрел и заметил, например, вот такую ошибку\. Её обнаружение возможно благодаря технологии data\-flow анализа, который учитывает возможные значения переменных в разных точках программы\. Если вам интересно, как вообще работает анализатор, предлагаю вашему вниманию заметку "[Технологии статического анализа кода PVS\-Studio](https://pvs-studio.ru/ru/blog/posts/0908/)"\.

```cpp
static std::unordered_multimap<char32_t, std::string>
loadDataFiles(const std::string &NamesFile, const std::string &AliasesFile) {
  ....
  auto FirstSemiPos = Line.find(';');
  if (FirstSemiPos == std::string::npos)                   // <=
    continue;
  auto SecondSemiPos = Line.find(';', FirstSemiPos + 1);
  if (FirstSemiPos == std::string::npos)                   // <=
    continue;
  ....
}
```

Предупреждение PVS\-Studio: V547 \[CWE\-570\] Expression 'FirstSemiPos \=\= std::string::npos' is always false\. UnicodeNameMappingGenerator\.cpp 46

Скорее всего, этот код писался с помощью Copy\-Paste\. Был размножен этот фрагмент:

```cpp
auto FirstSemiPos = Line.find(';');
if (FirstSemiPos == std::string::npos)
  continue;
```

Затем поменяли вызов функции _find_\. _First_ заменили на _Second_, но не везде\. В результате строчка

```cpp
if (FirstSemiPos == std::string::npos)
```

осталась без изменений\. Так как это условие уже проверялось выше, анализатор сообщает, что условие всегда ложно\. Такие ошибки очень легко просмотреть на code review, но они хорошо находятся с помощью PVS\-Studio\.

И ещё одна прям точно такая же ошибка:

```cpp
Status Host::ShellExpandArguments(ProcessLaunchInfo &launch_info) {
  ....
  auto data_sp = StructuredData::ParseJSON(output);
  if (!data_sp) {                                        // <=
    error.SetErrorString("invalid JSON");
    return error;
  }

  auto dict_sp = data_sp->GetAsDictionary();
  if (!data_sp) {                                        // <=
    error.SetErrorString("invalid JSON");
    return error;
  }
  ....
}
```

Предупреждение PVS\-Studio: V547 \[CWE\-570\] Expression '\!data\_sp' is always false\. Host\.cpp 248

**Фрагмент N41**

Для выявления двух предыдущих ошибок использовался анализ потока данных \(data\-flow анализ\)\. Для следующей ошибки используется ещё более изощрённая техника: символьное выполнение \(symbolic execution\)\. Смысл в том, что необязательно знать возможные значения переменных\. Чтобы определить, что условие истинно/ложно, можно решить уравнение\.

```cpp
void Sema::adjustMemberFunctionCC(QualType &T, bool IsStatic, bool IsCtorOrDtor,
                                  SourceLocation Loc) {
  ....
  CallingConv CurCC = FT->getCallConv();
  CallingConv ToCC = Context.getDefaultCallingConvention(IsVariadic, !IsStatic);

  if (CurCC == ToCC)
    return;
  ....
  CallingConv DefaultCC = 
    Context.getDefaultCallingConvention(IsVariadic, IsStatic);

  if (CurCC != DefaultCC || DefaultCC == ToCC)
    return;
  ....
}
```

Предупреждение PVS\-Studio: V560 \[CWE\-570\] A part of conditional expression is always false: DefaultCC \=\= ToCC\. SemaType\.cpp 7856

Давайте разбираться, почему анализатор решил, что правая часть условия всегда ложна\. Для удобства уберём всё лишнее и заменим имена:

```cpp
A = ....;
B = ....;
if (A == B) return;
C = ....;
if (A != C || C == B)
```

Разберём как работает этот код:

* есть 3 переменные A, B, C, значения которых нам неизвестны;
* но мы знаем, что если A \=\= B, то происходит выход из функции;
* следовательно, если функция продолжает выполняться, то A \!\= B;
* если A \!\= C, то, в силу [вычисления по короткой схеме](https://ru.wikipedia.org/wiki/%D0%92%D1%8B%D1%87%D0%B8%D1%81%D0%BB%D0%B5%D0%BD%D0%B8%D1%8F_%D0%BF%D0%BE_%D0%BA%D0%BE%D1%80%D0%BE%D1%82%D0%BA%D0%BE%D0%B9_%D1%81%D1%85%D0%B5%D0%BC%D0%B5), правое подвыражение не вычисляется;
* если правое подвыражение "C \=\= B" вычисляется, значит A \=\= C;
* если A \!\= B и A \=\= C, то С никак не может быть равно B\.

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

```cpp
if (CurCC != DefaultCC || DefaultCC != ToCC)
```

**Фрагмент N42**

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

```cpp
ObjectFileMachO::MachOCorefileAllImageInfos
ObjectFileMachO::GetCorefileAllImageInfos() {
  ....
  lldb::offset_t offset = MachHeaderSizeFromMagic(m_header.magic);
  ....
  uint32_t imgcount = m_data.GetU32(&offset);
  uint64_t entries_fileoff = m_data.GetU64(&offset);
  offset += 4; // uint32_t entries_size;
  offset += 4; // uint32_t unused;
  offset = entries_fileoff;                  // <=
  ....
}
```

Предупреждение PVS\-Studio: V519 \[CWE\-563, CERT\-MSC13\-C\] The 'offset' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 6888, 6890\. ObjectFileMachO\.cpp 6890

Очень странно в начале что\-то считать в переменной _offset_, а потом взять и перетереть её значение\. Возможно, здесь должно быть написано так:

```cpp
offset += 4; // uint32_t entries_size;
offset += 4; // uint32_t unused;
offset += entries_fileoff;
```

## Скачайте и попробуйте PVS\-Studio

Если вы активно используете предупреждения компиляторов, это очень хорошо\. Следующий шаг — внедрить специализированный статический анализатор PVS\-Studio\. Он поможет выявить на этапе создания кода ещё больше ошибок\. [Попробовать PVS\-Studio](https://pvs-studio.ru/ru/pvs-studio/try-free/)\.

Заодно предлагаю подписаться на [ежемесячную рассылку](https://pvs-studio.ru/ru/subscribe/), чтобы не пропускать интересные публикации и узнавать о новых возможностях PVS\-Studio\.