Проверка компилятора Ark, недавно сделанного Huawei с открытым исходным кодом

Летом 2019 года Huawei провела серию презентаций, анонсировав технологию Ark Compiler. Компания утверждает, что этот проект с открытым исходным кодом поможет разработчикам сделать систему Android и стороннее программное обеспечение более плавными и отзывчивыми. По традиции каждый новый многообещающий проект с открытым исходным кодом проходит через PVS-Studio, чтобы мы могли оценить качество его кода.

Введение

Компания Huawei впервые анонсировала компилятор Ark при запуске новых моделей смартфонов P30 и P30 Pro. Утверждается, что компилятор Ark улучшит беглость системы Android на 24% и скорость отклика на 44%. Сторонние приложения для Android также получат ускорение на 60% после перекомпиляции с помощью компилятора Ark. Версия проекта с открытым исходным кодом называется OpenArkCompiler; его исходный код доступен на Gitee, китайском форке GitHub.

Для проверки этого проекта я использовал статический анализатор кода PVS-Studio. Это инструмент для обнаружения ошибок и потенциальных уязвимостей в исходном коде программ C, C++, C# и Java.

Размер проекта 50 KLOC и проверка не заняла много времени. Небольшой проект означает скромные результаты: в статье речь пойдет об 11 из 39 предупреждений (высокого и среднего уровня).

В коде обнаружены дефекты

Предупреждение 1

V502 Возможно, оператор ?: работает не так, как ожидалось. Оператор ?: имеет более низкий приоритет, чем оператор ==. mir_parser.cpp 884

enum Opcode : uint8 {
  kOpUndef,
  ....
  OP_intrinsiccall,
  OP_intrinsiccallassigned,
  ....
  kOpLast,
};
bool MIRParser::ParseStmtIntrinsiccall(StmtNodePtr &stmt, bool isAssigned) {
  Opcode o = !isAssigned ? (....)
                         : (....);
  auto *intrnCallNode = mod.CurFuncCodeMemPool()->New<IntrinsiccallNode>(....);
  lexer.NextToken();
  if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
    intrnCallNode->SetIntrinsic(GetIntrinsicID(lexer.GetTokenKind()));
  } else {
    intrnCallNode->SetIntrinsic(static_cast<MIRIntrinsicID>(....));
  }
  ....
}

Нас интересует следующая часть:

if (o == !isAssigned ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

Приоритет оператора «==» выше, чем у тернарного оператора (?:). Поэтому условное выражение оценивается в неправильном порядке и эквивалентно следующему коду:

if ((o == !isAssigned) ? OP_intrinsiccall : OP_intrinsiccallassigned) {
  ....
}

Поскольку константы OP_intrinsiccall и OP_intrinsiccallassigned не равны нулю, условие будет возвращать true все время, что означает, что тело elseветвь является недостижимым кодом.

Предупреждение 2

V570 Переменная theDoubleVal присваивается самой себе. лексер.cpp 283

int64 theIntVal = 0;
float theFloatVal = 0.0;
double theDoubleVal = 0.0;
TokenKind MIRLexer
::GetFloatConst(uint32 valStart, uint32 startIdx, bool negative) {
  ....
  theIntVal = static_cast<int>(theFloatVal);
  theDoubleVal = static_cast<double>(theDoubleVal); // <=
  if (theFloatVal == -0) {
    theDoubleVal = -theDoubleVal;
  }
  ....
}

Переменная theDoubleVal присваивается самой себе без изменений. Разработчик, должно быть, намеревался вместо этого сохранить результат в theFloatVal, потому что именно эта переменная проверяется в следующей строке. Если это так, его также следует привести к типу float, а не double. Я думаю, что исправленная версия должна выглядеть так:

theFloatVal = static_cast<float>(theDoubleVal);
if (theFloatVal == -0) {
  theDoubleVal = -theDoubleVal;

или даже вот так, если программист просто прописал в условии не ту переменную:

if (theDoubleVal == -0) {
  theDoubleVal = -theDoubleVal;

Я все еще могу ошибаться; возможно, этот код следует исправить каким-то совершенно другим способом. Это выглядит неясным для стороннего программиста, такого как я.

Предупреждения 3–5

V524 Странно, что тело функции - полностью эквивалентно телу функции +. mpl_number.h 158

template <typename T, typename Tag>
inline Number<T, Tag> operator+(const Number<T, Tag> &lhs,
                                const Number<T, Tag> &rhs) {
  return Number<T, Tag>(lhs.get() + rhs.get());
}
template <typename T, typename Tag>
inline Number<T, Tag> operator-(const Number<T, Tag> &lhs,
                                const Number<T, Tag> &rhs) {
  return Number<T, Tag>(lhs.get() + rhs.get());
}

Заголовочный файл mpl_number.h содержит много повторяющегося кода с небольшими изменениями — и, конечно, ошибок. В этом примере операторы сложения и вычитания реализованы одинаково: программист забыл изменить знак операции в теле оператора вычитания.

Другие предупреждения этого типа:

• V524 Странно, что тело функции «-» полностью эквивалентно телу функции «+». mpl_number.h 233
• V524 Странно, что тело функции «-» полностью эквивалентно телу функции «+». mpl_number.h 238

Предупреждение 6

V560 Часть условного выражения всегда ложна: !firstImport. parser.cpp 2633

bool MIRParser::ParseMIRForImport() {
  ....
  if (paramIsIPA && firstImport) {
    BinaryMplt *binMplt = new BinaryMplt(mod);
    mod.SetBinMplt(binMplt);
    if (!(*binMplt).Import(...., paramIsIPA && !firstImport, paramIsComb)) {
      ....
    }
    ....
  }
  ....
}

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

paramIsIPA && !firstImport

Этот код либо содержит логическую ошибку, либо слишком сложен, и его можно упростить, передав константу false в функцию Import.

Предупреждение 7

V547 Выражение idx ›= 0 всегда верно. Значение беззнакового типа всегда ›= 0. lexer.h 129

char GetCharAtWithLowerCheck(uint32 idx) const {
  return idx >= 0 ? line[idx] : 0;
}

Эта проверка индексной переменной idx (›= 0) не имеет смысла, поскольку переменная не имеет знака. Возможно, оно предназначалось для сравнения с каким-то другим значением в качестве порога индексации в массив line, либо эту бессмысленную проверку нужно вообще убрать.

Предупреждение 8

V728 Излишняя проверка может быть упрощена. Оператор ‘||’ окружен противоположными выражениями ‘c != ‘\»’’ и ‘c == ‘\»’’. лексер.cpp 400

TokenKind MIRLexer::GetTokenWithPrefixDoubleQuotation() {
  ....
  char c = GetCurrentCharWithUpperCheck();
  while ((c != 0) &&
         (c != '\"' || (c == '\"' && GetCharAtWithLowerCheck(....) == '\\'))) {
    ....
  }
  ....
}

Анализатор обнаружил шаблон кода, который можно упростить. Это похоже на эту форму:

A || (!A && smth)

Выражение !A всегда будет оцениваться как true, поэтому исходное выражение можно упростить следующим образом:

while ((c != 0) && (c != '\"' || (GetCharAtWithLowerCheck(....) == '\\'))) {
  ....
}

Предупреждения 9–10

V728 Излишняя проверка может быть упрощена. ‘(A && !B) || (!A && B)’ эквивалентно выражению ‘bool(A) != bool(B)’. mir_nodes.cpp 1552

bool BinaryNode::Verify() const {
  ....
  if ((IsAddress(GetBOpnd(0)->GetPrimType()) &&
      !IsAddress(GetBOpnd(1)->GetPrimType()))
    ||
     (!IsAddress(GetBOpnd(0)->GetPrimType()) &&
       IsAddress(GetBOpnd(1)->GetPrimType()))) {
    ....
  }
  ....
}

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

if (IsAddress(GetBOpnd(0)->GetPrimType()) !=
    IsAddress(GetBOpnd(1)->GetPrimType()))
  ....
}

Аналогичным образом рефакторим еще один фрагмент кода:

• V728 Излишняя проверка может быть упрощена. ‘(А && В) || (!A && !B)’ эквивалентно выражению ‘bool(A) == bool(B)’. bin_mpl_import.cpp 702

Предупреждение 11

V1048 Переменной floatSpec-›floatStr присвоено такое же значение. input.inl 1356

static void SecInitFloatSpec(SecFloatSpec *floatSpec)
{
  floatSpec->floatStr = floatSpec->buffer;
  floatSpec->allocatedFloatStr = NULL;
  floatSpec->floatStrSize = sizeof(floatSpec->buffer) /
                            sizeof(floatSpec->buffer[0]);
  floatSpec->floatStr = floatSpec->buffer;
  floatSpec->floatStrUsedLen = 0;
}

Анализатор обнаружил две одинаковые инициализации переменной floatSpec-›floatStr. Я считаю, что вторую повторяющуюся строку можно удалить.

Вывод

Буквально несколько дней назад мы проверили еще один проект Huawei — Huawei Cloud DIS SDK. В настоящее время компания делает свои проекты открытыми, что является хорошей новостью для сообщества разработчиков. Такие проекты, как Ark Compiler или Harmony OS, очень молоды и еще не стали популярными, поэтому инвестиции в контроль качества кода на этом этапе должны быть очень выгодными, поскольку это может помочь избежать потенциальных уязвимостей и критики со стороны клиентов.

использованная литература

• Проверка LLVM, 2011
• Проверка LLVM, 2012
• Проверка НКУ, 2016
• Проверка LLVM, 2016
• Проверка PascalABC.NET, 2017
• Проверка Roslyn (платформа компилятора .NET), 2019
• Проверка LLVM, 2019