
Det er over to år siden den seneste kodegennemgang af LLVM-projektet blev udført med vores PVS-Studio-analysert. Lad os sikre os, at PVS-Studio-analysert stadig er det førende værktøj til at finde fejl og potentielle sårbarheder. For at gøre dette vil vi tjekke og finde nye fejl i version 8.0.0 af LLVM.
Artikel, der skal skrives
Ærligt talt, jeg havde ikke lyst til at skrive denne artikel. Det er ikke interessant at skrive om et projekt, vi allerede har gennemgået mange gange (, , ). Det ville være bedre at skrive om noget nyt, men jeg har ikke noget valg.
Hver gang der kommer en ny version af LLVM eller , får vi spørgsmål i vores indbakke af følgende art:
Se, den nye version af Clang Static Analyzer har lært at finde nye fejl! Jeg synes, relevansen af at bruge PVS-Studio falder. Clang finder flere fejl end før og indhenter PVS-Studio i sine muligheder. Hvad mener du om dette?
Til det vil jeg altid have lyst til at svare noget i stil med:
Vi sidder også ikke stille! Vi har væsentligt forbedret PVS-Studio-analysatorens muligheder. Så lad dig ikke bekymre, vi fortsætter med at lede, som vi altid har gjort.
Desværre er det et dårligt svar. Der er ikke nogen beviser i det. Og netop derfor skriver jeg denne artikel nu. Så projektet LLVM er blevet tjekket igen, og der er fundet et væld af forskellige fejl. De, som jeg fandt interessante, vil jeg nu vise. Disse fejl kan Clang Static Analyzer ikke finde (eller det er ekstremt besværligt at gøre det med det). Og vi kan. Desuden fandt og noterede jeg alle disse fejl på en aften.
Men skrivningen af artiklen tog flere uger. Jeg kunne simpelthen ikke tvinge mig selv til at få det hele skrevet ned som tekst :)
Forresten, hvis du er interesseret i, hvilke teknologier der anvendes i PVS-Studio-analysert til at finde fejl og potentielle sårbarheder, så vil jeg gerne præsentere denne .
Nye og gamle diagnoser
Som allerede nævnt, blev LLVM-projektet for omkring to år siden igen gennemgået, og de fundne fejl blev rettet. Nu vil denne artikel præsentere en ny portion fejl. Hvorfor blev der fundet nye fejl? Der er tre grunde til det:
- LLVM-projektet udvikler sig, der rettes gammel kode, og ny kode tilføjes. Naturligvis medfører den ændrede og ny skrivne kode nye fejl. Dette viser tydeligt, at statisk analyse skal anvendes regelmæssigt og ikke blot en gang imellem. Vore artikler viser tydeligt mulighederne for PVS-Studio-analysereren, men det har ikke noget med at forbedre kodens kvalitet eller reducere omkostningerne ved at rette fejl. Brug en statisk kodeanalysator regelmæssigt!
- Vi arbejder på at opdatere og forbedre de eksisterende diagnoser. Derfor kan analysatoren finde fejl, som ikke blev opdaget ved tidligere kontroller.
- PVS-Studio har fået nye diagnoser, som ikke var til stede for 2 år siden. Jeg har valgt at fremhæve dem i en separat sektion for tydelig at vise udviklingen af PVS-Studio.
Fejl opdaget af diagnoser, der eksisterede for 2 år siden
Fragment N1: Copy-Paste
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Tilføjet i 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Tilføjet i 7.0
Name == "avx512.mask.cvtps2pd.256" || // Tilføjet i 7.0
Name == "avx512.cvtusi2sd" || // Tilføjet i 7.0
Name.startswith("avx512.mask.permvar.") || // Tilføjet i 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Tilføjet i 7.0 // <=
Name == "sse2.pmulu.dq" || // Tilføjet i 7.0
Name == "sse41.pmuldq" || // Tilføjet i 7.0
Name == "avx2.pmulu.dq" || // Tilføjet i 7.0
....
}Advarsel fra PVS-Studio: [CWE-570] Der er identiske deludtryk ‘Name.startswith(«avx512.mask.permvar.»)’ til venstre og højre for ‘||’ operatoren. AutoUpgrade.cpp 73
Det tjekkes to gange, om navnet begynder med strengen «avx512.mask.permvar.». I den anden kontrol mente man tydeligvis at skrive noget andet, men glemte at rette den kopiérede tekst.
Fragment N2: Tastefejl
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;
....
}Advarsel fra PVS-Studio: V501 Der er identiske deludtryk ‘CXNameRange_WantQualifier’ til venstre og højre for ‘|’ operatoren. CIndex.cpp 7245
På grund af en tastefejl anvendes den samme navngivne konstant to gange. CXNameRange_WantQualifier.
Fragment N3: Forvirring med operatorernas prioriteter
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Advarsel fra PVS-Studio: [CWE-783] Måske fungerer ‘?:’ operatoren på en anden måde, end forventet. ‘?:’ operatoren har en lavere prioritet end ‘==’ operatoren. PPCTargetTransformInfo.cpp 404
Jeg mener, at dette er en meget smuk fejl. Ja, jeg ved, at jeg har en mærkelig opfattelse af skønhed :).
Nu, ifølge , beregnes udtrykket som følger:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Fra et praktisk synspunkt giver denne betingelse ikke mening, da den kan forkortes til:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Dette er en klar fejl. Det er sandsynligt, at man ønskede at sammenligne 0/1 med variablen Index. For at rette koden skal der tilføjes parenteser omkring den ternære operator:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))For resten er den ternære operator meget farlig og kan forårsage logiske fejl. Vær meget forsigtig med den, og vær ikke for grådig med at sætte runde parenteser. Jeg har tidligere dækket dette emne , i kapitlet 'Frygt operatoren ?: og sæt den i parentes'.
Fragment N4, N5: Nullpointer
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("kan ikke caste '") + LHS->getAsString() +
"' til streng");
return nullptr;
}
....
}Advarsel fra PVS-Studio: [CWE-476] Dereferencing of the null pointer ‘LHS’ might take place. TGParser.cpp 2152
Hvis pointeren LHS er nul, bør der gives en advarsel. Men i stedet vil der ske en dereferering af denne nul-pointer: LHS->getAsString().
Dette er en meget typisk situation, hvor en fejl gemmer sig i fejlhåndtereren, da den ikke testes. Statisk analyserer verificerer al nåbar kode, uanset hvor ofte den bruges. Dette er et meget godt eksempel på, hvordan statisk analyse supplerer andre testmetoder og fejlbeskyttelse.
En tilsvarende pointerbehandlingsfejl RHS er begået i koden lige nedenunder: V522 [CWE-476] Dereferencing of the null pointer ‘RHS’ might take place. TGParser.cpp 2186
Fragment N6: Brug af pointer efter flytning
static Expected
ExtractBlocks(....)
{
....
std::unique_ptr ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Funktionen blev ikke fundet??");
MiscompiledFunctions.push_back(NewF);
}
....
}Advarsel PVS-Studio: V522 [CWE-476] Dereferencing of the null pointer ‘ProgClone’ might take place. Miscompilation.cpp 601
I starten ophører den smarte pointer ProgClone med at eje objektet:
BD.setNewProgram(std::move(ProgClone));Faktisk, nu ProgClone — en nulpeger. Derfor bør der lidt længere nede finde sted en dereferensering af nulpegeren:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Men faktisk vil det ikke ske! Bemærk, at løkken faktisk ikke udføres.
I starten af containeren MiscompiledFunctions bliver tømt:
MiscompiledFunctions.clear();Derefter bruges størrelsen af denne container i løkkens betingelse:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Det er let at se, at løkken ikke starter. Jeg tror, det også er en fejl, og koden burde have været skrevet anderledes.
Det ser ud til, at vi har ramt den berømte fejlsymmetri! En fejl skjuler en anden :) .
Fragment N7: Brug af peger efter flytning
static Expected TestOptimizer(BugDriver &BD, std::unique_ptr Test,
std::unique_ptr Safe) {
outs() << " Optimering af de testede funktioner: ";
std::unique_ptr Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Fejl ved kørsel af denne rækkefølge af passes"
<< " på inputprogrammet!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}Advarsel PVS-Studio: V522 [CWE-476] Dereferensering af nulpegeren ‘Test’ kan finde sted. Miscompilation.cpp 709
Igen er det samme problem. I begyndelsen flyttes indholdet af objektet, og derefter bruges det som om intet var hændt. Jeg støder oftere og oftere på denne situation i kode, efter at semantikken for flytning blev introduceret i C++. Det er derfor, jeg elsker C++! Der er hele tiden nye måder at skyde sig selv i foden på. PVS-Studio vil altid have arbejde :).
Fragment N8: Nulpeger
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "";
else
Type->dump(*this);
}Advarsel PVS-Studio: V522 [CWE-476] Dereferensering af nulpegeren ‘Type’ kan finde sted. PrettyFunctionDumper.cpp 233
Udover fejlbehandlingsrutiner bliver funktioner til fejludskrift af data normalt ikke testet. Dette er netop et sådant tilfælde. Funktionen venter på en bruger, der i stedet for at løse sine problemer vil blive tvunget til at rette op på den.
Korrekt:
if (Type)
Type->dump(*this);
else
Printer << "";Fragment N9: Nulpeger
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // getAsString() + " vs. " + // getType()->getAsString());
....
}Advarsel PVS-Studio: V522 [CWE-476] Der kan ske dereferering af den nullpeger ‘Ty’. SearchableTableEmitter.cpp 614
Jeg tror, at alt er klart og ikke kræver forklaring.
Fragment N10: Tastefejl
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;
}Advarsel fra PVS-Studio: Variablen ‘Identifier->Type’ tildeles til sig selv. FormatTokenLexer.cpp 249
Det giver ikke mening at tildele en variabel til sig selv. Man ønskede sandsynligvis at skrive:
Identifier->Type = Question->Type;Fragment N11: Mistænkelig 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;
....
}Advarsel fra PVS-Studio: [CWE-478] Overvej at inspicere ‘switch’-sætningen. Det er muligt, at den første ‘case’ operator mangler. SystemZAsmParser.cpp 652
Der er en meget mistænkelig operator i starten break. Glemte de mon at skrive noget mere her?
Fragment N12: Tjek af pointer efter dereferering
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");
....
}Advarsel fra PVS-Studio: [CWE-476] Pointer’en ‘Callee’ blev anvendt, før den blev bekræftet mod nullptr. Tjek linjer: 172, 174. AMDGPUInline.cpp 172
Pointer Callee bliver derefereret ved funktionsopkaldet getTTI.
Og så viser det sig, at denne pointer skal kontrolleres for lighed nullptr:
if (!Callee || Callee->isDeclaration())Men det er allerede for sent…
Fragment N13 — N…: Tjek af pointer efter dereferering
Situationer som den, der er beskrevet i det tidligere kodefragment, er ikke unikke. De forekommer her:
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()) { // <=
....
}Advarsel PVS-Studio: V595 [CWE-476] Pointer’en ‘CalleeFn’ blev anvendt, før den blev bekræftet mod nullptr. Tjek linjer: 1079, 1081. SimplifyLibCalls.cpp 1079
Og her:
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()); // <=
....
}Advarsel fra PVS-Studio: V595 [CWE-476] Pointeren 'ND' blev brugt, før den blev kontrolleret mod nullptr. Tjek linjer: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Og her:
- V595 [CWE-476] Pointeren 'U' blev brugt, før den blev kontrolleret mod nullptr. Tjek linjer: 404, 407. DWARFFormValue.cpp 404
- V595 [CWE-476] Pointeren 'ND' blev brugt, før den blev kontrolleret mod nullptr. Tjek linjer: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Og herefter blev jeg ikke længere interesseret i at studere advarslerne med nummer V595. Så jeg ved ikke, om der er andre lignende fejl end dem, der er nævnt her. Sandsynligvis er der.
Fragment N17, N18: Mistænkelig forskydning
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Advarsel fra PVS-Studio: [CWE-190] Overvej at inspicere udtrykket ‘~(Size — 1) << 1’. Bitforskydning af 32-bit værdien med efterfølgende udvidelse til 64-bit type. AArch64AddressingModes.h 260
Det er muligvis ikke en fejl, og koden fungerer præcis som tiltænkt. Men dette er tydeligvis et meget mistænkeligt sted, og det skal kontrolleres.
Lad os sige, at variablen Størrelse er lig med 16, og så havde forfatteren af koden til hensigt at få variablen NImms til at have værdien:
1111111111111111111111111111111111111111111111111111111111100000
Men i virkeligheden vil værdien blive:
0000000000000000000000000000000011111111111111111111111111100000
Sagen er, at alle beregninger udføres med en 32-bit unsigned type. Og først derefter vil denne 32-bit unsigned type blive implicit udvidet til uint64_t. Her vil de højere bits være nul.
Situationen kan rettes således:
uint64_t NImms = ~static_cast(Size-1) << 1;En lignende situation: V629 [CWE-190] Overvej at inspicere udtrykket ‘Immr << 6’. Bitforskydning af 32-bit værdien med efterfølgende udvidelse til 64-bit type. AArch64AddressingModes.h 269
Fragment N19: Manglende nøgleord ellers?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp bruger "vcc" token.
// Spring det over.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Håndter valgfrie argumenter
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Ugyldig operandtype");
}
....
}Advarsel fra PVS-Studio: [CWE-670] Overvej at inspicere applikationens logik. Det er muligt, at 'else'-nøgleordet mangler. AMDGPUAsmParser.cpp 5655
Der er ingen fejl her, da then-blokken for den første if slutter med continue, så det er ikke vigtigt, om der er et nøgleord ellers eller ej. Under alle omstændigheder vil koden fungere ens. Ikke desto mindre ellers gør det koden mere uklar og farlig. Hvis den efterfølgende continue forsvinder, vil koden begynde at fungere helt anderledes. Efter min mening er det bedre at tilføje ellers.
Afsnit N20: Fire ens stavefejl
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;
}Advarsler fra PVS-Studio:
- V655 [CWE-480] Strengene blev CONCATENATED, men bruges ikke. Overvej at inspicere udtrykket 'Result + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] Strengene blev CONCATENATED, men bruges ikke. Overvej at inspicere udtrykket 'Result + "(ObjC Class) " + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Strengene blev CONCATENATED, men bruges ikke. Overvej at inspicere udtrykket 'Result + "(ObjC Class EH) " + Name.str()'. Symbol.cpp 38
- V655 [CWE-480] Strengene blev CONCATENATED, men bruges ikke. Overvej at inspicere udtrykket 'Result + "(ObjC IVar) " + Name.str()'. Symbol.cpp 41
Tilfældigvis bruges operatoren + i stedet for operatoren +=. Som et resultat får vi konstruktioner, der er meningsløse.
Afsnit N21: Ubestemt opførsel
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 kan ikke være tom");
for (auto &Op : Ops) {
assert(!Op.empty() && "Tom operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Forsøg selv at finde farlig kode. Her er et billede til at distrahere dig, så du ikke ser svaret med det samme:

Advarsel fra PVS-Studio: [CWE-758] Farlig konstruktion anvendes: 'FeaturesMap[Op] = FeaturesMap.size()', hvor 'FeaturesMap' er af 'map' klasse. Dette kan føre til ubestemt opførsel. RISCVCompressInstEmitter.cpp 490
Problemlinjen:
FeaturesMap[Op] = FeaturesMap.size();Hvis elementet Op ikke findes, oprettes et nyt element i kortet, og antallet af elementer i dette kort skrives dertil. Men det er uvist, om funktionen størrelse vil blive kaldt før eller efter tilføjelsen af det nye element.
Afsnit N22-N24: Gentagne tildelinger
Fejl 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;
}
....
}Advarsel fra PVS-Studio: [CWE-563] Variablen ‘NType’ tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 1663, 1664. MachOObjectFile.cpp 1664
Jeg tror ikke, der er en rigtig fejl her. Bare unødvendig gentagen tildeling. Men stadig en fejl.
Tilsvarende:
- V519 [CWE-563] Variablen ‘B.NDesc’ tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Variablen tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Flere gentagne tildelinger
Lad os nu se på en lidt anden mulighed for gentagen tildeling.
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;
....
}Advarsel PVS-Studio: V519 [CWE-563] Variablen ‘Alignment’ tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 1158, 1160. LoadStoreVectorizer.cpp 1160
Dette er en meget underlig kode, som tilsyneladende indeholder en logisk fejl. I begyndelsen, tildeles variablen Alignment en værdi afhængigt af en betingelse. Og så sker der igen en tildeling, men nu uden nogen kontrol.
Lignende situationer kan ses her:
- V519 [CWE-563] Variablen ‘Effects’ tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Variablen ‘ExpectNoDerefChunk’ tildeles værdier to gange efter hinanden. Måske er det en fejl. Tjek linjer: 4970, 4973. SemaType.cpp 4973
Fragment N28: Altid sand betingelse
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-instruktionssupport // <=
break;
}
....
}Advarsel fra PVS-Studio: [CWE-571] Udtryk ‘nextByte != 0x90’ er altid sandt. X86DisassemblerDecoder.cpp 379
Kontrollen giver ikke mening. Variablen nextByte er altid ikke lig med værdien 0x90, hvilket følger af den forrige kontrol. Dette er en slags logisk fejl.
Fragment N29 — N…: Altid sande/falske betingelser
Analysatoren rapporterer mange advarsler om, at hele betingelsen () eller dens del () altid sandt eller falsk. Ofte er dette ikke rigtige fejl, men blot uordentlig kode, resultatet af makroudvidelser og lignende. Der er dog grund til at se nærmere på alle disse advarsler, da der fra tid til anden optræder reelle logiske fejl. For eksempel er denne del af koden mistænkelig:
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;
....
}Advarsel fra PVS-Studio: [CWE-570] En del af betingelsen er altid falsk: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanten 0xE er værdien 14 i decimal. Tjekken RegNo == 0xe giver ikke mening, da hvis RegNo > 13, så vil funktionen afslutte sin udførelse.
Der har været mange andre advarsler med ID V547 og V560, men ligesom med , var jeg ikke interesseret i at studere disse advarsler. Det var tydeligt, at jeg havde nok materiale til at skrive en artikel :). Derfor er det uvist, hvor mange fejl af denne type der kan identificeres i LLVM ved hjælp af PVS-Studio.
Her er et eksempel på, hvorfor det er kedeligt at studere disse udløsninger. Analysatoren har ganske ret ved at komme med en advarsel for følgende kode. Men det er også ikke en fejl.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}PVS-Studio advarsel: V547 [CWE-570] Udtrykket ‘!HasError’ er altid falsk. UnwrappedLineParser.cpp 1635
Fragment N30: Mistænkelig returnering
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();
}
....
}Advarsel fra PVS-Studio: [CWE-670] En ubetinget ‘return’ inden for en løkke. R600OptimizeVectorRegisters.cpp 63
Dette er enten en fejl eller en speciel teknik, der er beregnet til at forklare noget for programmører, der læser koden. En sådan konstruktion forklarer mig intet og ser meget mistænkelig ud. Det er bedre ikke at skrive sådan :).
Træt? Så er det tid til at brygge en kop te eller kaffe.

Defekter afsløret af nye diagnostik
Jeg mener, 30 udløsninger fra gamle diagnoser er nok. Lad os nu se, hvad interessant vi kan finde med de nye diagnoser, der er blevet tilføjet til analysatoren efter tjekningen. I alt er der i denne periode blevet tilføjet 66 generelle diagnostikker til C++ analysatoren.
Fragment N31: Uopnåelig kode
Fejl CtorDtorRunner::run() {
....
hvis (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}Advarsel fra PVS-Studio: [CWE-561] Uopnåelig kode opdaget. Det er muligt, at der er en fejl til stede. ExecutionUtils.cpp 146
Som du kan se, afsluttes begge grene af operatoren if med et kald til operatoren returner. Derfor vil containeren CtorDtorsByPriority aldrig blive ryddet.
Fragment N32: Uopnåelig kode
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(), "uventet sammendragstype");
}
Lex.setIgnoreColonInIdentifiers(false);
return false;
}Advarsel PVS-Studio: V779 [CWE-561] Uopnåelig kode opdaget. Det er muligt, at der er en fejl til stede. LLParser.cpp 835
En interessant situation. Lad os først se på dette sted:
return ParseTypeIdEntry(SummaryID);
break;Ved første øjekast ser det ud til, at der ikke er nogen fejl her. Det ser ud til, at operatoren break her er overflødig, og den kan blot fjernes. Men det er ikke så simpelt.
Analyseren giver en advarsel på linjerne:
Lex.setIgnoreColonInIdentifiers(false);
return false;Og ja, denne kode er uopnåelig. Alle tilfælde i switch sluttes med at kalde operatoren returner. Og nu ser den meningsløse ensomme break ikke så harmløs ud! Måske skulle en af grene afsluttes på break, og ikke på returner?
Fragment N33: Tilfældig nulstilling af de øverste bits
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}Advarsel fra PVS-Studio: Størrelsen af bitmasken er mindre end størrelsen af den første operand. Dette vil forårsage tab af højere bits. RuntimeDyld.cpp 815
Bemærk, at funktionen getStubAlignment returnerer typen unsigned. Lad os beregne værdien af udtrykket, hvis vi antager, at funktionen returnerer værdien 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFF8u
Nu skal du bemærke, at variablen DataSize har en 64-bit unsigned type. Det betyder, at når operationen DataSize & 0xFFFFFFF8u udføres, vil alle treogtyve øverste bits blive nulstillet. Det er sandsynligvis ikke, hvad programmøren ønskede. Jeg formoder, at han ville beregne: DataSize & 0xFFFFFFFFFFFFFFF8u.
For at rette fejlen skal du skrive sådan her:
DataSize &= ~(static_cast(getStubAlignment()) - 1);Eller sådan her:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Mislykket eksplicit typecast
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Uventet skaleringsfaktor");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}Advarsel fra PVS-Studio: [CWE-190] Mulighed for overflow. Overvej at caste operanderne af ‘NumElts * Scale’ operatoren til ‘size_t’ typen, ikke resultatet. X86ISelLowering.h 1577
Eksplisit typecasting anvendes for at undgå overflødighed ved multiplikation af variabler af typen int. Men her beskytter eksplisit typecasting ikke mod overflødighed. Først vil variablerne blive multipliceret, og først derefter vil det 32-bit resultat blive udvidet til typen .
Fragment N35: Mislykket Copy-Paste
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] To lignende kodefragmenter blev fundet. Måske er dette en tastefejl, og ‘Op1’ variablen skulle anvendes i stedet for ‘Op0’. InstCombineCompares.cpp 5507
Denne nye interessante diagnose identificerer situationer, hvor et kodefragment er blevet kopieret, og man er begyndt at ændre nogle navne, men ét sted har man ikke rettet det.
Bemærk, at i det andet blok ændrede man Op0 og få en fungerende kommando. Op1. Men ét sted har man ikke rettet det. Det skulle sandsynligvis have været skrevet således:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Forvirring i variabler
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Advarsel fra PVS-Studio: [CWE-563] Variablen ‘Mode’ er tildelt, men er ikke brugt ved slutningen af funktionen. SIModeRegister.cpp 48
Det er meget farligt at give funktionsargumenter de samme navne som klassemedlemmer. Det er meget nemt at blive forvirret. Dette er netop sådan et tilfælde. Dette udtryk giver ikke mening:
Mode &= Mask;Argumentet for funktionen ændres. Og det er det. Dette argument anvendes ikke længere. Det skulle sandsynligvis have været skrevet således:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Forvirring i variabler
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;
}Advarsel PVS-Studio: V1001 [CWE-563] Variablen ‘Size’ bliver tildelt, men anvendes ikke ved slutningen af funktionen. Object.cpp 424
Situationen er den samme som før. Det skal skrives:
this->Size += this->EntrySize;Udsnit N38-N47: Pointeren blev glemt at blive tjekket
Tidligere har vi set eksempler på udløb af diagnosticeringen . Dens essens er, at pointeren i starten derefereres, og først derefter tjekkes. Den unge diagnosticering er modsatrettet i sin betydning, men opdager også mange fejl. Den identificerer situationer, hvor pointeren først blev tjekket, og derefter blev det glemt. Lad os se på sådanne tilfælde fundet inden for 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()); // <=
....
}Advarsel PVS-Studio: V1004 [CWE-476] Pointeren ‘Ptr’ blev brugt usikkert efter den blev verificeret mod nullptr. Tjek linjer: 729, 738. TargetTransformInfoImpl.h 738
Variabel Ptr kan være lig med nullptr, hvilket bevises af tjekket:
if (Ptr != nullptr)Men nedenfor derefereres denne pointer allerede uden forudgående tjek:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Lad os se på en anden lignende situation.
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(); // <=
....
}Advarsel PVS-Studio: V1004 [CWE-476] Pointeren ‘FD’ blev brugt usikkert efter den blev verificeret mod nullptr. Tjek linjer: 3228, 3231. CGDebugInfo.cpp 3231
Bemærk pointeren FD. Jeg er sikker på, at problemet er tydeligt, og der kræves ikke specielle forklaringer.
Og endnu:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast(Ptr.getType());
if (!PtrTy) { // getPointerAddressSpace()); // <=
....
}Advarsel fra PVS-Studio: V1004 [CWE-476] Pointeren 'PtrTy' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 960, 965. InterleavedLoadCombinePass.cpp 965
Hvordan beskytter man sig mod sådanne fejl? Vær mere opmærksom ved Code-Review, og brug den statiske analysetjeneste PVS-Studio til løbende kodekontrol.
Det giver ikke mening at medtage andre kodefragmenter med sådanne fejl. Jeg efterlader kun listen over advarsler i artiklen:
- V1004 [CWE-476] Pointeren 'Expr' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Pointeren 'PI' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Pointeren 'StatepointCall' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Pointeren 'RV' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Pointeren 'CalleeFn' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Pointeren 'TC' blev brugt usikkert efter at den blev verificeret mod nullptr. Tjek linjer: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Ikke kritisk, men en fejl (mulig hukommelseslækage)
std::unique_ptr createISelMutator() {
....
std::vector<std::unique_ptr> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Advarsel fra PVS-Studio: [CWE-460] En pointer uden ejer tilføjes til 'Strategies'-containeren via 'emplace_back'-metoden. En hukommelseslækage vil opstå ved en undtagelse. llvm-isel-fuzzer.cpp 58
For at tilføje et element til slutningen af en container af typen std::vector<std::unique_ptr> kan man ikke blot skrive xxx.push_back(new X), da der ikke er nogen implicit konvertering fra X* i std::unique_ptr.
En almindelig løsning er at skrive xxx.emplace_back(new X), da den kompilere: metoden emplace_back konstruerer elementet direkte fra argumenterne og kan derfor bruge tydelige konstruktører.
Dette er ikke sikkert. Hvis vektoren er fuld, vil der ske en genallokering af hukommelse. Genallokeringsoperationen kan mislykkes, hvilket kan føre til, at en undtagelse genereres std::bad_alloc. I dette tilfælde vil pointeren blive tabt, og det oprettede objekt vil aldrig blive slettet.
En sikker løsning er at oprette en unique_ptr, som ejer pointeren, indtil vektoren forsøger at genallokere hukommelse:
xxx.push_back(std::unique_ptr(new X))Fra C++14 kan man bruge 'std::make_unique':
xxx.push_back(std::make_unique())Denne type fejl er ikke kritisk for LLVM. Hvis det ikke er muligt at allokere hukommelse, vil compilerens arbejde blot stoppe. Men for applikationer med lang , som ikke bare kan afsluttes, hvis hukommelsen ikke kan allokeres, kan dette være en rigtig uheldig fejl.
Så selvom denne kode ikke udgør nogen praktisk fare for LLVM, fandt jeg det nyttigt at tale om dette fejlmønster og hvordan PVS-Studio-analyzatoren har lært at identificere det.
Andre advarsler af denne type:
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Passes'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. PassManager.h 546
- V1023 [CWE-460] En peger uden ejer tilføjes til 'AAs'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. AliasAnalysis.h 324
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Entries'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] En peger uden ejer tilføjes til 'AllEdges'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. CFGMST.h 268
- V1023 [CWE-460] En peger uden ejer tilføjes til 'VMaps'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Records'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. FDRLogBuilder.h 30
- V1023 [CWE-460] En peger uden ejer tilføjes til 'PendingSubmodules'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. ModuleMap.cpp 810
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Objects'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. DebugMap.cpp 88
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Strategies'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 685
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 686
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 688
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 689
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 690
- V1023 [CWE-460] En peger uden ejer tilføjes til 'Modifiers'-containeren af 'emplace_back'-metoden. Der vil opstå et hukommelseslækage i tilfælde af en undtagelse. llvm-stress.cpp 691
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Modifiers'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 692
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Modifiers'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 693
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Modifiers'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 694
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Operands'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Stash'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] En pointer uden ejer bliver tilføjet til 'Matchers'-containeren af metoden 'emplace_back'. En hukommelseslækage vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 2702
Konklusion
I alt har jeg noteret 60 advarsler, hvorefter jeg stoppede. Er der andre defekter, som PVS-Studio analysatoren opdager i LLVM? Ja, det er der. Men da jeg noterede kodefragmenter til artiklen, var det blevet sen aften, nærmere bestemt nat, og jeg besluttede, at det var tid til at slutte.
Jeg håber, det har været interessant for dig, og at du ønsker at prøve PVS-Studio analysatoren.
Du kan downloade analysatoren og få en prøve-nøgle på .
Det vigtigste er, at du bruger statisk analyse regelmæssigt. Enkeltstående kontroller, udført af os med henblik på at popularisere metodologien for statisk analyse og PVS-Studio, er ikke et normalt scenarie.
Held og lykke med at forbedre kvaliteten og pålideligheden af koden!
Hvis du ønsker at dele denne artikel med et engelsktalende publikum, skal du bruge linket til oversættelsen: Andrey Karpov. .
Kilde: habr.com
