
Uplynuli viac ako dva roky od poslednej kontroly zdrojového kódu projektu LLVM pomocou nášho analyzátora PVS-Studio. Poďme si overiť, či je analyzátor PVS-Studio stále vedúcim nástrojom na odhaľovanie chýb a potenciálnych zraniteľností. Na to skontrolujeme a nájdeme nové chyby v vydaní LLVM 8.0.0.
Článok, ktorý treba napísať
Musím sa priznať, že som nemal chuť písať tento článok. Nie je to zaujímavé písať o projekte, ktorý sme už viackrát kontrolovali (, , ). Radšej by som napísal o niečom novom, ale nemám na výber.
Prvýkrát, keď vyjde nová verzia LLVM alebo sa aktualizuje , dostávame do našej pošty otázky nasledujúceho typu:
Pozrite, nová verzia Clang Static Analyzer sa naučila nachádzať nové chyby! Mám pocit, že relevantnosť používania PVS-Studio sa zmenšuje. Clang nachádza viac chýb než predtým a dobieha v možnostiach PVS-Studio. Čo si o tom myslíte?
Na to vždy chcem odpovedať niečo v zmysle:
Ani my nesedíme so založenými rukami! Výrazne sme vylepšili možnosti analyzátora PVS-Studio. Takže sa nebojte, stále vedieme, ako aj predtým.
Žiaľ, to nie je dobrá odpoveď. Neobsahuje dôkazy. A práve preto teraz píšem tento článok. Takže projekt LLVM bol opäť skontrolovaný a boli nájdené rôzne chyby. Tie, ktoré mi prišli zaujímavé, vám teraz predstavím. Tieto chyby nemôže nájsť Clang Static Analyzer (alebo je to veľmi nepraktické robiť s jeho pomocou). A my ich dokážeme nájsť. Navyše, všetky tieto chyby som našiel a zapísal za jeden večer.
Písanie článku sa však naťahovalo na niekoľko týždňov. Nemohol som sa donútiť všetko to dať do textovej podoby :).
Mimochodom, ak máte záujem o to, aké technológie sa používajú v analyzátore PVS-Studio na odhaľovanie chýb a potenciálnych zraniteľností, odporúčam oboznámiť sa s touto .
Nové a staré diagnostiky
Ako už bolo spomenuté, pred približne dvoma rokmi bol projekt LLVM opäť skontrolovaný a nájdené chyby boli opravené. Teraz bude v tomto článku predstavená nová porcia chýb. Prečo boli nájdené nové chyby? Na to sú tri dôvody:
- Projekt LLVM sa vyvíja, starý kód sa mení a objavuje sa nový. Je samozrejmé, že v zmienenom a novo napísanom kóde sú nové chyby. To dobre ukazuje, že statická analýza by sa mala vykonávať pravidelne, nie len občas. Naše články dobre ukazujú možnosti analyzátora PVS-Studio, ale nemá to nič spoločné so zlepšením kvality kódu a znížením nákladov na opravu chýb. Používajte statický analyzátor kódu pravidelne!
- Vylepšujeme a zdokonaľujeme už existujúce diagnostiky. Preto môže analyzátor identifikovať chyby, ktoré sa predtým nevšimli počas predchádzajúcich kontrol.
- V PVS-Studio sa objavili nové diagnostiky, ktoré neexistovali pred 2 rokmi. Rozhodol som sa ich vyčleniť do samostatnej sekcie, aby som názorne ukázal vývoj PVS-Studio.
Defekty zistené diagnostikami, ktoré existovali pred 2 rokmi
Fragment N1: Copy-Paste
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
....
}Upozornenie PVS-Studio: [CWE-570] Existujú identické podvýrazy ‘Name.startswith(«avx512.mask.permvar.»)’ naľavo a napravo od operátora ‘||’. AutoUpgrade.cpp 73
Je dvakrát overené, že názov začína podreťazcom «avx512.mask.permvar.». Pri druhom overení chceli jasne napísať niečo iné, ale zabudli opraviť skopírovaný text.
Fragment N2: Preklep
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;
....
}Upozornenie PVS-Studio: V501 Existujú identické podvýrazy ‘CXNameRange_WantQualifier’ naľavo a napravo od operátora ‘|’. CIndex.cpp 7245
V dôsledku preklepu sa dvakrát používa tá istá pomenovaná konštanta CXNameRange_WantQualifier.
Fragment N3: Záměna priorít operátorov
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Upozornenie PVS-Studio: [CWE-783] Možno, že operátor ‘?:’ funguje iným spôsobom, než sa očakávalo. Operátor ‘?:’ má nižšiu prioritu ako operátor ‘==’. PPCTargetTransformInfo.cpp 404
Podľa môjho názoru je to veľmi pekná chyba. Áno, viem, že mám zvláštne názory na krásu :)
Teraz, podľa , sa výraz vypočíta nasledujúcim spôsobom:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Z praktického hľadiska takáto podmienka nedáva zmysel, pretože ju je možné zjednodušiť na:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Je to jasná chyba. Pravdepodobne sa 0/1 chcelo porovnať s premennou Index. Aby ste opravili kód, je potrebné pridať zátvorky okolo ternárneho operátora:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Mimochodom, ternárny operátor je veľmi nebezpečný a vyvoláva logické chyby. Buďte s ním veľmi opatrní a nebuďte zlí s používaním zátvoriek. Podrobne som túto tému preskúmal , v kapitole „Bojte sa operátora ?: a uzatvárajte ho do zátvoriek“.
Fragment N4, N5: Nullový ukazovateľ
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast(Result);
....
LHS = dyn_cast(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() +
"' to string");
return nullptr;
}
....
}Upozornenie PVS-Studio: [CWE-476] Dekrementarizácia nullového ukazovateľa ‘LHS’ môže nastať. TGParser.cpp 2152
Ak ukazovateľ LHS bude nulový, maloby byť vydané varovanie. Namiesto toho ale dôjde k de-referencovaniu tohto nulového ukazovateľa: LHS->getAsString().
Je to veľmi typická situácia, keď sa chyba skrýva v spracovateľských chybách, pretože ich nikto netestuje. Statické analyzátory kontrolujú celý dosiahnuteľný kód, nezávisle od toho, ako často sa používa. Toto je veľmi dobrý príklad toho, ako statická analýza dopĺňa iné metódy testovania a ochrany proti chybám.
Podobná chyba spracovania ukazovateľa RHS sa vyskytuje v kóde o niečo nižšie: V522 [CWE-476] Dekrementarizácia nullového ukazovateľa ‘RHS’ môže nastať. TGParser.cpp 2186
Fragment N6: Použitie ukazovateľa po presune
static Expected
ExtractBlocks(....)
{
....
std::unique_ptr ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Function not found??");
MiscompiledFunctions.push_back(NewF);
}
....
}Varovanie PVS-Studio: V522 [CWE-476] Dekrementarizácia nullového ukazovateľa ‘ProgClone’ môže nastať. Miscompilation.cpp 601
Na začiatku inteligentný ukazovateľ ProgClone prestáva vlastniť objekt:
BD.setNewProgram(std::move(ProgClone));V skutočnosti, teraz ProgClone — je nulový ukazovateľ. Preto by sa nižšie malo stať dereferencovanie nulového ukazovateľa:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Ale v skutočnosti k tomu nedôjde! Všimnite si, že cyklus sa skutočne nevykonáva.
Na začiatku kontajnera MiscompiledFunctions sa vyprázdňuje:
MiscompiledFunctions.clear();Potom sa veľkosť tohto kontajnera použije v podmienke cyklu:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Je ľahké vidieť, že cyklus sa nespustí. Myslím, že to je tiež chyba a kód by mal byť napísaný inak.
Zdá sa, že sme narazili na tú známu paritu chýb! Jedna chyba maskuje druhú : ).
Fragment N7: Použitie ukazovateľa po presune
static Expected TestOptimizer(BugDriver &BD, std::unique_ptr Test,
std::unique_ptr Safe) {
outs() << " Optimalizácia funkcií, ktoré sa testujú: ";
std::unique_ptr Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Chyba pri vykonávaní tejto sekvencie prechodov"
<< " na vstupnom programe!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}Upozornenie PVS-Studio: V522 [CWE-476] Môže dôjsť k dereferencovaniu nulového ukazovateľa ‘Test’. Miscompilation.cpp 709
Opäť rovnaká situácia. Na začiatku sa obsah objektu presúva a potom sa používa, akoby sa nič nestalo. Stále častejšie sa stretávam s touto situáciou v kóde programov, po implementácii semantiky presunu v C++. Za to milujem jazyk C++! Objavujú sa stále nové spôsoby, ako si streliť do nohy. Analyzátor PVS-Studio bude mať vždy prácu : ).
Fragment N8: Nulový ukazovateľ
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "";
else
Type->dump(*this);
}Upozornenie PVS-Studio: V522 [CWE-476] Môže dôjsť k dereferencovaniu nulového ukazovateľa ‘Type’. PrettyFunctionDumper.cpp 233
Okrem spracovateľov chýb sa zvyčajne netestujú ani funkcie na ladenie údajov. Pred nami je presne takýto prípad. Funkcia čaká na používateľa, ktorý namiesto toho, aby vyriešil svoje problémy, bude nútený sa zaoberať jej opravou.
Správne:
if (Type)
Type->dump(*this);
else
Printer << "";Fragment N9: Nulový ukazovateľ
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // getAsString() + " vs. " + // getType()->getAsString());
....
}Upozornenie PVS-Studio: V522 [CWE-476] Môže dôjsť k dereferencovaniu prázdneho ukazovateľa 'Ty'. SearchableTableEmitter.cpp 614
Myslím, že je to jasné a nevyžaduje to ďalšie vysvetlenie.
Fragment N10: Preklep
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;
}Upozornenie PVS-Studio: Premenná 'Identifier->Type' je priradená sama sebe. FormatTokenLexer.cpp 249
Nema zmysel priraďovať premennú samu sebe. Pravdepodobne sa myslelo:
Identifier->Type = Question->Type;Fragment N11: Podozrivý break
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Upozornenie PVS-Studio: [CWE-478] Zvážte kontrolu 'switch' výrazu. Je možné, že chýba prvý 'case' operátor. SystemZAsmParser.cpp 652
Na začiatku je veľmi podozrivý operátor break. Nezabudli ste sem napísať niečo ďalšie?
Fragment N12: Kontrola ukazovateľa po dereferencovaní
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");
....
}Upozornenie PVS-Studio: [CWE-476] Ukazovateľ 'Callee' bol použitý predtým, ako bol overený proti nullptr. Skontrolujte riadky: 172, 174. AMDGPUInline.cpp 172
Ukazovateľ Callee na začiatku sa dereferencuje v čase volania funkcie getTTI.
A potom sa ukazuje, že tento ukazovateľ by sa mal kontrolovať na rovnosť nullptr:
if (!Callee || Callee->isDeclaration())Ale už je neskoro...
Fragment N13 — N…: Kontrola ukazovateľa po dereferencovaní
Situácia uvedená v predchádzajúcom fragmente kódu nie je unikátna. Vyskytuje sa tu:
static Value *optimizeDoubleFP(CallInst *CI, IRBuilder &B,
bool isBinary, bool isPrecise = false) {
....
Function *CalleeFn = CI->getCalledFunction();
StringRef CalleeNm = CalleeFn->getName(); // getAttributes();
if (CalleeFn && !CalleeFn->isIntrinsic()) { // <=
....
}Upozornenie PVS-Studio: V595 [CWE-476] Ukazovateľ 'CalleeFn' bol použitý predtým, ako bol overený proti nullptr. Skontrolujte riadky: 1079, 1081. SimplifyLibCalls.cpp 1079
A tu:
void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
const Decl *Tmpl, Decl *New,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *OuterMostScope) {
....
NamedDecl *ND = dyn_cast(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null(ND->getDeclContext()); // isCXXInstanceMember()); // <=
....
}Upozornenie PVS-Studio: V595 [CWE-476] Ukazovateľ 'ND' bol použitý pred jeho overením voči nullptr. Skontrolujte riadky: 532, 534. SemaTemplateInstantiateDecl.cpp 532
A tu:
- V595 [CWE-476] Ukazovateľ 'U' bol použitý pred jeho overením voči nullptr. Skontrolujte riadky: 404, 407. DWARFFormValue.cpp 404
- V595 [CWE-476] Ukazovateľ 'ND' bol použitý pred jeho overením voči nullptr. Skontrolujte riadky: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Nuž a potom ma prestalo zaujímať štúdium varovaní s číslom V595. Takže neviem, či existujú nejaké podobné chyby, okrem tých, ktoré sú uvedené tu. Pravdepodobne áno.
Fragment N17, N18: Podozrivý posun
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Upozornenie PVS-Studio: [CWE-190] Zvážte preskúmanie výrazu '~(Size — 1) << 1'. Bitový posun 32-bitovej hodnoty so následným rozšírením na 64-bitový typ. AArch64AddressingModes.h 260
Možno to nie je chyba a kód funguje presne tak, ako bolo zamýšľané. Ale je to jasne veľmi podozrivá časť a je potrebné ju skontrolovať.
Predpokladajme, že premennej Veľkosť sa priradil hodnotu 16, a potom autor kódu plánoval získať do premennej NImms hodnotu:
1111111111111111111111111111111111111111111111111111111111100000
Avšak v skutočnosti dostaneme hodnotu:
0000000000000000000000000000000011111111111111111111111111100000
Ide o to, že všetky výpočty prebiehajú s použitím 32-bitového typu unsigned. A až potom bude tento 32-bitový bezsúvaričný typ implicitne rozšírený na uint64_t. Pri tom budú vyššie bity nulové.
Situáciu je možné opraviť takto:
uint64_t NImms = ~static_cast(Size-1) << 1;Podobná situácia: V629 [CWE-190] Zvážte preskúmanie výrazu 'Immr << 6'. Bitový posun 32-bitovej hodnoty so následným rozšírením na 64-bitový typ. AArch64AddressingModes.h 269
Fragment N19: Chýba kľúčové slovo else?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp použitie "vcc" token.
// Preskočiť to.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Spracovať voliteľné argumenty
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Neplatný typ operandu");
}
....
}Upozornenie PVS-Studio: [CWE-670] Zvážte preskúmanie logiky aplikácie. Je možné, že chýba kľúčové slovo 'else'. AMDGPUAsmParser.cpp 5655
Chyby tu nie sú. Pretože then-blok prvého ak končí na continue, takže nezáleží na tom, či existuje kľúčové slovo else alebo nie. V každom prípade bude kód fungovať rovnakým spôsobom. Avšak vynechanie else robiť kód menej zrozumiteľným a nebezpečným. Ak v budúcnosti continue zmizne, kód začne fungovať úplne inak. Podľa môjho názoru je lepšie pridať else.
Fragment N20: Štyri rovnaké preklepy
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;
}Upozornenia PVS-Studio:
- V655 [CWE-480] Ťažko sa určuje, že reťazce boli konkatenované, ale nie sú použité. Zvážte preskúmanie výrazu ‘Result + Name.str()’. Symbol.cpp 32
- V655 [CWE-480] Ťažko sa určuje, že reťazce boli konkatenované, ale nie sú použité. Zvážte preskúmanie výrazu ‘Result + „(ObjC Class) „ + Name.str()’. Symbol.cpp 35
- V655 [CWE-480] Ťažko sa určuje, že reťazce boli konkatenované, ale nie sú použité. Zvážte preskúmanie výrazu ‘Result + „(ObjC Class EH) „ + Name.str()’. Symbol.cpp 38
- V655 [CWE-480] Ťažko sa určuje, že reťazce boli konkatenované, ale nie sú použité. Zvážte preskúmanie výrazu ‘Result + „(ObjC IVar) „ + Name.str()’. Symbol.cpp 41
Namiesto operátora += sa náhodne používa operátor +. V dôsledku toho vznikajú konštrukcie, ktoré nedávajú zmysel.
Fragment N21: Neurčité správanie
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 nemôže byť prázdny");
for (auto &Op : Ops) {
assert(!Op.empty() && "Prázdny operátor");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Skúste sami nájsť nebezpečný kód. A toto je obrázok pre rozptýlenie, aby ste sa hneď pozerali na odpoveď:

Upozornenie PVS-Studio: [CWE-758] Používa sa nebezpečná konštrukcia: ‘FeaturesMap[Op] = FeaturesMap.size()’, kde ‘FeaturesMap’ je trieda ‘map’. To môže viesť k neurčitému správaniu. RISCVCompressInstEmitter.cpp 490
Problémový riadok:
FeaturesMap[Op] = FeaturesMap.size();Ak prvok Op nie je nájdený, vytvorí sa nový prvok v mape a tam sa zapíše počet prvkov v tejto mape. Lenže nie je známe, či bude volaná funkcia size pred alebo po pridaní nového prvku.
Fragment N22-N24: Opakované priradenia
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;
}
....
}Upozornenie PVS-Studio: [CWE-563] Premenná ‘NType’ je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 1663, 1664. MachOObjectFile.cpp 1664
Myslím, že skutočná chyba tu nie je. Ide len o zbytočné opakované priradenie. Ale aj tak je to prehliadnutie.
Podobne:
- V519 [CWE-563] Premenná ‘B.NDesc’ je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Premenná je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Ešte opakované priradenia
Teraz zoberieme trochu iný variant opakovaného priradenia.
bool Vectorizer::vectorizeLoadChain(
ArrayRef Chain,
SmallPtrSet *InstructionsProcessed) {
....
unsigned Alignment = getAlignment(L0);
....
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
StackAdjustedAlignment,
DL, L0, nullptr, &DT);
if (NewAlign != 0)
Alignment = NewAlign;
Alignment = NewAlign;
....
}Upozornenie PVS-Studio: V519 [CWE-563] Premenná ‘Alignment’ je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 1158, 1160. LoadStoreVectorizer.cpp 1160
Toto je veľmi zvláštny kód, ktorý zjavne obsahuje logickú chybu. Na začiatku je premennej Alignment priradená hodnota v závislosti od podmienky. A potom sa opäť vykoná priradenie, ale tentokrát bez akejkoľvek kontroly.
Podobné situácie je možné vidieť tu:
- V519 [CWE-563] Premenná ‘Effects’ je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Premenná ‘ExpectNoDerefChunk’ je priradená hodnoty dvakrát po sebe. Možno je to chyba. Skontrolujte riadky: 4970, 4973. SemaType.cpp 4973
Fragment N28: Vždy pravdivá podmienka
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;
}
....
}Upozornenie PVS-Studio: [CWE-571] Výraz ‘nextByte != 0x90’ je vždy pravdivý. X86DisassemblerDecoder.cpp 379
Kontrola nemá zmysel. Premenná nextByte je vždy nerovná hodnote 0x90, čo vyplýva z predchádzajúcej kontroly. To je nejaká logická chyba.
Fragment N29 — N…: Vždy pravdivé/nepravdivé podmienky
Analizátor vydáva veľa upozornení na to, že celá podmienka () alebo jej časť () je vždy pravdivé alebo nepravdivé. Často to nie sú skutočné chyby, ale len nepresný kód, výsledok nasadenia makier a podobne. Napriek tomu má zmysel pozrieť sa na všetky tieto výstrahy, pretože sa občas vyskytujú skutočné logické chyby. Napríklad tento kódový úsek je podozrivý:
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;
....
}Upozornenie PVS-Studio: [CWE-570] Časť podmienkového výrazu je vždy nepravdivá: RegNo == 0xe. ARMDisassembler.cpp 939
Konštanta 0xE je v desiatkovej sústave hodnota 14. Kontrola RegNo == 0xe nemá zmysel, pretože ak RegNo > 13, funkcia sa ukončí.
Bolo množstvo ďalších varovaní s identifikátorom V547 a V560, ale, ako v prípade s , skúmať tieto varovania ma nebavilo. Aj tak bolo jasné, že materiálu na napísanie článku mám dosť :). Preto nie je známe, koľko chýb tohto typu možno odhaliť v LLVM pomocou PVS-Studio.
Uvediem príklad, prečo skúmanie týchto upozornení je nudné. Analýzator má úplne pravdu, keď vyháňa výstrahu na nasledujúci kód. Ale to nie je chyba.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Upozornenie PVS-Studio: V547 [CWE-570] Výraz ‘!HasError’ je vždy nepravdivý. UnwrappedLineParser.cpp 1635
Fragment N30: Podozrivý return
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();
}
....
}Upozornenie PVS-Studio: [CWE-670] Nepodmienečné ‘return’ v slučke. R600OptimizeVectorRegisters.cpp 63
To je buď chyba, alebo špecifický trik, ktorý má niečo objasniť programátorom čítajúcim kód. Takáto konštrukcia mi nič neobjasňuje a vyzerá veľmi podozrivo. Lepšie to takto nepísať :).
Unavení? Potom je čas uvariť si čaj alebo kávu.

Defekty odhalené novými diagnostikami
Myslím, že 30 zranení starých diagnostík stačí. Poďme sa teraz pozrieť, čo zaujímavého môžeme nájsť novými diagnostikami, ktoré sa objavili v analyzátore už po kontrole. Celkovo sa za toto obdobie do C++ analyzátora pridalo 66 diagnostík všeobecného určenia.
Fragment N31: Nedosiahnuteľný kód
Chyba CtorDtorRunner::run() {
....
ak (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}Upozornenie PVS-Studio: [CWE-561] Zistený nedosiahnuteľný kód. Môže sa tu nachádzať chyba. ExecutionUtils.cpp 146
Ako vidíte, obe vetvy operátora ak sa končia volaním operátora return. Zodpovedajúco, kontajner CtorDtorsByPriority nikdy nebude vymazaný.
Fragment N32: Nedosiahnuteľný kód
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(), "neočakávaný typ súhrnu");
}
Lex.setIgnoreColonInIdentifiers(false);
return false;
}Upozornenie PVS-Studio: V779 [CWE-561] Zistený nedosiahnuteľný kód. Môže sa tu nachádzať chyba. LLParser.cpp 835
Zaujímavá situácia. Poďme sa najprv pozrieť na toto miesto:
return ParseTypeIdEntry(SummaryID);
break;Na prvý pohľad sa zdá, že tu nie je chyba. Zdá sa, že operátor break je tu zbytočný a dá sa jednoducho odstrániť. Avšak, nie je to také jednoduché.
Analyzátor vydáva varovanie na riadky:
Lex.setIgnoreColonInIdentifiers(false);
return false;A naozaj, tento kód je nedosiahnuteľný. Všetky prípady v switch sa končia volaním operátora return. A teraz bezvýznamná osamelá break nevyzerá tak bezškodná! Možno by jedna z vetiev mala končiť na break, a nie na return?
Fragment N33: Náhodné nulovanie vyšších bitov
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Očakávané<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}Upozornenie PVS-Studio: Veľkosť bitovej masky je menšia než veľkosť prvého operandu. To spôsobí stratu vyšších bitov. RuntimeDyld.cpp 815
Všimnite si, že funkcia getStubAlignment vracia typ unsigned. Vypočítajme hodnotu výrazu, ak predpokladáme, že funkcia vráti hodnotu 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFF8u
Teraz si všimnite, že premenná DataSize má 64-bitový bezsídlený typ. Získava sa, že pri vykonaní operácie DataSize & 0xFFFFFFF8u budú všetky tridsať dva vyšších bitov nulované. Najpravdepodobnejšie to nebolo to, čo programátor chcel. Podozrievam, že chcel vypočítať: DataSize & 0xFFFFFFFFFFFFFFF8u.
Aby ste opravili chybu, mali by ste napísať takto:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Alebo takto:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Neúspešné explicitné pretypovanie
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Neočakávaný faktor škálovania");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}Upozornenie PVS-Studio: [CWE-190] Možný pretečenie. Zvážte pretypovanie operandov operátora 'NumElts * Scale' na typ 'size_t', nie výsledok. X86ISelLowering.h 1577
Explicitné pretypovanie sa používa na zabránenie pretečeniu pri násobení premenných typu int. Avšak, tu explicitné pretypovanie neochráni pred pretečením. Najprv budú premenné vynásobené a až potom 32-bitový výsledok násobenia bude rozšírený na typ .
Fragment N35: Neúspešné skopírovanie-vloženie
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;
}
....
}[CWE-682] Našli sa dva podobné fragmenty kódu. Možno ide o preklep a namiesto 'Op0' by sa mala použiť premenná 'Op1'. InstCombineCompares.cpp 5507
Tento nový zaujímavý diagnostikujúci nástroj odhaľuje situácie, keď bol fragment kódu skopírovaný a začali sa meniť niektoré názvy, ale na jednom mieste nebol upravený.
Všimnite si, že v druhom bloku sa menil Op0 na Op1. Ale na jednom mieste nebol upravený. Skôr by sa malo napísať takto:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Zmätok s premennými
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Upozornenie PVS-Studio: [CWE-563] Premenná 'Mode' je priradená, ale na konci funkcie sa nepoužíva. SIModeRegister.cpp 48
Je veľmi nebezpečné dávať argumentom funkcií tie isté názvy ako členom triedy. Veľmi rýchlo sa dá zmiasť. Toto je presne taký prípad. Tento výraz nedáva zmysel:
Mode &= Mask;Mení sa argument funkcie. A to je všetko. Tento argument sa už viac nepoužíva. Skôr by sa malo napísať takto:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Zmätok s premennými
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(Sym));
Size += this->EntrySize;
}Upozornenie PVS-Studio: V1001 [CWE-563] Premenná 'Size' bola priradená, ale na konci funkcie nebola použitá. Object.cpp 424
Situácia je rovnaká ako predchádzajúca. Malo by byť napísané:
this->Size += this->EntrySize;Fragment N38-N47: Ukazovateľ sa zabudol skontrolovať
Skôr sme sa zaoberali príkladmi spustenia diagnostiky . Jej podstatou je, že ukazovateľ sa na začiatku dereferencuje, a iba potom kontroluje. Nová diagnostika je významovo opačná, ale tiež odhaľuje veľmi veľa chýb. Odhaluje situácie, keď sa ukazovateľ na začiatku skontroloval, a potom sa na to zabudlo. Pozrime sa na takéto prípady, ktoré boli nájdené v rámci LLVM.
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}Upozornenie PVS-Studio: V1004 [CWE-476] Ukazovateľ 'Ptr' bol použitý nesprávne po jeho overení voči nullptr. Skontrolujte riadky: 729, 738. TargetTransformInfoImpl.h 738
Premenná Ptr môže byť rovné nullptr, čo dokazuje kontrola:
if (Ptr != nullptr)Avšak nižšie sa tento ukazovateľ dereferencuje už bez predchádzajúcej kontroly:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Pozrime sa na ďalší podobný prípad.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast(GD.getDecl());
SmallVector ArgTypes;
if (FD) // parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs()->getCallConv(); // <=
....
}Upozornenie PVS-Studio: V1004 [CWE-476] Ukazovateľ 'FD' bol použitý nesprávne po jeho overení voči nullptr. Skontrolujte riadky: 3228, 3231. CGDebugInfo.cpp 3231
Všimnite si ukazovateľ FD. Som si istý, že problém je dobre viditeľný a nie sú potrebné žiadne špeciálne vysvetlenia.
A ešte:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast(Ptr.getType());
if (!PtrTy) { // getPointerAddressSpace()); // <=
....
}Upozornenie PVS-Studio: V1004 [CWE-476] Ukazateľ 'PtrTy' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 960, 965. InterleavedLoadCombinePass.cpp 965
Ako sa chrániť pred takýmito chybami? Buďte opatrní pri Code-Review a pravidelne používajte statický analyzátor PVS-Studio na kontrolu kódu.
Nemá zmysel uvádzať ďalšie útržky kódu s chybami tohto typu. V článku ponechám iba zoznam upozornení:
- V1004 [CWE-476] Ukazateľ 'Expr' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Ukazateľ 'PI' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Ukazateľ 'StatepointCall' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Ukazateľ 'RV' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Ukazateľ 'CalleeFn' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Ukazateľ 'TC' bol použitý bezpečne po overení na nullptr. Skontrolujte riadky: 1819, 1824. Driver.cpp 1824
Útržok N48-N60: Nie kritické, ale defekt (možná únik pamäte)
std::unique_ptr createISelMutator() {
....
std::vector<std::unique_ptr> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Upozornenie PVS-Studio: [CWE-460] Ukazateľ bez vlastníka bol pridaný do kontajnera 'Strategies' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-isel-fuzzer.cpp 58
Na pridanie prvku na koniec kontajnera typu std::vector<std::unique_ptr> nie je možné len napísať xxx.push_back(new X), pretože neexistuje implicitná konverzia z X* v std::unique_ptr.
Bežným riešením je napísať xxx.emplace_back(new X), pretože sa kompiluje: metóda emplace_back konštruuje prvok priamo z argumentov a preto môže používať explicitné konštruktory.
To nie je bezpečné. Ak je vektor plný, nastane prealokácia pamäte. Operácia prealokácie pamäte môže zlyhať, čo spôsobí vygenerovanie výnimky std::bad_alloc. V takom prípade sa ukazateľ stratí a vytvorený objekt už nikdy nebude odstránený.
Bezpečným riešením je vytvorenie unique_ptr, ktorý bude vlastníkom ukazateľa, kým sa vektor pokúša prealokovať pamäť:
xxx.push_back(std::unique_ptr(new X))Od C++14 možete koristiti 'std::make_unique':
xxx.push_back(std::make_unique())Tento typ chyby nie je pre LLVM kritický. Ak sa nepodarí alokovať pamäť, práca kompilátora sa jednoducho zastaví. Avšak pre aplikácie s dlhým , ktoré nemôžu len tak skončiť, ak sa nepodarilo alokovať pamäť, to môže byť skutočná nepríjemná chyba.
Takže, aj keď tento kód nepredstavuje praktické nebezpečenstvo pre LLVM, považoval som za užitočné povedať o tomto vzore chýb a o tom, že analyzátor PVS-Studio sa naučil ho odhalovať.
Iné varovania tohto typu:
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Passes' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. PassManager.h 546
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'AAs' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. AliasAnalysis.h 324
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Entries' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'AllEdges' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. CFGMST.h 268
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'VMaps' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Records' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. FDRLogBuilder.h 30
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'PendingSubmodules' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. ModuleMap.cpp 810
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Objects' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. DebugMap.cpp 88
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Strategies' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 685
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 686
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 688
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 689
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 690
- V1023 [CWE-460] Ukazateľ bez vlastníka je pridaný do kontajnera 'Modifiers' metódou 'emplace_back'. K úniku pamäte dôjde v prípade výnimky. llvm-stress.cpp 691
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Modifiers’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. llvm-stress.cpp 692
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Modifiers’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. llvm-stress.cpp 693
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Modifiers’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. llvm-stress.cpp 694
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Operands’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Stash’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Pomocník bez vlastníka byl přidán do kontejneru ‘Matchers’ metodou ’emplace_back’. V případě výjimky dojde k úniku paměti. GlobalISelEmitter.cpp 2702
Záver
Celkem jsem zaznamenal 60 varování, po čemž jsem přestal. Existují další chyby, které analyzátor PVS-Studio v LLVM odhaluje? Ano, existují. Nicméně když jsem si připravoval fragmenty kódu pro článek, bylo pozdě večer, vlastně už noc, a rozhodl jsem se, že je čas skončit.
Doufám, že vás to zaujalo a budete chtít vyzkoušet analyzátor PVS-Studio.
Můžete si stáhnout analyzátor a získat zkušební klíč na .
Hlavně používejte statickou analýzu pravidelně. Jednorázové kontroly, které provádíme s cílem popularizace metodologie statické analýzy a PVS-Studio, nejsou normálním scénářem.
Hodně štěstí při zlepšování kvality a spolehlivosti kódu!
Pokud chcete sdílet tento článek s anglicky mluvícím publikem, prosím použijte odkaz na překlad: Andrey Karpov. .
Zdroj: habr.com
