Во время презентаций летом 2019 года Huawei анонсировала технологию Ark Compiler. По заверениям представителей компании, этот проект с открытым исходным кодом позволяет существенно повысить плавность и отзывчивость Android и сторонних приложений. Новый интересный открытый проект по традиции должен пройти проверку качества кода с помощью PVS-Studio.
Впервые компилятор Huawei Ark был представлен вместе с запуском смартфонов Huawei P30 и P30 Pro. По заявлению Huawei, компилятор Ark повышает плавность работы Android на 24%, а скорость отклика – на 44%. При этом сторонние приложения для Android, после перекомпиляции с помощью Ark, могут работать на 60% быстрее. Открытый проект имеет название OpenArkCompiler. Его исходный код доступен на китайском аналоге сайта GitHub – Gitee.
Для проверки проекта использовался статический анализатор кода – PVS-Studio. Это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C++, C# и Java.
Анализатор быстро справился с проектом на 50К строк кода. Для маленького проекта и результаты анализа скромные: в статью вошло 11 предупреждений из 39 (уровней High и Medium).
Предупреждение 1
V502 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 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 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 продублировали много кода с незначительными изменениями. И, конечно, допустили ошибки. В этом примере операторы сложения и вычитания реализованы одинаково. В теле оператора вычитания забыли поменять знак операции.
Ещё несколько примеров приведу списком:
Предупреждение 6
V560 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 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 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 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()))
....
}Ещё одно место, где можно провести аналогичный рефакторинг:
Предупреждение 11
V1048 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. Компания Huawei начала активно открывать код для общественности, что не может не радовать сообщество разработчиков. Такие проекты, как Ark Compiler или Harmony OS, только появились и ещё не стали массовыми. Вложиться в контроль качества кода проектов на этом этапе будет очень выгодным, так как можно избежать появления потенциальных уязвимостей и критики пользователей.