Od poslední kontroly kódu projektu LLVM pomocí našeho analyzátoru PVS-Studio uplynuly více než dva roky. Pojďme se ujistit, že analyzátor PVS-Studio je stále předním nástrojem pro identifikaci chyb a potenciálních zranitelností. Za tímto účelem zkontrolujeme a najdeme nové chyby ve vydání LLVM 8.0.0.
Článek k napsání
Abych byl upřímný, nechtěl jsem tento článek psát. Není zajímavé psát o projektu, který jsme již několikrát kontrolovali (, , ). Je lepší psát o něčem novém, ale nemám na výběr.
Pokaždé, když je vydána nebo aktualizována nová verze LLVM , dostáváme na naši poštu dotazy následujícího typu:
Podívejte, nová verze Clang Static Analyzer se naučila najít nové chyby! Zdá se mi, že relevance používání PVS-Studio klesá. Clang nachází více chyb než dříve a dohání možnosti PVS-Studia. Co si o tom myslíš?
Na to chci vždy odpovědět něco jako:
Ani my nesedíme nečinně! Výrazně jsme vylepšili možnosti analyzátoru PVS-Studio. Takže se nebojte, pokračujeme ve vedení jako doposud.
Bohužel je to špatná odpověď. Nejsou v tom žádné důkazy. A proto nyní píšu tento článek. Projekt LLVM byl tedy znovu zkontrolován a byly v něm nalezeny různé chyby. Nyní předvedu ty, které se mi zdály zajímavé. Clang Static Analyzer nemůže najít tyto chyby (nebo je extrémně nepohodlné to udělat s jeho pomocí). Ale můžeme. Navíc jsem všechny tyto chyby našel a zapsal během jednoho večera.
Ale psaní článku trvalo několik týdnů. Prostě jsem se nemohl přimět to všechno dát do textu :).
Mimochodem, pokud vás zajímá, jaké technologie se používají v analyzátoru PVS-Studio k identifikaci chyb a potenciálních zranitelností, pak doporučuji seznámit se s tímto .
Nová a stará diagnostika
Jak již bylo uvedeno, zhruba před dvěma lety byl projekt LLVM znovu zkontrolován a nalezené chyby byly opraveny. Nyní tento článek představí novou várku chyb. Proč byly nalezeny nové chyby? Jsou pro to 3 důvody:
- Projekt LLVM se vyvíjí, mění starý kód a přidává nový kód. Přirozeně se v upraveném a napsaném kódu objevují nové chyby. To jasně ukazuje, že statická analýza by měla být používána pravidelně a ne příležitostně. Naše články dobře ukazují možnosti analyzátoru PVS-Studio, ale to nemá nic společného se zlepšováním kvality kódu a snižováním nákladů na opravy chyb. Pravidelně používejte analyzátor statického kódu!
- Dokončujeme a vylepšujeme stávající diagnostiku. Analyzátor tak může identifikovat chyby, kterých si během předchozích skenování nevšiml.
- V PVS-Studiu se objevila nová diagnostika, která před 2 lety neexistovala. Rozhodl jsem se je vyzdvihnout v samostatné sekci, abych názorně ukázal vývoj PVS-Studio.
Závady zjištěné diagnostikou, která existovala před 2 lety
Fragment N1: Kopírovat-Vložit
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
....
}Upozornění PVS-Studio: [CWE-570] Nalevo a napravo od '||' jsou identické podvýrazy 'Name.startswith("avx512.mask.permvar.")' operátor. AutoUpgrade.cpp 73
Je dvakrát zkontrolováno, že název začíná podřetězcem "avx512.mask.permvar.". Při druhé kontrole chtěli evidentně napsat něco jiného, ale zapomněli opravit zkopírovaný text.
Fragment N2: Překlep
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;
....
}Warning PVS-Studio: V501 Nalevo a napravo od '|' jsou identické podvýrazy 'CXNameRange_WantQualifier' operátor. CIndex.cpp 7245
Kvůli překlepu je stejná pojmenovaná konstanta použita dvakrát CXNameRange_WantQualifier.
Fragment N3: Záměna s prioritou operátoru
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Upozornění PVS-Studio: [CWE-783] Operátor '?:' možná funguje jiným způsobem, než se očekávalo. Operátor '?:' má nižší prioritu než operátor '=='. PPCTargetTransformInfo.cpp 404
Podle mě je to moc krásný omyl. Ano, vím, že mám divné představy o kráse :).
Nyní podle , výraz je vyhodnocen takto:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Z praktického hlediska taková podmínka nedává smysl, protože ji lze redukovat na:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())To je jasná chyba. S největší pravděpodobností chtěli porovnat 0/1 s proměnnou index. Chcete-li opravit kód, musíte přidat závorky kolem ternárního operátoru:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Mimochodem, ternární operátor je velmi nebezpečný a vyvolává logické chyby. Buďte s ním velmi opatrní a nebuďte lakomí se závorkami. Podíval jsem se na toto téma podrobněji , v kapitole „Pozor na ?: Operátor a uzavřete jej do závorek“.
Fragment N4, N5: Nulový ukazatel
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;
}
....
}Upozornění PVS-Studio: [CWE-476] Může dojít k dereferencování nulového ukazatele 'LHS'. TGParser.cpp 2152
Pokud ukazatel LHS je nulová, mělo by být vydáno varování. Místo toho však bude tento stejný nulový ukazatel dereferencován: LHS->getAsString().
Toto je velmi typická situace, kdy je chyba skryta v obslužném programu chyb, protože je nikdo netestuje. Statické analyzátory kontrolují veškerý dosažitelný kód bez ohledu na to, jak často se používá. Toto je velmi dobrý příklad toho, jak statická analýza doplňuje další techniky testování a ochrany proti chybám.
Podobná chyba při manipulaci s ukazatelem RHS povoleno v kódu níže: V522 [CWE-476] Může dojít k dereferencování nulového ukazatele 'RHS'. TGParser.cpp 2186
Fragment N6: Použití ukazatele po přesunutí
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);
}
....
}Varování PVS-Studio: V522 [CWE-476] Může dojít k dereferencování nulového ukazatele 'ProgClone'. Chybná kompilace.cpp 601
Na začátku chytrý ukazatel ProgClone přestane vlastnit předmět:
BD.setNewProgram(std::move(ProgClone));Vlastně teď ProgClone je nulový ukazatel. Proto by dereference nulového ukazatele měla nastat těsně pod:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Ale ve skutečnosti se to nestane! Všimněte si, že smyčka není ve skutečnosti provedena.
Na začátku kontejneru MiscompiledFunctions vymazáno:
MiscompiledFunctions.clear();Dále se velikost tohoto kontejneru použije v podmínce smyčky:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Je snadné vidět, že smyčka nezačíná. Myslím, že to je také chyba a kód by měl být napsán jinak.
Zdá se, že jsme narazili na pověstnou paritu chyb! Jedna chyba maskuje druhou :).
Fragment N7: Použití ukazatele po přesunutí
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;
}
....
}Varování PVS-Studio: V522 [CWE-476] Může dojít k dereferencování nulového ukazatele 'Test'. Chybná kompilace.cpp 709
Opět stejná situace. Nejprve se obsah objektu přesune a pak se používá, jako by se nic nestalo. Tuto situaci vidím stále častěji v programovém kódu poté, co se v C++ objevila sémantika pohybu. To je důvod, proč miluji jazyk C++! Existuje stále více nových způsobů, jak si ustřelit vlastní nohu. Analyzátor PVS-Studio bude vždy fungovat :).
Fragment N8: Nulový ukazatel
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);
}Varování PVS-Studio: V522 [CWE-476] Může dojít k dereferencování nulového ukazatele 'Typ'. PrettyFunctionDumper.cpp 233
Kromě obslužných rutin chyb se obvykle netestují funkce tisku ladění. Právě takový případ máme před sebou. Funkce čeká na uživatele, který místo řešení svých problémů bude nucen ji opravit.
Správně:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Nulový ukazatel
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());
....
}Varování PVS-Studio: V522 [CWE-476] Může dojít k dereferencování nulového ukazatele 'Ty'. SearchableTableEmitter.cpp 614
Myslím, že vše je jasné a nevyžaduje vysvětlení.
Fragment N10: Překlep
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;
}Upozornění PVS-Studio: Proměnná 'Identifier->Type' je přiřazena sama sobě. FormatTokenLexer.cpp 249
Nemá smysl přiřazovat proměnnou sama sobě. S největší pravděpodobností chtěli napsat:
Identifier->Type = Question->Type;Fragment N11: Podezřelý zlom
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Upozornění PVS-Studio: [CWE-478] Zvažte kontrolu prohlášení o „přepnutí“. Je možné, že chybí první operátor „případu“. SystemZAsmParser.cpp 652
Na začátku je velmi podezřelý operátor rozbít. Nezapomněl jsi sem napsat ještě něco?
Fragment N12: Kontrola ukazatele po dereferencování
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");
....
}Upozornění PVS-Studio: [CWE-476] Ukazatel 'Callee' byl použit předtím, než byl ověřen proti nullptr. Kontrolní řádky: 172, 174. AMDGPUInline.cpp 172
Ukazatel Callee na začátku je dereference v době volání funkce getTTI.
A pak se ukáže, že tento ukazatel by měl být zkontrolován na rovnost nullptr:
if (!Callee || Callee->isDeclaration())Ale už je pozdě…
Fragment N13 - N...: Kontrola ukazatele po dereferencování
Situace popsaná v předchozím fragmentu kódu není jedinečná. Objevuje se zde:
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()) { // <=
....
}Upozornění PVS-Studio: V595 [CWE-476] Ukazatel 'CalleeFn' byl použit předtím, než byl ověřen proti nullptr. Kontrolní řádky: 1079, 1081. SimplifyLibCalls.cpp 1079
A zde:
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()); // <=
....
}Varování PVS-Studio: V595 [CWE-476] Ukazatel 'ND' byl použit předtím, než byl ověřen proti nullptr. Kontrolní řádky: 532, 534. SemaTemplateInstantiateDecl.cpp 532
A zde:
- V595 [CWE-476] Ukazatel 'U' byl použit předtím, než byl ověřen proti nullptr. Kontrolní řádky: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Ukazatel 'ND' byl použit před ověřením proti nullptr. Kontrolní řádky: 2149, 2151. SemaTemplateInstantiate.cpp 2149
A pak jsem přestal mít zájem studovat varování s číslem V595. Tak nevím, jestli podobných chyb kromě zde uvedených je víc. S největší pravděpodobností existuje.
Fragment N17, N18: Podezřelý posun
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Upozornění PVS-Studio: [CWE-190] Zvažte kontrolu výrazu '~(Velikost - 1) << 1'. Bitový posun 32bitové hodnoty s následným rozšířením na 64bitový typ. AArch64AddressingModes.h 260
Nemusí to být chyba a kód funguje přesně tak, jak bylo zamýšleno. Ale toto je zjevně velmi podezřelé místo a je třeba jej zkontrolovat.
Řekněme proměnnou Velikost se rovná 16 a autor kódu pak plánoval, že jej dostane do proměnné NImms hodnota:
1111111111111111111111111111111111111111111111111111111111100000
Ve skutečnosti však bude výsledek:
0000000000000000000000000000000011111111111111111111111111100000
Faktem je, že všechny výpočty probíhají pomocí 32bitového typu bez znaménka. A teprve potom bude tento 32bitový nepodepsaný typ implicitně rozšířen na uint64_t. V tomto případě budou nejvýznamnější bity nula.
Situaci můžete napravit takto:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Podobná situace: V629 [CWE-190] Zvažte kontrolu výrazu 'Immr << 6'. Bitový posun 32bitové hodnoty s následným rozšířením na 64bitový typ. AArch64AddressingModes.h 269
Fragment N19: Chybí klíčové slovo jiný?
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");
}
....
}Upozornění PVS-Studio: [CWE-670] Zvažte kontrolu logiky aplikace. Je možné, že klíčové slovo „jinak“ chybí. AMDGPUAsmParser.cpp 5655
Tady to nemá chybu. Od tehdejšího bloku prvního if končí s pokračovat, pak je to jedno, je tam klíčové slovo jiný nebo ne. V každém případě bude kód fungovat stejně. Stále chybí jiný kód je nejasnější a nebezpečnější. Pokud v budoucnu pokračovat zmizí, kód začne fungovat úplně jinak. Podle mého názoru je lepší přidat jiný.
Fragment N20: Čtyři překlepy stejného typu
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;
}Upozornění PVS-Studio:
- V655 [CWE-480] Řetězce byly zřetězeny, ale nejsou použity. Zvažte kontrolu výrazu 'Result + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] Řetězce byly zřetězeny, ale nejsou použity. Zvažte kontrolu výrazu 'Result + "(ObjC Class)" + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Řetězce byly zřetězeny, ale nejsou použity. Zvažte kontrolu výrazu 'Result + "(ObjC Class EH) " + Name.str()'. Symbol.cpp 38
- V655 [CWE-480] Řetězce byly zřetězeny, ale nejsou použity. Zvažte kontrolu výrazu 'Result + "(ObjC IVar)" + Name.str()'. Symbol.cpp 41
Náhodou se místo operátoru += používá operátor +. Výsledkem jsou návrhy, které postrádají smysl.
Fragment N21: Nedefinované chování
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();
}
}
}Pokuste se najít nebezpečný kód sami. A toto je obrázek k rozptýlení pozornosti, abyste se hned nedívali na odpověď:
Upozornění PVS-Studio: [CWE-758] Je použita nebezpečná konstrukce: 'FeaturesMap[Op] = FeaturesMap.size()', kde 'FeaturesMap' je třídy 'map'. To může vést k nedefinovanému chování. RISCVCompressInstEmitter.cpp 490
Problémový řádek:
FeaturesMap[Op] = FeaturesMap.size();Pokud prvek Op není nalezen, pak se v mapě vytvoří nový prvek a zapíše se tam počet prvků v této mapě. Jen není známo, zda bude funkce volána velikost před nebo po přidání nového prvku.
Fragment N22-N24: Opakovaná zadání
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;
}
....
}Upozornění PVS-Studio: [CWE-563] Proměnné 'NType' jsou přiřazeny hodnoty dvakrát za sebou. Možná je to chyba. Kontrolní řádky: 1663, 1664. MachOObjectFile.cpp 1664
Nemyslím si, že by zde došlo k nějaké skutečné chybě. Jen zbytečné opakované zadání. Ale pořád omyl.
Podobně:
- V519 [CWE-563] Proměnné 'B.NDesc' jsou přiřazeny hodnoty dvakrát za sebou. Možná je to chyba. Kontrolní řádky: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Proměnné jsou přiřazeny hodnoty dvakrát po sobě. Možná je to chyba. Kontrolní řádky: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Další přeřazení
Nyní se podíváme na trochu jinou verzi přeřazení.
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;
....
}Varování PVS-Studio: V519 [CWE-563] Proměnné 'Alignment' jsou přiřazeny hodnoty dvakrát za sebou. Možná je to chyba. Kontrolní řádky: 1158, 1160. LoadStoreVectorizer.cpp 1160
Toto je velmi zvláštní kód, který zjevně obsahuje logickou chybu. Na začátku variabilní SMĚR hodnota je přiřazena v závislosti na stavu. A pak k zadání dojde znovu, ale nyní bez jakékoli kontroly.
Podobné situace lze vidět zde:
- V519 [CWE-563] Proměnné 'Efekty' jsou přiřazeny hodnoty dvakrát za sebou. Možná je to chyba. Kontrolní řádky: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Proměnné 'ExpectNoDerefChunk' jsou přiřazeny hodnoty dvakrát za sebou. Možná je to chyba. Kontrolní řádky: 4970, 4973. SemaType.cpp 4973
Fragment N28: Vždy pravdivý stav
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;
}
....
}Upozornění PVS-Studio: [CWE-571] Výraz 'nextByte != 0x90' je vždy pravdivý. X86DisassemblerDecoder.cpp 379
Kontrola nemá smysl. Variabilní dalšíByte vždy se nerovná hodnotě 0x90, což vyplývá z předchozí kontroly. To je nějaká logická chyba.
Fragment N29 - N...: Vždy pravdivé/nepravdivé podmínky
Analyzátor vydává mnoho varování, že celý stav () nebo jeho část () je vždy pravda nebo nepravda. Často se nejedná o skutečné chyby, ale prostě o odfláknutý kód, výsledek rozšiřování maker a podobně. Nicméně má smysl se na všechna tato varování podívat, protože čas od času se vyskytnou skutečné logické chyby. Například tato část kódu je podezřelá:
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;
....
}Upozornění PVS-Studio: [CWE-570] Část podmíněného výrazu je vždy nepravdivá: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanta 0xE je hodnota 14 v desítkové soustavě. Zkouška RegNo == 0xe nedává smysl, protože kdyby RegNo > 13, pak funkce dokončí své provádění.
S ID V547 a V560 bylo mnoho dalších varování, ale jako u , neměl jsem zájem tato varování studovat. To už bylo jasné, že mám dost materiálu na napsání článku :). Proto není známo, kolik chyb tohoto typu lze identifikovat v LLVM pomocí PVS-Studio.
Dám vám příklad, proč je studium těchto spouštěčů nudné. Analyzátor má naprostou pravdu, když vydává varování pro následující kód. Ale to není chyba.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Varování PVS-Studio: V547 [CWE-570] Výraz '!HasError' je vždy nepravdivý. UnwrappedLineParser.cpp 1635
Fragment N30: Podezřelý návrat
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();
}
....
}Upozornění PVS-Studio: [CWE-670] Bezpodmínečný 'návrat' v rámci smyčky. R600OptimizeVectorRegisters.cpp 63
Jedná se buď o chybu, nebo o specifickou techniku, která má něco vysvětlit programátorům čtoucím kód. Tento design mi nic nevysvětluje a vypadá velmi podezřele. Takhle je lepší nepsat :).
Unavený? Pak je čas uvařit si čaj nebo kávu.
Závady zjištěné novou diagnostikou
Myslím, že 30 aktivací staré diagnostiky je dost. Pojďme se nyní podívat, co zajímavého lze najít s novou diagnostikou, která se objevila v analyzátoru poté kontroly. Během této doby bylo do analyzátoru C++ přidáno celkem 66 univerzálních diagnostik.
Fragment N31: Nedosažitelný kód
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();
}Upozornění PVS-Studio: [CWE-561] Zjištěn nedostupný kód. Je možné, že došlo k chybě. ExecutionUtils.cpp 146
Jak vidíte, obě pobočky operátora if končí hovorem s operátorem zpáteční. V souladu s tím kontejner CtorDtorsByPriority nebude nikdy vyčištěno.
Fragment N32: Nedosažitelný 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(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}Varování PVS-Studio: V779 [CWE-561] Zjištěn nedostupný kód. Je možné, že došlo k chybě. LLParser.cpp 835
Zajímavá situace. Nejprve se podívejme na toto místo:
return ParseTypeIdEntry(SummaryID);
break;Na první pohled se zdá, že zde není žádná chyba. Vypadá to na operátora rozbít je zde jeden navíc a můžete ho jednoduše smazat. Nicméně, ne všechno tak jednoduché.
Analyzátor vydá varování na řádcích:
Lex.setIgnoreColonInIdentifiers(false);
return false;A skutečně, tento kód je nedosažitelný. Všechny případy v přepnout končí hovorem od operátora zpáteční. A teď nesmyslně sám rozbít nevypadá tak neškodně! Možná by jedna z větví měla skončit s rozbít, ne na zpáteční?
Fragment N33: Náhodný reset vysokých bitů
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);
....
}Upozornění PVS-Studio: Velikost bitové masky je menší než velikost prvního operandu. To způsobí ztrátu vyšších bitů. RuntimeDyld.cpp 815
Vezměte prosím na vědomí, že funkce getStubAlignment vrátí typ nepodepsaný. Vypočítejme hodnotu výrazu za předpokladu, že funkce vrátí hodnotu 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Nyní si všimněte, že proměnná DataSize má 64bitový typ bez znaménka. Ukázalo se, že při provádění operace DataSize & 0xFFFFFFFF8u se všech třicet dva bitů vyššího řádu vynuluje. S největší pravděpodobností to není to, co programátor chtěl. Mám podezření, že chtěl vypočítat: DataSize & 0xFFFFFFFFFFFFFF8u.
Chcete-li chybu opravit, měli byste napsat toto:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Nebo tak:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Přetypování explicitního typu se nezdařilo
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);
....
}Upozornění PVS-Studio: [CWE-190] Možné přetečení. Zvažte přetypování operandů operátoru 'NumElts * Scale' na typ 'size_t', nikoli výsledek. X86ISelLowering.h 1577
Explicitní typ přetypování se používá k zamezení přetečení při násobení typových proměnných int. Explicitní typové obsazení zde však nechrání před přetečením. Nejprve dojde k vynásobení proměnných a teprve poté se 32bitový výsledek násobení rozšíří na typ .
Fragment N35: Kopírování a vkládání se nezdařilo
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] Byly nalezeny dva podobné fragmenty kódu. Možná je to překlep a místo 'Op1' by měla být použita proměnná 'Op0'. InstCombineCompares.cpp 5507
Tato nová zajímavá diagnostika identifikuje situace, kdy byl zkopírován kus kódu a některá jména v něm se začala měnit, ale na jednom místě to neopravili.
Upozorňujeme, že ve druhém bloku se změnily Op0 na Op1. Ale na jednom místě to neopravili. S největší pravděpodobností to mělo být napsáno takto:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Proměnná záměna
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Upozornění PVS-Studio: [CWE-563] Proměnná 'Mode' je přiřazena, ale do konce funkce se nepoužívá. SIModeRegister.cpp 48
Je velmi nebezpečné dávat argumentům funkcí stejná jména jako členům třídy. Je velmi snadné se splést. Právě takový případ máme před sebou. Tento výraz nedává smysl:
Mode &= Mask;Argument funkce se změní. To je vše. Tento argument se již nepoužívá. S největší pravděpodobností jsi to měl napsat takto:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Proměnná záměna
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;
}Varování PVS-Studio: V1001 [CWE-563] Proměnná 'Size' je přiřazena, ale do konce funkce se nepoužívá. Object.cpp 424
Situace je podobná předchozí. Mělo by být napsáno:
this->Size += this->EntrySize;Fragment N38-N47: Zapomněli zkontrolovat index
Dříve jsme se podívali na příklady diagnostického spouštění . Jeho podstatou je, že ukazatel je na začátku dereferencován a teprve poté kontrolován. Mladá diagnostika je významově opačný, ale také odhaluje spoustu chyb. Identifikuje situace, kdy byl ukazatel na začátku zkontrolován a poté zapomenut. Podívejme se na takové případy nalezené uvnitř LLVM.
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()); // <=
....
}Varování PVS-Studio: V1004 [CWE-476] Ukazatel 'Ptr' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 729, 738. TargetTransformInfoImpl.h 738
Proměnná Ptr mohou být rovné nullptr, o čemž svědčí šek:
if (Ptr != nullptr)Nicméně pod tímto ukazatelem je dereferencováno bez předběžné kontroly:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Podívejme se na další podobný případ.
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(); // <=
....
}Varování PVS-Studio: V1004 [CWE-476] Ukazatel 'FD' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 3228, 3231. CGDebugInfo.cpp 3231
Dávejte pozor na znamení FD. Jsem si jist, že problém je jasně viditelný a není potřeba žádné zvláštní vysvětlení.
A dál:
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()); // <=
....
}Varování PVS-Studio: V1004 [CWE-476] Ukazatel 'PtrTy' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 960, 965. InterleavedLoadCombinePass.cpp 965
Jak se před podobnými chybami chránit? Věnujte větší pozornost Code-Review a používejte statický analyzátor PVS-Studio k pravidelné kontrole kódu.
Nemá smysl citovat další fragmenty kódu s chybami tohoto typu. V článku ponechám pouze výčet varování:
- V1004 [CWE-476] Ukazatel 'Expr' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Ukazatel 'PI' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Ukazatel 'StatepointCall' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Ukazatel 'RV' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Ukazatel 'CalleeFn' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Ukazatel 'TC' byl použit nebezpečně poté, co byl ověřen proti nullptr. Kontrolní řádky: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Není kritický, ale je defektní (možný únik paměti)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Upozornění PVS-Studio: [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Strategies' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-isel-fuzzer.cpp 58
Chcete-li přidat prvek na konec kontejneru, jako je std::vektor > neumíš jen tak psát xxx.push_back(nové X), protože neexistuje žádná implicitní konverze z X* в std::unique_ptr.
Častým řešením je psaní xxx.emplace_back(nové X)protože kompiluje: metodu emplace_back konstruuje prvek přímo z jeho argumentů a může proto používat explicitní konstruktory.
Není to bezpečné. Pokud je vektor plný, pak se paměť znovu přidělí. Operace přerozdělení paměti může selhat, což má za následek vyvolání výjimky std::bad_alloc. V tomto případě bude ukazatel ztracen a vytvořený objekt nebude nikdy odstraněn.
Bezpečným řešením je tvořit unique_ptrkterý bude vlastnit ukazatel předtím, než se vektor pokusí přerozdělit paměť:
xxx.push_back(std::unique_ptr<X>(new X))Od C++ 14 můžete použít 'std::make_unique':
xxx.push_back(std::make_unique<X>())Tento typ defektu není pro LLVM kritický. Pokud paměť nelze alokovat, kompilátor se jednoduše zastaví. Nicméně pro aplikace s dlouhým , který se nemůže jen tak ukončit, pokud selže alokace paměti, to může být opravdu ošklivá chyba.
Takže ačkoli tento kód nepředstavuje praktickou hrozbu pro LLVM, považoval jsem za užitečné mluvit o tomto chybovém vzoru ao tom, že analyzátor PVS-Studio se ho naučil identifikovat.
Další varování tohoto typu:
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Passes' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. PassManager.h 546
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'AAs' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. AliasAnalysis.h 324
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Entries' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'AllEdges' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. CFGMST.h 268
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'VMaps' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Records' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. FDRLogBuilder.h 30
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'PendingSubmodules' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. ModuleMap.cpp 810
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Objects' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. DebugMap.cpp 88
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Strategies' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 685
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 686
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 688
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 689
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 690
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 691
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 692
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 693
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Modifiers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. llvm-stress.cpp 694
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Operandy' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Stash' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Ukazatel bez vlastníka je přidán do kontejneru 'Matchers' metodou 'emplace_back'. V případě výjimky dojde k nevracení paměti. GlobalISelEmitter.cpp 2702
Závěr
Vydal jsem celkem 60 varování a pak jsem přestal. Existují další vady, které analyzátor PVS-Studio detekuje v LLVM? Ano mám. Když jsem však vypisoval fragmenty kódu pro článek, byl pozdní večer, nebo spíše noc, a rozhodl jsem se, že je čas to nazvat dnem.
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 klíč od minolovky na .
A co je nejdůležitější, pravidelně používejte statickou analýzu. Jednorázové kontroly, kterou provádíme za účelem popularizace metodiky statické analýzy a PVS-Studio nejsou běžným scénářem.
Hodně štěstí při zlepšování kvality a spolehlivosti vašeho kódu!
Pokud chcete tento článek sdílet s anglicky mluvícím publikem, použijte odkaz na překlad: Andrey Karpov. .
Zdroj: www.habr.com