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

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

Введение

Впервые компилятор Huawei Ark был представлен вместе с запуском смартфонов Huawei P30 и P30 Pro. По заявлению Huawei, компилятор Ark повышает плавность работы Android на 24%, а скорость отклика – на 44%. При этом сторонние приложения для Android, после перекомпиляции с помощью Ark, могут работать на 60% быстрее. Открытый проект имеет название OpenArkCompiler ^[1]. Его исходный код доступен на китайском аналоге сайта GitHub – Gitee ^[2].

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

Анализатор быстро справился с проектом на 50К строк кода. Для маленького проекта и результаты анализа скромные: в статью вошло 11 предупреждений из 39 (уровней High и Medium).

Обзор дефектов кода

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

V502 ^[4] Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '==' operator. 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 имеют ненулевые значения, то это условие всегда возвращает истинное значение. Тело ветки else является недостижимым кодом.

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

V570 ^[5] The 'theDoubleVal' variable is assigned to itself. lexer.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 ^[6] It is odd that the body of '-' function is fully equivalent to the body of '+' function. 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 It is odd that the body of '-' function is fully equivalent to the body of '+' function. mpl_number.h 233
• V524 It is odd that the body of '-' function is fully equivalent to the body of '+' function. mpl_number.h 238

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

V560 ^[7] A part of conditional expression is always false: !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. В этом случае выражение

paramIsIPA && !firstImport

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

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

V547 ^[8] Expression 'idx >= 0' is always true. Unsigned type value is always >= 0. lexer.h 129

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

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

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

V728 ^[9] An excessive check can be simplified. The '||' operator is surrounded by opposite expressions 'c != '"'' and 'c == '"''. lexer.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 ^[9] An excessive check can be simplified. The '(A && !B) || (!A && B)' expression is equivalent to the 'bool(A) != bool(B)' expression. 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 An excessive check can be simplified. The '(A && B) || (!A && !B)' expression is equivalent to the 'bool(A) == bool(B)' expression. bin_mpl_import.cpp 702

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

V1048 ^[10] The 'floatSpec->floatStr' variable was assigned the same value. 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;
}

Анализатор обнаружил 2 одинаковые строки инициализации переменной floatSpec->floatStr. Скорее всего, лишнюю строку можно удалить.

Заключение

Совсем недавно мы делали обзор кода Huawei Cloud DIS SDK ^[11]. Компания Huawei начала активно открывать код для общественности, что не может не радовать сообщество разработчиков. Такие проекты, как Ark Compiler или Harmony OS, только появились и ещё не стали массовыми. Вложиться в контроль качества кода проектов на этом этапе будет очень выгодным, так как можно избежать появления потенциальных уязвимостей и критики пользователей.

Дополнительные ссылки

1. Проверка LLVM в 2011 ^[12]
2. Проверка LLVM в 2012 ^[13]
3. Проверка GCC в 2016 ^[14]
4. Проверка LLVM в 2016 ^[15]
5. Проверка PascalABC.NET в 2017 ^[16]
6. Проверка Roslyn (.NET Compiler Platform) в 2019 ^[17]
7. Проверка LLVM в 2019 ^[18]

^[19]

Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Svyatoslav Razmyslov. Checking the Ark Compiler Recently Made Open-Source by Huawei ^[19].

Автор: Святослав

Источник ^[20]