Au trecut mai bine de doi ani de la ultima verificare de cod a proiectului LLVM folosind analizorul nostru PVS-Studio. Să ne asigurăm că analizatorul PVS-Studio este încă un instrument de vârf pentru identificarea erorilor și a potențialelor vulnerabilități. Pentru a face acest lucru, vom verifica și găsi noi erori în versiunea LLVM 8.0.0.
Articol de scris
Sincer să fiu, nu am vrut să scriu acest articol. Nu este interesant să scriem despre un proiect pe care l-am verificat deja de mai multe ori (, , ). Este mai bine să scriu despre ceva nou, dar nu am de ales.
De fiecare dată când o nouă versiune de LLVM este lansată sau actualizată , primim întrebări de următorul tip în e-mailul nostru:
Uite, noua versiune a Clang Static Analyzer a învățat să găsească noi erori! Mi se pare că relevanța utilizării PVS-Studio este în scădere. Clang găsește mai multe erori decât înainte și ajunge din urmă cu capabilitățile PVS-Studio. Ce părere ai despre această?
La asta vreau mereu să răspund la ceva de genul:
Nici noi nu stam degeaba! Am îmbunătățit semnificativ capacitățile analizorului PVS-Studio. Așa că nu vă faceți griji, continuăm să conducem ca înainte.
Din păcate, acesta este un răspuns prost. Nu există dovezi în ea. Și de aceea scriu acest articol acum. Deci, proiectul LLVM a fost din nou verificat și au fost găsite o varietate de erori în el. Acum le voi demonstra pe cele care mi s-au părut interesante. Clang Static Analyzer nu poate găsi aceste erori (sau este extrem de incomod să faceți acest lucru cu ajutorul său). Dar noi putem. Mai mult, am găsit și notat toate aceste erori într-o singură seară.
Dar scrierea articolului a durat câteva săptămâni. Pur și simplu nu m-am putut decide să pun toate astea în text :).
Apropo, dacă sunteți interesat de ce tehnologii sunt utilizate în analizatorul PVS-Studio pentru a identifica erorile și potențialele vulnerabilități, atunci vă sugerez să vă familiarizați cu acest lucru. .
Diagnostice noi și vechi
După cum sa menționat deja, în urmă cu aproximativ doi ani, proiectul LLVM a fost din nou verificat, iar erorile găsite au fost corectate. Acum acest articol va prezenta un nou lot de erori. De ce au fost găsite erori noi? Există 3 motive pentru aceasta:
- Proiectul LLVM evoluează, schimbând codul vechi și adăugând cod nou. Desigur, există noi erori în codul modificat și scris. Acest lucru demonstrează clar că analiza statică ar trebui utilizată în mod regulat și nu ocazional. Articolele noastre arată bine capacitățile analizorului PVS-Studio, dar acest lucru nu are nimic de-a face cu îmbunătățirea calității codului și reducerea costului remedierii erorilor. Utilizați în mod regulat un analizor de cod static!
- Finalizăm și îmbunătățim diagnosticele existente. Prin urmare, analizorul poate identifica erori pe care nu le-a observat în timpul scanărilor anterioare.
- Au apărut noi diagnostice în PVS-Studio care nu existau acum 2 ani. Am decis să le evidențiez într-o secțiune separată pentru a arăta clar dezvoltarea PVS-Studio.
Defecte identificate prin diagnosticare care au existat acum 2 ani
Fragment N1: Copiere-Lipire
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
....
}Avertisment PVS-Studio: [CWE-570] Există sub-expresii identice „Nume.startswith("avx512.mask.permvar.")' la stânga și la dreapta lui '||' operator. AutoUpgrade.cpp 73
Se verifică de două ori dacă numele începe cu subșirul „avx512.mask.permvar.”. La a doua verificare, evident că au vrut să scrie altceva, dar au uitat să corecteze textul copiat.
Fragment N2: greșeală
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;
....
}Avertisment PVS-Studio: V501 Există sub-expresii identice „CXNameRange_WantQualifier” la stânga și la dreapta lui „|” operator. CIndex.cpp 7245
Din cauza unei greșeli de tipar, aceeași constantă numită este folosită de două ori CXNameRange_WantQualifier.
Fragment N3: Confuzie cu prioritatea operatorului
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Avertisment PVS-Studio: [CWE-783] Poate că operatorul „?:” funcționează într-un mod diferit decât era de așteptat. Operatorul „?:” are o prioritate mai mică decât operatorul „==”. PPCTargetTransformInfo.cpp 404
După părerea mea, aceasta este o greșeală foarte frumoasă. Da, știu că am idei ciudate despre frumusețe :).
Acum, conform , expresia este evaluată după cum urmează:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Din punct de vedere practic, o astfel de condiție nu are sens, deoarece se poate reduce la:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Aceasta este o greșeală clară. Cel mai probabil, au vrut să compare 0/1 cu o variabilă index. Pentru a remedia codul, trebuie să adăugați paranteze în jurul operatorului ternar:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Apropo, operatorul ternar este foarte periculos și provoacă erori logice. Fii foarte atent cu el și nu fi lacom cu parantezele. M-am uitat la acest subiect mai detaliat , în capitolul „Atenție la ?: Operator și închideți-l în paranteze.”
Fragment N4, N5: Pointer nul
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;
}
....
}Avertisment PVS-Studio: [CWE-476] Ar putea avea loc dereferențiarea pointerului nul „LHS”. TGParser.cpp 2152
Dacă indicatorul LHS este nulă, trebuie emis un avertisment. Cu toate acestea, în schimb, același indicator nul va fi dereferențiat: LHS->getAsString().
Aceasta este o situație foarte tipică când o eroare este ascunsă într-un handler de erori, deoarece nimeni nu le testează. Analizoarele statice verifică tot codul accesibil, indiferent cât de des este folosit. Acesta este un exemplu foarte bun al modului în care analiza statică completează alte tehnici de testare și de protecție împotriva erorilor.
Eroare similară de manipulare a indicatorului RHS permis în codul de mai jos: V522 [CWE-476] Ar putea avea loc dereferențiarea pointerului nul „RHS”. TGParser.cpp 2186
Fragment N6: Folosirea indicatorului după mutare
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);
}
....
}Avertisment PVS-Studio: V522 [CWE-476] Ar putea avea loc dereferențiarea pointerului nul „ProgClone”. Compilare greșită.cpp 601
La început un indicator inteligent ProgClone încetează să dețină obiectul:
BD.setNewProgram(std::move(ProgClone));De fapt, acum ProgClone este un pointer nul. Prin urmare, o dereferire a indicatorului nul ar trebui să apară chiar mai jos:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Dar, în realitate, acest lucru nu se va întâmpla! Rețineți că bucla nu este de fapt executată.
La începutul recipientului Funcții compilate greșit sters:
MiscompiledFunctions.clear();În continuare, dimensiunea acestui container este utilizată în condiția buclei:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Este ușor de observat că bucla nu începe. Cred că și acesta este o eroare și codul ar trebui scris diferit.
Se pare că am întâlnit acea faimoasă paritate a erorilor! O greșeală maschează alta :).
Fragment N7: Folosirea indicatorului după mutare
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;
}
....
}Avertisment PVS-Studio: V522 [CWE-476] Ar putea avea loc dereferențiarea pointerului nul „Test”. Compilare greșită.cpp 709
Din nou aceeași situație. La început, conținutul obiectului este mutat, apoi este folosit ca și cum nimic nu s-ar fi întâmplat. Văd această situație din ce în ce mai des în codul programului după ce semantica mișcării a apărut în C++. Acesta este motivul pentru care îmi place limbajul C++! Există din ce în ce mai multe moduri noi de a-ți trage singur piciorul. Analizorul PVS-Studio va avea întotdeauna de lucru :).
Fragment N8: Pointer nul
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);
}Avertisment PVS-Studio: V522 [CWE-476] S-ar putea să aibă loc dereferențiarea pointerului nul „Tip”. PrettyFunctionDumper.cpp 233
Pe lângă manipulatorii de erori, funcțiile de imprimare de depanare nu sunt de obicei testate. Avem doar un astfel de caz în fața noastră. Funcția îl așteaptă pe utilizator, care, în loc să-și rezolve problemele, va fi obligat să o repare.
corecta:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Pointer nul
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());
....
}Avertisment PVS-Studio: V522 [CWE-476] Ar putea avea loc dereferențiarea pointerului nul „Ty”. SearchableTableEmitter.cpp 614
Cred că totul este clar și nu necesită explicații.
Fragment N10: greșeală
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;
}Avertisment PVS-Studio: Variabila „Identifier->Type” este atribuită ei însăși. FormatTokenLexer.cpp 249
Nu are rost să-i atribui o variabilă. Cel mai probabil au vrut să scrie:
Identifier->Type = Question->Type;Fragment N11: Rupere suspectă
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Avertisment PVS-Studio: [CWE-478] Luați în considerare inspectarea declarației „comutator”. Este posibil ca primul operator „caz” să lipsească. SystemZAsmParser.cpp 652
Există un operator foarte suspect la început rupe. Ai uitat să scrii altceva aici?
Fragment N12: Verificarea unui indicator după dereferențiere
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");
....
}Avertisment PVS-Studio: [CWE-476] Indicatorul „Callee” a fost utilizat înainte de a fi verificat împotriva nullptr. Verifică linii: 172, 174. AMDGPUInline.cpp 172
Pointer Callee la început este dereferențiat în momentul în care funcția este apelată getTTI.
Și apoi se dovedește că acest indicator ar trebui verificat pentru egalitate nullptr:
if (!Callee || Callee->isDeclaration())Dar e prea tarziu...
Fragment N13 - N...: Verificarea unui indicator după dereferențiere
Situația discutată în fragmentul de cod anterior nu este unică. Apare aici:
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()) { // <=
....
}Avertisment PVS-Studio: V595 [CWE-476] Indicatorul „CalleeFn” a fost utilizat înainte de a fi verificat împotriva nullptr. Verifică linii: 1079, 1081. SimplifyLibCalls.cpp 1079
Si aici:
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()); // <=
....
}Avertisment PVS-Studio: V595 [CWE-476] Pointerul „ND” a fost folosit înainte de a fi verificat împotriva nullptr. Verifică linii: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Si aici:
- V595 [CWE-476] Indicatorul „U” a fost utilizat înainte de a fi verificat împotriva nullptr. Verifică linii: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Indicatorul „ND” a fost utilizat înainte de a fi verificat împotriva nullptr. Verifică linii: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Și apoi am devenit neinteresat să studiez avertismentele cu numărul V595. Deci nu știu dacă există mai multe erori similare în afară de cele enumerate aici. Cel mai probabil există.
Fragment N17, N18: Schimbare suspectă
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Avertisment PVS-Studio: [CWE-190] Luați în considerare inspectarea expresiei „~(Dimensiune - 1) << 1”. Schimbarea de biți a valorii de 32 de biți cu o extindere ulterioară la tipul de 64 de biți. AArch64AddressingModes.h 260
Este posibil să nu fie o eroare și codul funcționează exact așa cum a fost prevăzut. Dar acesta este în mod clar un loc foarte suspect și trebuie verificat.
Să spunem variabila Mărimea este egal cu 16, iar apoi autorul codului a plănuit să îl introducă într-o variabilă NImms valoarea:
1111111111111111111111111111111111111111111111111111111111100000
Cu toate acestea, în realitate, rezultatul va fi:
0000000000000000000000000000000011111111111111111111111111100000
Faptul este că toate calculele au loc folosind tipul nesemnat pe 32 de biți. Și numai atunci, acest tip nesemnat pe 32 de biți va fi implicit extins la uint64_t. În acest caz, cei mai semnificativi biți vor fi zero.
Puteți remedia situația astfel:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Situație similară: V629 [CWE-190] Luați în considerare inspectarea expresiei „Immr << 6”. Schimbarea de biți a valorii de 32 de biți cu o extindere ulterioară la tipul de 64 de biți. AArch64AddressingModes.h 269
Fragment N19: Lipsește cuvântul cheie altfel?
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");
}
....
}Avertisment PVS-Studio: [CWE-670] Luați în considerare inspectarea logicii aplicației. Este posibil ca cuvântul cheie „altfel” să lipsească. AMDGPUAsmParser.cpp 5655
Nu există nicio greșeală aici. De atunci-blocul primului if se termină cu continua, atunci nu contează, există un cuvânt cheie altfel sau nu. În orice caz, codul va funcționa la fel. Încă ratat altfel face codul mai neclar și mai periculos. Dacă în viitor continua dispare, codul va începe să funcționeze complet diferit. După părerea mea, este mai bine să adaug altfel.
Fragment N20: Patru greșeli de scriere de același tip
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;
}Avertismente PVS-Studio:
- V655 [CWE-480] Șirurile au fost concatenate, dar nu sunt folosite. Luați în considerare inspectarea expresiei „Rezultat + Nume.str()”. Simbol.cpp 32
- V655 [CWE-480] Șirurile au fost concatenate, dar nu sunt folosite. Luați în considerare inspectarea expresiei „Rezultat + „(Clasa ObjC)” + Nume.str()”. Simbol.cpp 35
- V655 [CWE-480] Șirurile au fost concatenate, dar nu sunt folosite. Luați în considerare inspectarea expresiei „Rezultat + „(ObjC Class EH)” + Name.str()”. Simbol.cpp 38
- V655 [CWE-480] Șirurile au fost concatenate, dar nu sunt folosite. Luați în considerare inspectarea expresiei „Rezultat + „(ObjC IVar)” + Nume.str()”. Simbol.cpp 41
Din întâmplare, se folosește operatorul + în locul operatorului +=. Rezultatul sunt modele care sunt lipsite de sens.
Fragment N21: Comportament nedefinit
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();
}
}
}Încercați să găsiți singur codul periculos. Și aceasta este o imagine pentru a distrage atenția pentru a nu privi imediat răspunsul:
Avertisment PVS-Studio: [CWE-758] Se utilizează o construcție periculoasă: „FeaturesMap[Op] = FeaturesMap.size()”, unde „FeaturesMap” este din clasa „map”. Acest lucru poate duce la un comportament nedefinit. RISCVCompressInstEmitter.cpp 490
Linia problemei:
FeaturesMap[Op] = FeaturesMap.size();Dacă elementul Op nu este găsit, atunci este creat un nou element în hartă și numărul de elemente din această hartă este scris acolo. Nu se știe dacă funcția va fi apelată mărimea înainte sau după adăugarea unui nou element.
Fragment N22-N24: Atribuții repetate
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;
}
....
}Avertisment PVS-Studio: [CWE-563] Variabilei „NType” i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Verifică linii: 1663, 1664. MachOObjectFile.cpp 1664
Nu cred că este o adevărată greșeală aici. Doar o misiune repetată inutilă. Dar tot o gafă.
În mod similar:
- V519 [CWE-563] Variabilei „B.NDesc” i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Verifică linii: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Variabilei i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Linii de verificare: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Mai multe realocări
Acum să ne uităm la o versiune ușor diferită de reatribuire.
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;
....
}Avertisment PVS-Studio: V519 [CWE-563] Variabilei „Aliniere” i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Verifică linii: 1158, 1160. LoadStoreVectorizer.cpp 1160
Acesta este un cod foarte ciudat care aparent conține o eroare logică. La început, variabilă Aliniere se atribuie o valoare în funcție de condiție. Și apoi sarcina are loc din nou, dar acum fără nicio verificare.
Situații similare pot fi văzute aici:
- V519 [CWE-563] Variabilei „Efecte” i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Verifică linii: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Variabilei „ExpectNoDerefChunk” i se atribuie valori de două ori succesiv. Poate că aceasta este o greșeală. Verifică linii: 4970, 4973. SemaType.cpp 4973
Fragment N28: Stare întotdeauna adevărată
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;
}
....
}Avertisment PVS-Studio: [CWE-571] Expresia „nextByte != 0x90” este întotdeauna adevărată. X86DisassemblerDecoder.cpp 379
Verificarea nu are sens. Variabil nextByte întotdeauna nu este egal cu valoarea 0x90, care rezultă din verificarea anterioară. Acesta este un fel de eroare logică.
Fragment N29 - N...: Întotdeauna condiții adevărate/false
Analizorul emite multe avertismente că întreaga stare () sau o parte a acestuia () este întotdeauna adevărată sau falsă. Adesea, acestea nu sunt erori reale, ci pur și simplu cod neglijent, rezultatul extinderii macro și altele asemenea. Cu toate acestea, este logic să privim toate aceste avertismente, deoarece erorile logice autentice apar din când în când. De exemplu, această secțiune de cod este suspectă:
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;
....
}Avertisment PVS-Studio: [CWE-570] O parte a expresiei condiționate este întotdeauna falsă: RegNo == 0xe. ARMDisassembler.cpp 939
Constanta 0xE este valoarea 14 în zecimală. Examinare RegNo == 0xe nu are sens pentru că dacă RegNo > 13, atunci funcția își va finaliza execuția.
Au existat multe alte avertismente cu ID-urile V547 și V560, dar ca și cu , nu m-a interesat să studiez aceste avertismente. Era deja clar că am material suficient pentru a scrie un articol :). Prin urmare, nu se știe câte erori de acest tip pot fi identificate în LLVM folosind PVS-Studio.
Vă voi da un exemplu de ce studierea acestor declanșatoare este plictisitoare. Analizatorul are perfectă dreptate în a emite un avertisment pentru următorul cod. Dar aceasta nu este o greșeală.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Avertisment PVS-Studio: V547 [CWE-570] Expresia „!HasError” este întotdeauna falsă. UnwrappedLineParser.cpp 1635
Fragment N30: Întoarcere suspectă
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();
}
....
}Avertisment PVS-Studio: [CWE-670] O „întoarcere” necondiționată într-o buclă. R600OptimizeVectorRegisters.cpp 63
Aceasta este fie o eroare, fie o tehnică specifică care are scopul de a explica ceva programatorilor care citesc codul. Acest design nu îmi explică nimic și pare foarte suspect. E mai bine sa nu scrii asa :).
Obosit? Atunci este timpul să faci ceai sau cafea.
Defecte identificate prin noi diagnostice
Cred că 30 de activări ale diagnosticelor vechi sunt suficiente. Să vedem acum ce lucruri interesante se pot găsi cu noile diagnostice apărute în analizor după verificări. În acest timp, un total de 66 de diagnostice de uz general au fost adăugate la analizorul C++.
Fragment N31: Cod inaccesibil
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();
}Avertisment PVS-Studio: [CWE-561] Cod inaccesibil detectat. Este posibil să existe o eroare. ExecutionUtils.cpp 146
După cum puteți vedea, ambele ramuri ale operatorului if se încheie cu un apel către operator reveni. În consecință, containerul CtorDtorsByPriority nu va fi niciodată șters.
Fragment N32: Cod inaccesibil
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;
}Avertisment PVS-Studio: V779 [CWE-561] Cod inaccesibil detectat. Este posibil să existe o eroare. LLParser.cpp 835
Interesanta situatie. Să ne uităm mai întâi la acest loc:
return ParseTypeIdEntry(SummaryID);
break;La prima vedere, se pare că nu există nicio eroare aici. Arată ca operatorul rupe există unul suplimentar aici și îl puteți șterge pur și simplu. Cu toate acestea, nu toate sunt atât de simple.
Analizatorul emite un avertisment pe liniile:
Lex.setIgnoreColonInIdentifiers(false);
return false;Și într-adevăr, acest cod este inaccesibil. Toate cazurile în comuta se încheie cu un apel de la operator reveni. Și acum fără sens singur rupe nu arata atat de inofensiv! Poate că una dintre ramuri ar trebui să se termine cu rupe, nu pe reveni?
Fragment N33: Resetare aleatorie a biților înalți
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);
....
}Avertisment PVS-Studio: Mărimea măștii de biți este mai mică decât dimensiunea primului operand. Acest lucru va cauza pierderea de biți mai mari. RuntimeDyld.cpp 815
Vă rugăm să rețineți că funcția getStubAlignment returnează tipul nesemnat. Să calculăm valoarea expresiei, presupunând că funcția returnează valoarea 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Acum observați că variabila DataSize are un tip nesemnat pe 64 de biți. Se pare că atunci când se efectuează operația DataSize & 0xFFFFFFF8u, toți cei treizeci și doi de biți de ordin înalt vor fi resetati la zero. Cel mai probabil, nu asta și-a dorit programatorul. Bănuiesc că a vrut să calculeze: DataSize & 0xFFFFFFFFFFFFFFF8u.
Pentru a remedia eroarea, ar trebui să scrieți asta:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Sau așa:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Tipul explicit eșuat
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);
....
}Avertisment PVS-Studio: [CWE-190] Posibil preaplin. Luați în considerare turnarea operanzilor operatorului „NumElts * Scale” la tipul „size_t”, nu la rezultat. X86ISelLowering.h 1577
Castingul de tip explicit este folosit pentru a evita depășirea atunci când se înmulțesc variabilele de tip int. Cu toate acestea, turnarea de tip explicit aici nu protejează împotriva depășirii. Mai întâi, variabilele vor fi înmulțite și abia apoi rezultatul înmulțirii pe 32 de biți va fi extins la tipul .
Fragment N35: Copiere-lipire eșuată
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] Au fost găsite două fragmente de cod similare. Poate că aceasta este o greșeală de tipar și ar trebui folosită variabila „Op1” în loc de „Op0”. InstCombineCompares.cpp 5507
Acest nou diagnostic interesant identifică situațiile în care o bucată de cod a fost copiată și unele nume din ea au început să fie schimbate, dar într-un loc nu l-au corectat.
Vă rugăm să rețineți că în al doilea bloc s-au schimbat Op0 pe Op1. Dar într-un loc nu au rezolvat-o. Cel mai probabil ar fi trebuit scris astfel:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Confuzie variabilă
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Avertisment PVS-Studio: [CWE-563] Variabila „Mod” este atribuită, dar nu este utilizată până la sfârșitul funcției. SIModeRegister.cpp 48
Este foarte periculos să dai argumentelor funcției aceleași nume ca membrii clasei. Este foarte ușor să fii confuz. Avem doar un astfel de caz în fața noastră. Această expresie nu are sens:
Mode &= Mask;Argumentul funcției se schimbă. Asta e tot. Acest argument nu mai este folosit. Cel mai probabil ar fi trebuit să scrii așa:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Confuzie variabilă
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;
}Avertisment PVS-Studio: V1001 [CWE-563] Variabila „Dimensiune” este atribuită, dar nu este utilizată până la sfârșitul funcției. Obiect.cpp 424
Situația este similară cu cea anterioară. Ar trebui scris:
this->Size += this->EntrySize;Fragment N38-N47: Au uitat să verifice indexul
Anterior, am analizat exemple de declanșare a diagnosticului . Esența sa este că indicatorul este dereferențiat la început și abia apoi verificat. Diagnostice tinere are sensul opus, dar dezvăluie și o mulțime de erori. Identifică situațiile în care indicatorul a fost verificat la început și apoi a uitat să facă acest lucru. Să ne uităm la astfel de cazuri găsite în 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()); // <=
....
}Avertisment PVS-Studio: V1004 [CWE-476] Pointerul „Ptr” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 729, 738. TargetTransformInfoImpl.h 738
variabil PTR poate fi egal nullptr, după cum reiese din cec:
if (Ptr != nullptr)Cu toate acestea, sub acest indicator este dereferențiat fără verificare preliminară:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Să luăm în considerare un alt caz similar.
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(); // <=
....
}Avertisment PVS-Studio: V1004 [CWE-476] Pointerul „FD” a fost utilizat în mod nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 3228, 3231. CGDebugInfo.cpp 3231
Acordați atenție semnului FD. Sunt sigur că problema este clar vizibilă și nu este necesară o explicație specială.
Și mai departe:
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()); // <=
....
}Avertisment PVS-Studio: V1004 [CWE-476] Pointerul „PtrTy” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 960, 965. InterleavedLoadCombinePass.cpp 965
Cum să te protejezi de astfel de erori? Fiți mai atenți la Code-Review și utilizați analizorul static PVS-Studio pentru a vă verifica regulat codul.
Nu are rost să cităm alte fragmente de cod cu erori de acest tip. Voi lăsa doar o listă de avertismente în articol:
- V1004 [CWE-476] Pointerul „Expr” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Pointerul „PI” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Pointerul „StatepointCall” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Indicatorul „RV” a fost folosit în mod nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Pointerul „CalleeFn” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Pointerul „TC” a fost folosit nesigur după ce a fost verificat împotriva nullptr. Verifică linii: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Nu este critic, dar este un defect (posibilă scurgere de memorie)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Avertisment PVS-Studio: [CWE-460] Un pointer fără proprietar este adăugat la containerul „Strategii” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-isel-fuzzer.cpp 58
Pentru a adăuga un element la capătul unui container, cum ar fi std::vector > nu poți doar să scrii xxx.push_back(nou X), deoarece nu există o conversie implicită din X* в std::unique_ptr.
O soluție comună este să scrieți xxx.emplace_back(nou X)întrucât compilează: metodă plasează_înapoi construiește un element direct din argumentele sale și, prin urmare, poate folosi constructori expliciți.
Nu este sigur. Dacă vectorul este plin, atunci memoria este realocată. Operația de realocare a memoriei poate eșua, rezultând o excepție std::bad_alloc. În acest caz, indicatorul va fi pierdut și obiectul creat nu va fi niciodată șters.
O soluție sigură este crearea unic_ptrcare va deține indicatorul înainte ca vectorul să încerce să realoce memoria:
xxx.push_back(std::unique_ptr<X>(new X))Din C++14, puteți folosi „std::make_unique”:
xxx.push_back(std::make_unique<X>())Acest tip de defect nu este critic pentru LLVM. Dacă memoria nu poate fi alocată, compilatorul se va opri pur și simplu. Cu toate acestea, pentru aplicații cu lung , care nu se poate termina doar dacă alocarea memoriei eșuează, aceasta poate fi o eroare reală urâtă.
Deci, deși acest cod nu reprezintă o amenințare practică pentru LLVM, mi s-a părut util să vorbesc despre acest model de eroare și că analizatorul PVS-Studio a învățat să-l identifice.
Alte avertismente de acest tip:
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul 'Passes' prin metoda 'emplace_back'. O scurgere de memorie va avea loc în cazul unei excepții. PassManager.h 546
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „AAs” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. AliasAnalysis.h 324
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Entries” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „AllEdges” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. CFGMST.h 268
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „VMaps” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Înregistrări” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. FDRLogBuilder.h 30
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „PendingSubmodules” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. ModuleMap.cpp 810
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Obiecte” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. DebugMap.cpp 88
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Strategii” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 685
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 686
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 688
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 689
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 690
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 691
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 692
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 693
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Modificatori” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. llvm-stress.cpp 694
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Operanzi” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Stash” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Un pointer fără proprietar este adăugat la containerul „Matchers” prin metoda „emplace_back”. O scurgere de memorie va avea loc în cazul unei excepții. GlobalISelEmitter.cpp 2702
Concluzie
Am emis 60 de avertismente în total și apoi m-am oprit. Există și alte defecte pe care analizorul PVS-Studio le detectează în LLVM? Da, am. Cu toate acestea, când scriam fragmente de cod pentru articol, era seara târziu, sau mai degrabă chiar noaptea, și am decis că era timpul să numim asta.
Sper că l-ai găsit interesant și vei dori să încerci analizorul PVS-Studio.
Puteți descărca analizorul și obțineți cheia pentru mine de la .
Cel mai important, utilizați în mod regulat analiza statică. Verificări unice, realizat de noi în scopul popularizării metodologiei de analiză statică și PVS-Studio nu sunt un scenariu normal.
Mult succes în îmbunătățirea calității și fiabilității codului dvs.!
Dacă doriți să împărtășiți acest articol unui public vorbitor de engleză, vă rugăm să folosiți linkul de traducere: Andrey Karpov. .
Sursa: www.habr.com