Прошло је више од две године од последње провере кода ЛЛВМ пројекта помоћу нашег ПВС-Студио анализатора. Хајде да се уверимо да је ПВС-Студио анализатор и даље водећи алат за идентификацију грешака и потенцијалних рањивости. Да бисмо то урадили, проверићемо и пронаћи нове грешке у издању ЛЛВМ 8.0.0.
Чланак који треба написати
Да будем искрен, нисам желео да пишем овај чланак. Није занимљиво писати о пројекту који смо већ неколико пута проверавали (, , ). Боље је писати о нечем новом, али немам избора.
Сваки пут када се објави или ажурира нова верзија ЛЛВМ-а , добијамо питања следећег типа путем поште:
Погледајте, нова верзија Цланг Статиц Анализер је научила да пронађе нове грешке! Чини ми се да је релевантност коришћења ПВС-Студио све мања. Цланг проналази више грешака него раније и сустиже могућности ПВС-Студио. Шта мислите о овоме?
На ово увек желим да одговорим нешто попут:
Ни ми не седимо беспослени! Значајно смо унапредили могућности анализатора ПВС-Студио. Зато не брините, настављамо да водимо као и до сада.
Нажалост, ово је лош одговор. У њему нема доказа. И зато сада пишем овај чланак. Дакле, ЛЛВМ пројекат је још једном проверен и у њему су пронађене разне грешке. Сада ћу показати оне који су ми се чинили занимљиви. Цланг Статиц Анализер не може да пронађе ове грешке (или је крајње незгодно то учинити уз његову помоћ). Али можемо. Штавише, све ове грешке сам пронашао и записао за једно вече.
Али писање чланка је трајало неколико недеља. Једноставно нисам могао да се натерам да све ово ставим у текст :).
Иначе, ако вас занима које се технологије користе у анализатору ПВС-Студио за идентификацију грешака и потенцијалних рањивости, онда предлажем да се упознате са овим .
Нова и стара дијагностика
Као што је већ напоменуто, пре око две године ЛЛВМ пројекат је још једном проверен, а пронађене грешке су исправљене. Сада ће овај чланак представити нову групу грешака. Зашто су пронађене нове грешке? Постоје 3 разлога за ово:
- ЛЛВМ пројекат се развија, мења стари код и додаје нови код. Наравно, има нових грешака у модификованом и писаном коду. Ово јасно показује да статичку анализу треба користити редовно, а не повремено. Наши чланци добро показују могућности анализатора ПВС-Студио, али то нема никакве везе са побољшањем квалитета кода и смањењем трошкова исправљања грешака. Редовно користите статички анализатор кода!
- Довршавамо и унапређујемо постојећу дијагностику. Стога, анализатор може да идентификује грешке које није приметио током претходних скенирања.
- У ПВС-Студију се појавила нова дијагностика која није постојала пре 2 године. Одлучио сам да их истакнем у посебном одељку како бих јасно показао развој ПВС-Студија.
Дефекти идентификовани дијагностиком који су постојали пре 2 године
Фрагмент Н1: Цопи-Пасте
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}Упозорење ПВС-Студио: [ЦВЕ-570] Постоје идентични подизрази 'Наме.стартсвитх("авк512.маск.пермвар.")' лево и десно од '||' оператер. АутоУпграде.цпп 73
Двоструко се проверава да име почиње поднизом "авк512.маск.пермвар.". У другој провери су очигледно хтели да напишу нешто друго, али су заборавили да исправе копирани текст.
Фрагмент Н2: Грешка у куцању
enum CXNameRefFlags {
CXNameRange_WantQualifier = 0x1,
CXNameRange_WantTemplateArgs = 0x2,
CXNameRange_WantSinglePiece = 0x4
};
void AnnotateTokensWorker::HandlePostPonedChildCursor(
CXCursor Cursor, unsigned StartTokenIndex) {
const auto flags = CXNameRange_WantQualifier | CXNameRange_WantQualifier;
....
}Упозорење ПВС-Студио: В501 Постоје идентични подизрази 'ЦКСНамеРанге_ВантКуалифиер' лево и десно од '|' оператер. ЦИндек.цпп 7245
Због грешке у куцању иста именована константа се користи два пута ЦКСНамеРанге_ВантКуалифиер.
Фрагмент Н3: Забуна са приоритетом оператора
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Упозорење ПВС-Студио: [ЦВЕ-783] Можда оператор '?:' ради на другачији начин него што се очекивало. Оператор '?:' има нижи приоритет од оператора '=='. ППЦТаргетТрансформИнфо.цпп 404
По мом мишљењу, ово је веома лепа грешка. Да, знам да имам чудне идеје о лепоти :).
Сада, према , израз се процењује на следећи начин:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Са практичне тачке гледишта, такав услов нема смисла, јер се може свести на:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Ово је јасна грешка. Највероватније су желели да упореде 0/1 са променљивом индекс. Да бисте поправили код, потребно је да додате заграде око тернарног оператора:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Иначе, тернарни оператор је веома опасан и изазива логичке грешке. Будите веома опрезни са тим и немојте бити похлепни са заградама. Погледао сам ову тему детаљније , у поглављу „Чувајте се ?: Оператор и ставите га у заграде.“
Фрагмент Н4, Н5: Нулл показивач
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast<TypedInit>(Result);
....
LHS = dyn_cast<TypedInit>(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() +
"' to string");
return nullptr;
}
....
}Упозорење ПВС-Студио: [ЦВЕ-476] Дереференцирање нултог показивача 'ЛХС' може се десити. ТГПарсер.цпп 2152
Ако показивач ЛХС је нулл, треба издати упозорење. Међутим, уместо тога, овај исти нул показивач ће бити дереференциран: ЛХС->гетАсСтринг().
Ово је врло типична ситуација када је грешка скривена у обрађивачу грешака, пошто је нико не тестира. Статички анализатори проверавају сав доступан код, без обзира колико често се користи. Ово је веома добар пример како статичка анализа допуњује друге технике тестирања и заштите од грешака.
Слична грешка у руковању показивачем РХД дозвољено у коду испод: В522 [ЦВЕ-476] Дереференцирање нулте показивача 'РХС' може се десити. ТГПарсер.цпп 2186
Фрагмент Н6: Коришћење показивача након померања
static Expected<bool>
ExtractBlocks(....)
{
....
std::unique_ptr<Module> ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // <=
MiscompiledFunctions.clear();
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Function not found??");
MiscompiledFunctions.push_back(NewF);
}
....
}Упозорење ПВС-Студио: В522 [ЦВЕ-476] Дереференцирање нултог показивача 'ПрогЦлоне' може да се деси. Мисцомпилатион.цпп 601
На почетку паметан показивач ПрогЦлоне престаје да поседује објекат:
BD.setNewProgram(std::move(ProgClone));У ствари, сада ПрогЦлоне је нулти показивач. Према томе, нулти показивач би требало да се појави одмах испод:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Али, у стварности, то се неће догодити! Имајте на уму да се петља заправо не извршава.
На почетку контејнера МисцомпиледФунцтионс очишћено:
MiscompiledFunctions.clear();Затим, величина овог контејнера се користи у стању петље:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Лако је видети да петља не почиње. Мислим да је и ово грешка и код треба да буде другачије написан.
Чини се да смо наишли на тај фамозни паритет грешака! Једна грешка маскира другу :).
Фрагмент Н7: Коришћење показивача након померања
static Expected<bool> TestOptimizer(BugDriver &BD, std::unique_ptr<Module> Test,
std::unique_ptr<Module> Safe) {
outs() << " Optimizing functions being tested: ";
std::unique_ptr<Module> Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Error running this sequence of passes"
<< " on the input program!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}ПВС-Студио упозорење: В522 [ЦВЕ-476] Дереференцирање нултог показивача 'Тест' може се десити. Мисцомпилатион.цпп 709
Опет иста ситуација. Прво се помера садржај објекта, а затим се користи као да се ништа није догодило. Све чешће видим ову ситуацију у програмском коду након што се у Ц++ појавила семантика покрета. Због тога волим језик Ц++! Све је више нових начина да себи одбијете ногу. ПВС-Студио анализатор ће увек имати посла :).
Фрагмент Н8: Нулл показивач
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "<unknown-type>";
else
Type->dump(*this);
}ПВС-Студио упозорење: В522 [ЦВЕ-476] Дереференцирање нултог показивача 'Типе' може доћи. ПреттиФунцтионДумпер.цпп 233
Поред руковаоца грешкама, функције исписа за отклањање грешака се обично не тестирају. Пред нама је управо такав случај. Функција чека корисника, који ће, уместо да решава своје проблеме, бити приморан да је поправи.
Тачно:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Фрагмент Н9: Нулл показивач
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector<Record *> &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // <=
PrintFatalError(Twine("Field '") + Field.Name + "' of table '" +
Table.Name + "' has incompatible type: " +
Ty->getAsString() + " vs. " + // <=
TI->getType()->getAsString());
....
}ПВС-Студио упозорење: В522 [ЦВЕ-476] Дереференцирање нулте показивача 'Ти' може доћи. СеарцхаблеТаблеЕмиттер.цпп 614
Мислим да је све јасно и не захтева објашњење.
Фрагмент Н10: Грешка у куцању
bool FormatTokenLexer::tryMergeCSharpNullConditionals() {
....
auto &Identifier = *(Tokens.end() - 2);
auto &Question = *(Tokens.end() - 1);
....
Identifier->ColumnWidth += Question->ColumnWidth;
Identifier->Type = Identifier->Type; // <=
Tokens.erase(Tokens.end() - 1);
return true;
}Упозорење ПВС-Студио: Променљива 'Идентифиер->Типе' је додељена самој себи. ФорматТокенЛекер.цпп 249
Нема смисла додељивати променљиву самој себи. Највероватније су хтели да напишу:
Identifier->Type = Question->Type;Фрагмент Н11: Сумњив прелом
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Упозорење ПВС-Студио: [ЦВЕ-478] Размислите о прегледу изјаве 'свитцх'. Могуће је да недостаје први оператор 'цасе'. СистемЗАсмПарсер.цпп 652
На почетку је веома сумњив оператер разбити. Да ли сте заборавили да напишете још нешто овде?
Фрагмент Н12: Провера показивача након дереференцирања
InlineCost AMDGPUInliner::getInlineCost(CallSite CS) {
Function *Callee = CS.getCalledFunction();
Function *Caller = CS.getCaller();
TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
if (!Callee || Callee->isDeclaration())
return llvm::InlineCost::getNever("undefined callee");
....
}Упозорење ПВС-Студио: [ЦВЕ-476] 'Цаллее' показивач је коришћен пре него што је верификован у односу на нуллптр. Контролне линије: 172, 174. АМДГПУИнлине.цпп 172
Поинтер Цаллее на почетку се дереференцира у тренутку позива функције гетТТИ.
А онда се испоставља да овај показивач треба проверити на једнакост нуллптр:
if (!Callee || Callee->isDeclaration())Али касно је…
Фрагмент Н13 - Н...: Провера показивача након дереференцирања
Ситуација о којој се говори у претходном фрагменту кода није јединствена. Овде се појављује:
static Value *optimizeDoubleFP(CallInst *CI, IRBuilder<> &B,
bool isBinary, bool isPrecise = false) {
....
Function *CalleeFn = CI->getCalledFunction();
StringRef CalleeNm = CalleeFn->getName(); // <=
AttributeList CalleeAt = CalleeFn->getAttributes();
if (CalleeFn && !CalleeFn->isIntrinsic()) { // <=
....
}Упозорење ПВС-Студио: В595 [ЦВЕ-476] 'ЦаллееФн' показивач је коришћен пре него што је верификован у односу на нуллптр. Линије за проверу: 1079, 1081. СимплифиЛибЦаллс.цпп 1079
И овде:
void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
const Decl *Tmpl, Decl *New,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *OuterMostScope) {
....
NamedDecl *ND = dyn_cast<NamedDecl>(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <=
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember()); // <=
....
}ПВС-Студио упозорење: В595 [ЦВЕ-476] 'НД' показивач је коришћен пре него што је верификован у односу на нуллптр. Контролни редови: 532, 534. СемаТемплатеИнстантиатеДецл.цпп 532
И овде:
- В595 [ЦВЕ-476] 'У' показивач је коришћен пре него што је верификован у односу на нуллптр. Контролне линије: 404, 407. ДВАРФормВалуе.цпп 404
- В595 [ЦВЕ-476] 'НД' показивач је коришћен пре него што је верификован у односу на нуллптр. Линије за проверу: 2149, 2151. СемаТемплатеИнстантиате.цпп 2149
А онда сам постао незаинтересован за проучавање упозорења са бројем В595. Тако да не знам да ли има још сличних грешака осим овде наведених. Највероватније постоји.
Фрагмент Н17, Н18: Сумњив помак
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Упозорење ПВС-Студио: [ЦВЕ-190] Размислите о прегледу израза '~(Сизе - 1) << 1'. Померање бита 32-битне вредности са накнадним проширењем на 64-битни тип. ААрцх64АддрессингМодес.х 260
Можда није грешка и код ради тачно како је предвиђено. Али ово је очигледно веома сумњиво место и треба га проверити.
Рецимо променљива veličina је једнако 16, а онда је аутор кода планирао да га добије у променљивој НИммс вредност:
1111111111111111111111111111111111111111111111111111111111100000
Међутим, у стварности резултат ће бити:
0000000000000000000000000000000011111111111111111111111111100000
Чињеница је да се сви прорачуни одвијају користећи 32-битни неозначени тип. И тек тада ће овај 32-битни непотписани тип бити имплицитно проширен на уинт64_т. У овом случају, најважнији битови ће бити нула.
Ситуацију можете поправити овако:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Слична ситуација: В629 [ЦВЕ-190] Размислите о прегледу израза 'Иммр << 6'. Померање бита 32-битне вредности са накнадним проширењем на 64-битни тип. ААрцх64АддрессингМодес.х 269
Фрагмент Н19: Недостаје кључна реч друго?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token.
// Skip it.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Handle optional arguments
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Invalid operand type");
}
....
}Упозорење ПВС-Студио: [ЦВЕ-670] Размислите о инспекцији логике апликације. Могуће је да недостаје кључна реч „друго“. АМДГПУАсмПарсер.цпп 5655
Овде нема грешке. Од тадашњег блока првог if завршава са наставити, онда нема везе, постоји кључна реч друго или не. У сваком случају, код ће радити исто. Још увек промашен друго чини код нејаснијим и опаснијим. Ако у будућности наставити нестане, код ће почети да ради потпуно другачије. По мом мишљењу боље је додати друго.
Фрагмент Н20: Четири грешке у куцању истог типа
LLVM_DUMP_METHOD void Symbol::dump(raw_ostream &OS) const {
std::string Result;
if (isUndefined())
Result += "(undef) ";
if (isWeakDefined())
Result += "(weak-def) ";
if (isWeakReferenced())
Result += "(weak-ref) ";
if (isThreadLocalValue())
Result += "(tlv) ";
switch (Kind) {
case SymbolKind::GlobalSymbol:
Result + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClass:
Result + "(ObjC Class) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClassEHType:
Result + "(ObjC Class EH) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCInstanceVariable:
Result + "(ObjC IVar) " + Name.str(); // <=
break;
}
OS << Result;
}ПВС-Студио упозорења:
- В655 [ЦВЕ-480] Низови су спојени, али се не користе. Размислите о прегледу израза „Резултат + Име.стр()“. Симбол.цпп 32
- В655 [ЦВЕ-480] Низови су спојени, али се не користе. Размислите о прегледу израза 'Резултат + "(ОбјЦ Цласс)" + Наме.стр()'. Симбол.цпп 35
- В655 [ЦВЕ-480] Низови су спојени, али се не користе. Размислите о прегледу израза 'Резултат + "(ОбјЦ Цласс ЕХ) " + Наме.стр()'. Симбол.цпп 38
- В655 [ЦВЕ-480] Низови су спојени, али се не користе. Размислите о прегледу израза 'Резултат + "(ОбјЦ ИВар)" + Наме.стр()'. Симбол.цпп 41
Случајно се користи оператор + уместо оператора +=. Резултат су дизајни који су лишени значења.
Фрагмент Н21: Недефинисано понашање
static void getReqFeatures(std::map<StringRef, int> &FeaturesMap,
const std::vector<Record *> &ReqFeatures) {
for (auto &R : ReqFeatures) {
StringRef AsmCondString = R->getValueAsString("AssemblerCondString");
SmallVector<StringRef, 4> Ops;
SplitString(AsmCondString, Ops, ",");
assert(!Ops.empty() && "AssemblerCondString cannot be empty");
for (auto &Op : Ops) {
assert(!Op.empty() && "Empty operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Покушајте сами да пронађете опасну шифру. А ово је слика за одвраћање пажње како не бисте одмах погледали одговор:
Упозорење ПВС-Студио: [ЦВЕ-758] Користи се опасна конструкција: 'ФеатуресМап[Оп] = ФеатуресМап.сизе()', где је 'ФеатуресМап' класе 'мап'. Ово може довести до недефинисаног понашања. РИСЦВЦомпрессИнстЕмиттер.цпп 490
Проблемска линија:
FeaturesMap[Op] = FeaturesMap.size();Ако елемент Op није пронађен, онда се креира нови елемент у мапи и тамо се уписује број елемената у овој мапи. Само је непознато да ли ће функција бити позвана величина пре или после додавања новог елемента.
Фрагмент Н22-Н24: Поновљени задаци
Error MachOObjectFile::checkSymbolTable() const {
....
} else {
MachO::nlist STE = getSymbolTableEntry(SymDRI);
NType = STE.n_type; // <=
NType = STE.n_type; // <=
NSect = STE.n_sect;
NDesc = STE.n_desc;
NStrx = STE.n_strx;
NValue = STE.n_value;
}
....
}Упозорење ПВС-Студио: [ЦВЕ-563] Променљивој 'НТипе' се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 1663, 1664. МацхООбјецтФиле.цпп 1664
Мислим да овде нема праве грешке. Само непотребан поновљени задатак. Али ипак грешка.
Слично:
- В519 [ЦВЕ-563] Променљивој 'Б.НДесц' се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 1488, 1489. ллвм-нм.цпп 1489
- В519 [ЦВЕ-563] Променљивој се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 59, 61. цофф2иамл.цпп 61
Фрагмент Н25-Н27: Још преименовања
Погледајмо сада мало другачију верзију прерасподеле.
bool Vectorizer::vectorizeLoadChain(
ArrayRef<Instruction *> Chain,
SmallPtrSet<Instruction *, 16> *InstructionsProcessed) {
....
unsigned Alignment = getAlignment(L0);
....
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
StackAdjustedAlignment,
DL, L0, nullptr, &DT);
if (NewAlign != 0)
Alignment = NewAlign;
Alignment = NewAlign;
....
}Упозорење ПВС-Студио: В519 [ЦВЕ-563] Варијабли 'Алигнмент' се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 1158, 1160. ЛоадСтореВецторизер.цпп 1160
Ово је веома чудан код који очигледно садржи логичку грешку. На почетку, променљиво Сврставање вредност се додељује у зависности од услова. А онда се задатак поново јавља, али сада без икакве провере.
Сличне ситуације се могу видети овде:
- В519 [ЦВЕ-563] Варијабли 'Еффецтс' се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 152, 165. ВебАссемблиРегСтацкифи.цпп 165
- В519 [ЦВЕ-563] Променљивој 'ЕкпецтНоДерефЦхунк' се додељују вредности два пута узастопно. Можда је ово грешка. Контролне линије: 4970, 4973. СемаТипе.цпп 4973
Фрагмент Н28: Увек истинито стање
static int readPrefixes(struct InternalInstruction* insn) {
....
uint8_t byte = 0;
uint8_t nextByte;
....
if (byte == 0xf3 && (nextByte == 0x88 || nextByte == 0x89 ||
nextByte == 0xc6 || nextByte == 0xc7)) {
insn->xAcquireRelease = true;
if (nextByte != 0x90) // PAUSE instruction support // <=
break;
}
....
}Упозорење ПВС-Студио: [ЦВЕ-571] Израз 'нектБите != 0к90' је увек тачан. Кс86ДисассемблерДецодер.цпп 379
Проверавање нема смисла. Променљива нектБите увек није једнак вредности 0x90, што следи из претходне провере. Ово је нека врста логичке грешке.
Фрагмент Н29 - Н...: Увек тачни/нетачни услови
Анализатор издаје многа упозорења да је читаво стање () или његов део () је увек тачно или нетачно. Често то нису праве грешке, већ једноставно неуредан код, резултат проширења макроа и слично. Међутим, има смисла погледати сва ова упозорења, јер се праве логичке грешке с времена на време дешавају. На пример, овај део кода је сумњив:
static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
uint64_t Address, const void *Decoder) {
DecodeStatus S = MCDisassembler::Success;
if (RegNo > 13)
return MCDisassembler::Fail;
if ((RegNo & 1) || RegNo == 0xe)
S = MCDisassembler::SoftFail;
....
}Упозорење ПВС-Студио: [ЦВЕ-570] Део условног израза је увек нетачан: РегНо == 0ке. АРМДисассемблер.цпп 939
Константа 0кЕ је вредност 14 у децималу. Преглед РегНо == 0ке нема смисла јер ако РегНо > 13, онда ће функција завршити своје извршавање.
Било је много других упозорења са ИД-овима В547 и В560, али као и са , нисам био заинтересован за проучавање ових упозорења. Већ је било јасно да имам довољно материјала да напишем чланак :). Стога је непознато колико грешака овог типа може бити идентификовано у ЛЛВМ-у помоћу ПВС-Студио.
Даћу вам пример зашто је проучавање ових покретача досадно. Анализатор је потпуно у праву када је издао упозорење за следећи код. Али ово није грешка.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Упозорење ПВС-Студио: В547 [ЦВЕ-570] Израз '!ХасЕррор' је увек лажан. УнвраппедЛинеПарсер.цпп 1635
Фрагмент Н30: Сумњив повратак
static bool
isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) {
for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg),
E = MRI.def_instr_end(); It != E; ++It) {
return (*It).isImplicitDef();
}
....
}Упозорење ПВС-Студио: [ЦВЕ-670] Безусловни 'повратак' унутар петље. Р600ОптимизеВецторРегистерс.цпп 63
Ово је или грешка или специфична техника која има за циљ да објасни нешто програмерима који читају код. Овај дизајн ми ништа не објашњава и изгледа веома сумњиво. Боље је не писати тако :).
Уморан? Онда је време да направите чај или кафу.
Дефекти идентификовани новом дијагностиком
Мислим да је довољно 30 активација старе дијагностике. Хајде сада да видимо шта се занимљиво може наћи са новом дијагностиком која се после појавила у анализатору провере. Укупно је 66 дијагностика опште намене додато у Ц++ анализатор током овог времена.
Фрагмент Н31: Недоступан код
Error CtorDtorRunner::run() {
....
if (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}Упозорење ПВС-Студио: [ЦВЕ-561] Откривен је недоступан код. Могуће је да постоји грешка. ЕкецутионУтилс.цпп 146
Као што видите, обе гране оператера if завршава позивом оператеру повратак. Сходно томе, контејнер ЦторДторсБиПриорити никада неће бити очишћена.
Фрагмент Н32: Недоступан код
bool LLParser::ParseSummaryEntry() {
....
switch (Lex.getKind()) {
case lltok::kw_gv:
return ParseGVEntry(SummaryID);
case lltok::kw_module:
return ParseModuleEntry(SummaryID);
case lltok::kw_typeid:
return ParseTypeIdEntry(SummaryID); // <=
break; // <=
default:
return Error(Lex.getLoc(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}ПВС-Студио упозорење: В779 [ЦВЕ-561] Откривен недоступан код. Могуће је да постоји грешка. ЛЛПарсер.цпп 835
Занимљива ситуација. Хајде да прво погледамо ово место:
return ParseTypeIdEntry(SummaryID);
break;На први поглед се чини да овде нема грешке. Изгледа као оператер разбити овде постоји још један и можете га једноставно избрисати. Међутим, није све тако једноставно.
Анализатор издаје упозорење на редовима:
Lex.setIgnoreColonInIdentifiers(false);
return false;И заиста, овај код је недостижан. Сви случајеви у пребацити завршава позивом оператера повратак. А сада бесмислено сам разбити не изгледа тако безазлено! Можда би једна од грана требало да се заврши са разбити, није на повратак?
Фрагмент Н33: Насумично ресетовање високих битова
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}Упозорење ПВС-Студио: Величина битне маске је мања од величине првог операнда. Ово ће узроковати губитак виших битова. РунтимеДилд.цпп 815
Имајте на уму да је функција гетСтубАлигнмент враћа тип непотписан. Хајде да израчунамо вредност израза, под претпоставком да функција враћа вредност 8:
~(гетСтубАлигнмент() - 1)
~(8у-1)
0кФФФФФФФФ8у
Сада приметите да променљива ДатаСизе има 64-битни неозначени тип. Испоставило се да ће приликом извођења операције ДатаСизе & 0кФФФФФФФ8у, сва тридесет два бита високог реда бити ресетована на нулу. Највероватније, ово није оно што је програмер желео. Претпостављам да је хтео да израчуна: ДатаСизе & 0кФФФФФФФФФФФФФФФ8у.
Да бисте исправили грешку, требало би да напишете ово:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Или тако:
DataSize &= ~(getStubAlignment() - 1ULL);Фрагмент Н34: Неуспешно пребацивање експлицитног типа
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Unexpected scaling factor");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}Упозорење ПВС-Студио: [ЦВЕ-190] Могуће преливање. Размислите о пребацивању операнада оператора 'НумЕлтс * Сцале' на тип 'сизе_т', а не резултат. Кс86ИСелЛоверинг.х 1577
Експлицитно преливање типа се користи да би се избегло преливање при множењу променљивих типа инт. Међутим, експлицитно пребацивање типа овде не штити од преливања. Прво ће се променљиве помножити, а тек онда ће 32-битни резултат множења бити проширен на тип .
Фрагмент Н35: Неуспешно копирање и лепљење
Instruction *InstCombiner::visitFCmpInst(FCmpInst &I) {
....
if (!match(Op0, m_PosZeroFP()) && isKnownNeverNaN(Op0, &TLI)) {
I.setOperand(0, ConstantFP::getNullValue(Op0->getType()));
return &I;
}
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op0->getType())); // <=
return &I;
}
....
}[ЦВЕ-682] Пронађена су два слична фрагмента кода. Можда је ово грешка у куцању и треба користити променљиву 'Оп1' уместо 'Оп0'. ИнстЦомбинеЦомпарес.цпп 5507
Ова нова занимљива дијагностика идентификује ситуације у којима је део кода копиран и нека имена у њему су почела да се мењају, али на једном месту то нису исправили.
Имајте на уму да су се у другом блоку променили ОпКСНУМКС на ОпКСНУМКС. Али на једном месту то нису поправили. Највероватније је требало да буде написано овако:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Фрагмент Н36: Променљива конфузија
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Упозорење ПВС-Студио: [ЦВЕ-563] Променљива 'Моде' је додељена, али се не користи до краја функције. СИМодеРегистер.цпп 48
Веома је опасно давати аргументима функције иста имена као и члановима класе. Врло је лако збунити се. Пред нама је управо такав случај. Овај израз нема смисла:
Mode &= Mask;Аргумент функције се мења. То је све. Овај аргумент се више не користи. Највероватније је требало да то напишете овако:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Фрагмент Н37: Променљива конфузија
class SectionBase {
....
uint64_t Size = 0;
....
};
class SymbolTableSection : public SectionBase {
....
};
void SymbolTableSection::addSymbol(Twine Name, uint8_t Bind, uint8_t Type,
SectionBase *DefinedIn, uint64_t Value,
uint8_t Visibility, uint16_t Shndx,
uint64_t Size) {
....
Sym.Value = Value;
Sym.Visibility = Visibility;
Sym.Size = Size;
Sym.Index = Symbols.size();
Symbols.emplace_back(llvm::make_unique<Symbol>(Sym));
Size += this->EntrySize;
}Упозорење ПВС-Студио: В1001 [ЦВЕ-563] Променљива 'Величина' је додељена, али се не користи до краја функције. Објецт.цпп 424
Ситуација је слична претходној. Требало би да буде написано:
this->Size += this->EntrySize;Фрагмент Н38-Н47: Заборавили су да провере индекс
Раније смо погледали примере дијагностичког покретања . Његова суштина је да се показивач на почетку дереференцира, а тек онда проверава. Млада дијагностика је супротан по значењу, али открива и доста грешака. Он идентификује ситуације у којима је показивач на почетку проверен, а затим заборављен да то уради. Хајде да погледамо такве случајеве који се налазе унутар ЛЛВМ-а.
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef<const Value *> Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}Упозорење ПВС-Студио: В1004 [ЦВЕ-476] Показивач 'Птр' је коришћен небезбедно након што је верификован у односу на нуллптр. Контролне линије: 729, 738. ТаргетТрансформИнфоИмпл.х 738
Вариабле pTR могу бити једнаки нуллптр, о чему сведочи чек:
if (Ptr != nullptr)Међутим, испод овог показивача се дереференцира без прелиминарне провере:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Хајде да размотримо још један сличан случај.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast<FunctionDecl>(GD.getDecl());
SmallVector<QualType, 16> ArgTypes;
if (FD) // <=
for (const ParmVarDecl *Parm : FD->parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs<FunctionType>()->getCallConv(); // <=
....
}Упозорење ПВС-Студио: В1004 [ЦВЕ-476] 'ФД' показивач је небезбедно коришћен након што је верификован у односу на нуллптр. Контролне линије: 3228, 3231. ЦГДебугИнфо.цпп 3231
Обратите пажњу на знак FD. Сигуран сам да је проблем јасно видљив и није потребно посебно објашњење.
И даље:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast<PointerType>(Ptr.getType());
if (!PtrTy) { // <=
Result = Polynomial();
BasePtr = nullptr;
}
unsigned PointerBits =
DL.getIndexSizeInBits(PtrTy->getPointerAddressSpace()); // <=
....
}Упозорење ПВС-Студио: В1004 [ЦВЕ-476] 'ПтрТи' показивач је коришћен небезбедно након што је верификован у односу на нуллптр. Контролне линије: 960, 965. ИнтерлеаведЛоадЦомбинеПасс.цпп 965
Како се заштитити од таквих грешака? Будите пажљивији на Цоде-Ревиев и користите ПВС-Студио статички анализатор да редовно проверавате свој код.
Нема смисла цитирати друге фрагменте кода са грешкама овог типа. У чланку ћу оставити само листу упозорења:
- В1004 [ЦВЕ-476] Показивач 'Екпр' је небезбедно коришћен након што је верификован у односу на нуллптр. Контролне линије: 1049, 1078. ДебугИнфоМетадата.цпп 1078
- В1004 [ЦВЕ-476] 'ПИ' показивач је небезбедно коришћен након што је верификован у односу на нуллптр. Контролне линије: 733, 753. ЛегациПассМанагер.цпп 753
- В1004 [ЦВЕ-476] Показивач 'СтатепоинтЦалл' је небезбедно коришћен након што је верификован у односу на нуллптр. Контролне линије: 4371, 4379. Верифиер.цпп 4379
- В1004 [ЦВЕ-476] 'РВ' показивач је небезбедно коришћен након што је верификован у односу на нуллптр. Контролне линије: 2263, 2268. ТГПарсер.цпп 2268
- В1004 [ЦВЕ-476] Показивач 'ЦаллееФн' је небезбедно коришћен након што је верификован у односу на нуллптр. Линије за проверу: 1081, 1096. СимплифиЛибЦаллс.цпп 1096
- В1004 [ЦВЕ-476] 'ТЦ' показивач је коришћен небезбедно након што је верификован у односу на нуллптр. Контролни редови: 1819, 1824. Дривер.цпп 1824
Фрагмент Н48-Н60: Није критично, али је квар (могуће цурење меморије)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Упозорење ПВС-Студио: [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Стратегије' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-исел-фуззер.цпп 58
Да бисте додали елемент на крај контејнера као што је стд::вецтор > не можеш само да пишеш ккк.пусх_бацк(нови Кс), пошто не постоји имплицитна конверзија из X* в стд::уникуе_птр.
Уобичајено решење је писање ккк.емплаце_бацк(ново Кс)пошто саставља: метод емплаце_бацк конструише елемент директно из његових аргумената и стога може да користи експлицитне конструкторе.
Није безбедно. Ако је вектор пун, меморија се поново додељује. Операција прерасподеле меморије може да не успе, што ће резултирати избацивањем изузетка стд::бад_аллоц. У овом случају, показивач ће бити изгубљен и креирани објекат никада неће бити обрисан.
Безбедно решење је стварање уникуе_птркоји ће поседовати показивач пре него што вектор покуша да поново додели меморију:
xxx.push_back(std::unique_ptr<X>(new X))Од Ц++14, можете користити 'стд::маке_уникуе':
xxx.push_back(std::make_unique<X>())Ова врста дефекта није критична за ЛЛВМ. Ако меморија не може да се додели, компајлер ће једноставно стати. Међутим, за апликације са дугим , који се не може само прекинути ако алокација меморије не успе, ово може бити права гадна грешка.
Дакле, иако овај код не представља практичну претњу за ЛЛВМ, сматрао сам корисним да причам о овом обрасцу грешке и да је ПВС-Студио анализатор научио да га идентификује.
Остала упозорења ове врсте:
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Пролази' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ПассМанагер.х 546
- В1023 [ЦВЕ-460] Показивач без власника се додаје у 'ААс' контејнер методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. АлиасАналисис.х 324
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Ентриес' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ДВАРФДебугФраме.цпп 519
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'АллЕдгес' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ЦФГМСТ.х 268
- В1023 [ЦВЕ-460] Показивач без власника се додаје у 'ВМапс' контејнер методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. СимплеЛоопУнсвитцх.цпп 2012
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Рецордс' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ФДРЛогБуилдер.х 30
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'ПендингСубмодулес' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. МодулеМап.цпп 810
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Објекти' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ДебугМап.цпп 88
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Стратегије' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-исел-фуззер.цпп 60
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 685
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 686
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 688
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 689
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 690
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 691
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 692
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 693
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Модификатори' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ллвм-стресс.цпп 694
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Операнди' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ГлобалИСелЕмиттер.цпп 1911
- В1023 [ЦВЕ-460] Показивач без власника се додаје у 'Стасх' контејнер методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ГлобалИСелЕмиттер.цпп 2100
- В1023 [ЦВЕ-460] Показивач без власника се додаје у контејнер 'Матцхерс' методом 'емплаце_бацк'. Доћи ће до цурења меморије у случају изузетка. ГлобалИСелЕмиттер.цпп 2702
Закључак
Издао сам укупно 60 опомена и онда престао. Да ли постоје други недостаци које ПВС-Студио анализатор открива у ЛЛВМ-у? Да имам. Међутим, када сам исписивао фрагменте кода за чланак, било је касно вече, тачније чак ноћ, и одлучио сам да је време да то назовем даном.
Надам се да вам је било занимљиво и да ћете желети да испробате ПВС-Студио анализатор.
Можете преузети анализатор и добити кључ миноловца на .
Најважније, редовно користите статичку анализу. Једнократне провере, које спроводимо у циљу популаризације методологије статичке анализе и ПВС-Студио нису нормалан сценарио.
Срећно у побољшању квалитета и поузданости вашег кода!
Ако желите да поделите овај чланак са публиком која говори енглески, користите линк за превод: Андреј Карпов. .
Извор: ввв.хабр.цом