Der er gået mere end to år siden sidste kodetjek af LLVM-projektet ved hjælp af vores PVS-Studio-analysator. Lad os sikre os, at PVS-Studio-analysatoren stadig er et førende værktøj til at identificere fejl og potentielle sårbarheder. For at gøre dette vil vi kontrollere og finde nye fejl i LLVM 8.0.0-udgivelsen.
Artikel, der skal skrives
For at være ærlig havde jeg ikke lyst til at skrive denne artikel. Det er ikke interessant at skrive om et projekt, som vi allerede har tjekket flere gange (, , ). Det er bedre at skrive om noget nyt, men jeg har ikke noget valg.
Hver gang en ny version af LLVM frigives eller opdateres , modtager vi spørgsmål af følgende type i vores mail:
Se, den nye version af Clang Static Analyzer har lært at finde nye fejl! Det forekommer mig, at relevansen af at bruge PVS-Studio er faldende. Clang finder flere fejl end før og indhenter mulighederne i PVS-Studio. Hvad tænker du om det her?
Til dette vil jeg altid svare noget som:
Vi sidder heller ikke ledige! Vi har væsentligt forbedret PVS-Studio-analysatorens muligheder. Så bare rolig, vi fortsætter med at lede som før.
Desværre er dette et dårligt svar. Der er ingen beviser i det. Og det er derfor, jeg skriver denne artikel nu. Så LLVM-projektet er igen blevet kontrolleret, og der er fundet en række fejl i det. Jeg vil nu demonstrere dem, der forekom mig interessante. Clang Static Analyzer kan ikke finde disse fejl (eller det er ekstremt ubelejligt at gøre det med dens hjælp). Men det kan vi. Desuden fandt og skrev jeg alle disse fejl ned på en aften.
Men at skrive artiklen tog flere uger. Jeg kunne bare ikke få mig selv til at sætte alt dette ind i tekst :).
Forresten, hvis du er interesseret i, hvilke teknologier der bruges i PVS-Studio-analysatoren til at identificere fejl og potentielle sårbarheder, så foreslår jeg, at du gør dig bekendt med dette .
Ny og gammel diagnostik
Som allerede nævnt blev LLVM-projektet for omkring to år siden igen kontrolleret, og de fundne fejl blev rettet. Nu vil denne artikel præsentere en ny batch af fejl. Hvorfor blev der fundet nye fejl? Det er der 3 grunde til:
- LLVM-projektet udvikler sig, ændrer gammel kode og tilføjer ny kode. Naturligvis er der nye fejl i den ændrede og skrevne kode. Dette viser tydeligt, at statisk analyse bør bruges regelmæssigt og ikke lejlighedsvis. Vores artikler viser godt PVS-Studio-analysatorens muligheder, men dette har intet at gøre med at forbedre kodekvaliteten og reducere omkostningerne ved at rette fejl. Brug en statisk kodeanalysator regelmæssigt!
- Vi færdiggør og forbedrer eksisterende diagnostik. Derfor kan analysatoren identificere fejl, som den ikke har bemærket under tidligere scanninger.
- Ny diagnostik er dukket op i PVS-Studio, som ikke fandtes for 2 år siden. Jeg besluttede at fremhæve dem i et separat afsnit for tydeligt at vise udviklingen af PVS-Studio.
Defekter identificeret ved diagnostik, der eksisterede for 2 år siden
Fragment N1: Copy-Paste
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}PVS-Studio advarsel: [CWE-570] Der er identiske underudtryk 'Name.startswith("avx512.mask.permvar.")' til venstre og til højre for '||' operatør. AutoUpgrade.cpp 73
Det dobbelttjekkes, at navnet begynder med understrengen "avx512.mask.permvar.". I den anden kontrol ville de åbenbart skrive noget andet, men glemte at rette den kopierede tekst.
Fragment N2: Trykfejl
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 PVS-Studio: V501 Der er identiske underudtryk 'CXNameRange_WantQualifier' til venstre og til højre for '|' operatør. CIndex.cpp 7245
På grund af en tastefejl bruges den samme navngivne konstant to gange CXNameRange_WantQualifier.
Fragment N3: Forvirring med operatørprioritet
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}PVS-Studio advarsel: [CWE-783] Måske fungerer '?:'-operatøren på en anden måde, end det var forventet. Operatoren '?:' har en lavere prioritet end operatoren '=='. PPCTargetTransformInfo.cpp 404
Efter min mening er dette en meget smuk fejltagelse. Ja, jeg ved godt, at jeg har mærkelige ideer om skønhed :).
Nu ifølge , er udtrykket vurderet som følger:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Fra et praktisk synspunkt giver en sådan betingelse ikke mening, da den kan reduceres til:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Dette er en klar fejl. Mest sandsynligt ønskede de at sammenligne 0/1 med en variabel Indeks. For at rette koden skal du tilføje parenteser omkring den ternære operator:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Forresten er den ternære operatør meget farlig og fremkalder logiske fejl. Vær meget forsigtig med det og vær ikke grådig med parenteser. Jeg så på dette emne mere detaljeret , i kapitlet "Pas på ?: Operatør og sæt den i parentes."
Fragment N4, N5: Nul pointer
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;
}
....
}PVS-Studio advarsel: [CWE-476] Dereference af nul-markøren 'LHS' kan finde sted. TGParser.cpp 2152
Hvis markøren LHS er nul, bør der gives en advarsel. Men i stedet vil denne samme nul-pointer blive derefereret: LHS->getAsString().
Dette er en meget typisk situation, når en fejl er skjult i en fejlbehandler, da ingen tester dem. Statiske analysatorer kontrollerer al tilgængelig kode, uanset hvor ofte den bruges. Dette er et meget godt eksempel på, hvordan statisk analyse supplerer andre test- og fejlbeskyttelsesteknikker.
Lignende pointerhåndteringsfejl RHS tilladt i koden lige nedenfor: V522 [CWE-476] Dereference af nul-markøren 'RHS' kan finde sted. TGParser.cpp 2186
Fragment N6: Brug af markøren efter flytning
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);
}
....
}PVS-Studio Advarsel: V522 [CWE-476] Dereference af nul-markøren 'ProgClone' kan finde sted. Miscompilation.cpp 601
I begyndelsen en smart pointer ProgClone ophører med at eje objektet:
BD.setNewProgram(std::move(ProgClone));Faktisk nu ProgClone er en nul pointer. Derfor bør en nul pointer-dereference forekomme lige nedenfor:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Men i virkeligheden vil dette ikke ske! Bemærk, at løkken faktisk ikke udføres.
I begyndelsen af beholderen Fejlkompilerede funktioner ryddet:
MiscompiledFunctions.clear();Dernæst bruges størrelsen af denne beholder i sløjfetilstanden:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Det er nemt at se, at løkken ikke starter. Jeg tror, at dette også er en fejl, og koden bør skrives anderledes.
Det ser ud til, at vi har stødt på den berømte paritet af fejl! En fejl skjuler den anden :).
Fragment N7: Brug af markøren efter flytning
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;
}
....
}PVS-Studio-advarsel: V522 [CWE-476] Der kan forekomme frareference af nul-markøren 'Test'. Miscompilation.cpp 709
Samme situation igen. Først flyttes objektets indhold, og derefter bruges det, som om intet var hændt. Jeg ser denne situation oftere og oftere i programkode efter bevægelsessemantik dukkede op i C++. Det er derfor, jeg elsker C++ sproget! Der er flere og flere nye måder at skyde sit eget ben af. PVS-Studio analysatoren vil altid have arbejde :).
Fragment N8: Nul pointer
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);
}PVS-Studio-advarsel: V522 [CWE-476] Dereference af nul-markøren 'Type' kan finde sted. PrettyFunctionDumper.cpp 233
Ud over fejlbehandlere bliver fejlfindingsudskriftsfunktioner normalt ikke testet. Vi har netop sådan en sag foran os. Funktionen venter på brugeren, som i stedet for at løse sine problemer vil blive tvunget til at ordne det.
korrigere:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Nul pointer
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());
....
}PVS-Studio advarsel: V522 [CWE-476] Dereference af nul-markøren 'Ty' kan finde sted. SearchableTableEmitter.cpp 614
Jeg tror, at alt er klart og ikke kræver forklaring.
Fragment N10: Trykfejl
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;
}PVS-Studio advarsel: Variablen 'Identifier->Type' er tildelt sig selv. FormatTokenLexer.cpp 249
Det nytter ikke at tildele en variabel til sig selv. Mest sandsynligt ville de skrive:
Identifier->Type = Question->Type;Fragment N11: Mistænkeligt brud
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}PVS-Studio advarsel: [CWE-478] Overvej at inspicere "switch"-erklæringen. Det er muligt, at den første "case"-operatør mangler. SystemZAsmParser.cpp 652
Der er en meget mistænkelig operatør i begyndelsen bryde. Har du glemt at skrive noget andet her?
Fragment N12: Kontrol af en pointer efter dereference
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");
....
}PVS-Studio advarsel: [CWE-476] 'Callee'-markøren blev brugt, før den blev verificeret mod nullptr. Tjek linjer: 172, 174. AMDGPUInline.cpp 172
Markør Callee i begyndelsen dereferences på det tidspunkt, hvor funktionen kaldes getTTI.
Og så viser det sig, at denne pointer skal tjekkes for ligestilling nullptr:
if (!Callee || Callee->isDeclaration())Men det er for sent...
Fragment N13 - N...: Kontrol af en pointer efter dereference
Situationen diskuteret i det foregående kodefragment er ikke unik. Det vises her:
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()) { // <=
....
}PVS-Studio advarsel: V595 [CWE-476] 'CalleeFn'-markøren blev brugt, før den blev verificeret 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<NamedDecl>(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <=
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember()); // <=
....
}PVS-Studio advarsel: V595 [CWE-476] 'ND'-markøren blev brugt, før den blev verificeret mod nullptr. Tjek linjer: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Og her:
- V595 [CWE-476] 'U'-markøren blev brugt, før den blev verificeret mod nullptr. Tjek linjer: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND'-markøren blev brugt, før den blev verificeret mod nullptr. Tjek linjer: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Og så blev jeg uinteresseret i at studere advarslerne med nummer V595. Så jeg ved ikke, om der er flere lignende fejl end dem, der er anført her. Det er der højst sandsynligt.
Fragment N17, N18: Mistænkeligt skift
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}PVS-Studio advarsel: [CWE-190] Overvej at inspicere udtrykket '~(Størrelse - 1) << 1'. Bitforskydning af 32-bit-værdien med en efterfølgende udvidelse til 64-bit-typen. AArch64AddressingModes.h 260
Det er muligvis ikke en fejl, og koden fungerer nøjagtigt efter hensigten. Men dette er helt klart et meget mistænkeligt sted og skal tjekkes.
Lad os sige variablen Størrelse er lig med 16, og så planlagde forfatteren af koden at få den i en variabel NImms betyder:
1111111111111111111111111111111111111111111111111111111111100000
Men i virkeligheden bliver resultatet:
0000000000000000000000000000000011111111111111111111111111100000
Faktum er, at alle beregninger sker ved hjælp af 32-bit usigneret type. Og først derefter vil denne 32-bit usignerede type implicit blive udvidet til uint64_t. I dette tilfælde vil de mest signifikante bits være nul.
Du kan rette situationen sådan her:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Lignende situation: V629 [CWE-190] Overvej at inspicere udtrykket 'Immr << 6'. Bitforskydning af 32-bit-værdien med en efterfølgende udvidelse til 64-bit-typen. AArch64AddressingModes.h 269
Fragment N19: Manglende nøgleord andet?
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");
}
....
}PVS-Studio advarsel: [CWE-670] Overvej at inspicere applikationens logik. Det er muligt, at søgeordet "andet" mangler. AMDGPUAsmParser.cpp 5655
Der er ingen fejl her. Siden den daværende blok af den første if slutter med fortsæt, så er det lige meget, der er et nøgleord andet eller ikke. Uanset hvad vil koden fungere på samme måde. Stadig savnet andet gør koden mere uklar og farlig. Hvis i fremtiden fortsæt forsvinder, vil koden begynde at fungere helt anderledes. Efter min mening er det bedre at tilføje andet.
Fragment N20: Fire tastefejl af samme type
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;
}PVS-Studio advarsler:
- V655 [CWE-480] Strengene blev sammenkædet, men bruges ikke. Overvej at inspicere udtrykket 'Resultat + Navn.str()'. Symbol.cpp 32
- V655 [CWE-480] Strengene blev sammenkædet, men bruges ikke. Overvej at inspicere udtrykket 'Result + "(ObjC Class)" + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Strengene blev sammenkædet, men bruges ikke. Overvej at inspicere udtrykket 'Result + "(ObjC Class EH) " + Name.str()'. Symbol.cpp 38
- V655 [CWE-480] Strengene blev sammenkædet, men bruges ikke. Overvej at inspicere udtrykket 'Resultat + "(ObjC IVar)" + Navn.str()'. Symbol.cpp 41
Ved et uheld bruges + operatoren i stedet for += operatoren. Resultatet er designs, der er blottet for mening.
Fragment N21: Udefineret adfærd
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();
}
}
}Prøv selv at finde den farlige kode. Og dette er et billede for at distrahere opmærksomheden for ikke straks at se på svaret:
PVS-Studio advarsel: [CWE-758] Der bruges farlig konstruktion: 'FeaturesMap[Op] = FeaturesMap.size()', hvor 'FeaturesMap' er af 'map'-klassen. Dette kan føre til udefineret adfærd. RISCVCompressInstEmitter.cpp 490
Problemlinje:
FeaturesMap[Op] = FeaturesMap.size();Hvis element Op ikke findes, så oprettes et nyt element i kortet, og antallet af elementer i dette kort skrives der. Det er bare uvist, om funktionen bliver kaldt størrelse før eller efter tilføjelse af et nyt element.
Fragment N22-N24: Gentagne opgaver
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;
}
....
}PVS-Studio advarsel: [CWE-563] Variablen 'NType' tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 1663, 1664. MachOObjectFile.cpp 1664
Jeg tror ikke, der er en reel fejl her. Bare en unødvendig gentaget opgave. Men stadig en bommert.
Ligeledes:
- V519 [CWE-563] Variablen 'B.NDesc' tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Variablen tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Flere omplaceringer
Lad os nu se på en lidt anden version af omplacering.
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;
....
}PVS-Studio advarsel: V519 [CWE-563] Variablen 'Alignment' tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 1158, 1160. LoadStoreVectorizer.cpp 1160
Dette er meget mærkelig kode, der tilsyneladende indeholder en logisk fejl. I begyndelsen, variabel Justering en værdi tildeles afhængigt af betingelsen. Og så opstår opgaven igen, men nu uden kontrol.
Lignende situationer kan ses her:
- V519 [CWE-563] Variablen 'Effekter' tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Variablen 'ExpectNoDerefChunk' tildeles værdier to gange i træk. Måske er dette en fejl. Tjek linjer: 4970, 4973. SemaType.cpp 4973
Fragment N28: Altid sand tilstand
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;
}
....
}PVS-Studio advarsel: [CWE-571] Udtrykket 'nextByte != 0x90' er altid sandt. X86DisassemblerDecoder.cpp 379
At tjekke giver ikke mening. Variabel næste Byte altid ikke lig med værdien 0x90, som følger af den foregående kontrol. Dette er en form for logisk fejl.
Fragment N29 - N...: Altid sande/falske forhold
Analysatoren udsender mange advarsler om, at hele tilstanden () eller en del heraf () er altid sandt eller falsk. Ofte er der ikke tale om rigtige fejl, men blot sjusket kode, resultatet af makroudvidelse og lignende. Det giver dog mening at se på alle disse advarsler, da der fra tid til anden opstår ægte logiske fejl. For eksempel er denne sektion af kode 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;
....
}PVS-Studio advarsel: [CWE-570] En del af betinget udtryk er altid falsk: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanten 0xE er værdien 14 i decimal. Undersøgelse RegNo == 0xe giver ikke mening, fordi hvis RegNo > 13, så vil funktionen fuldføre sin udførelse.
Der var mange andre advarsler med ID'er V547 og V560, men som med , jeg var ikke interesseret i at studere disse advarsler. Det var allerede klart, 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.
Jeg vil give dig et eksempel på, hvorfor det er kedeligt at studere disse triggere. Analysatoren har helt ret i at udstede en advarsel for følgende kode. Men dette er heller 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 tilbagevenden
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();
}
....
}PVS-Studio advarsel: [CWE-670] En ubetinget 'retur' inden for en løkke. R600OptimizeVectorRegisters.cpp 63
Dette er enten en fejl eller en specifik teknik, der er beregnet til at forklare noget for programmører, der læser koden. Dette design forklarer mig ikke noget og ser meget mistænkeligt ud. Det er bedre ikke at skrive sådan :).
Træt? Så er det tid til at lave te eller kaffe.
Defekter identificeret ved ny diagnostik
Jeg tror, at 30 aktiveringer af gammel diagnostik er nok. Lad os nu se, hvilke interessante ting der kan findes med den nye diagnostik, der dukkede op i analysatoren efter checks. I løbet af denne tid blev i alt 66 generel diagnostik tilføjet til C++-analysatoren.
Fragment N31: Uopnåelig kode
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();
}PVS-Studio advarsel: [CWE-561] Uopnåelig kode fundet. Det er muligt, at der er en fejl. ExecutionUtils.cpp 146
Som du kan se, begge grene af operatøren if afsluttes med et opkald til operatøren afkast. I overensstemmelse hermed er beholderen CtorDtorsByPriority vil 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(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}PVS-Studio advarsel: V779 [CWE-561] Uopnåelig kode fundet. Det er muligt, at der er en fejl. LLParser.cpp 835
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 ligner operatøren bryde der er en ekstra her, og du kan simpelthen slette den. Dog ikke alt så enkelt.
Analysatoren udsender en advarsel på linjerne:
Lex.setIgnoreColonInIdentifiers(false);
return false;Og denne kode er faktisk ikke tilgængelig. Alle sager i skifte afsluttes med et opkald fra operatøren afkast. Og nu meningsløs alene bryde ser ikke så harmløst ud! Måske en af grenene skal slutte med brydemen ikke på afkast?
Fragment N33: Tilfældig nulstilling af høje bits
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);
....
}PVS-Studio advarsel: Størrelsen af bitmasken er mindre end størrelsen af den første operand. Dette vil medføre tab af højere bits. RuntimeDyld.cpp 815
Bemærk venligst, at funktionen getStubAlignment returtype usigneret. Lad os beregne værdien af udtrykket, idet vi antager, at funktionen returnerer værdien 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Bemærk nu, at variablen Datastørrelse har en 64-bit usigneret type. Det viser sig, at når du udfører DataSize & 0xFFFFFFF8u-operationen, nulstilles alle toogtredive bits af høj orden til nul. Det er højst sandsynligt ikke, hvad programmøren ønskede. Jeg formoder, at han ville beregne: DataSize & 0xFFFFFFFFFFFFFFF8u.
For at rette fejlen skal du skrive dette:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Eller så:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Mislykket eksplicit type cast
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);
....
}PVS-Studio advarsel: [CWE-190] Muligt overløb. Overvej at caste operander af operatoren 'NumElts * Scale' til typen 'size_t', ikke resultatet. X86ISelLowering.h 1577
Eksplicit type casting bruges til at undgå overløb ved multiplicering af typevariabler int. Eksplicit typestøbning her beskytter dog ikke mod overløb. Først vil variablerne blive ganget, og først derefter vil 32-bit resultatet af multiplikationen 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 skal bruges i stedet for 'Op0'. InstCombineCompares.cpp 5507
Denne nye interessante diagnostik identificerer situationer, hvor et stykke kode er blevet kopieret, og nogle navne i det er begyndt at blive ændret, men ét sted har de ikke rettet det.
Bemærk venligst, at de ændrede sig i den anden blok Op0 på Op1. Men ét sted fik de det ikke rettet. Det burde højst sandsynligt have været skrevet sådan her:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Variabel forvirring
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};PVS-Studio advarsel: [CWE-563] 'Mode'-variablen er tildelt, men bruges ikke 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. Vi har netop sådan en sag foran os. Dette udtryk giver ikke mening:
Mode &= Mask;Funktionsargumentet ændres. Det er alt. Dette argument bruges ikke længere. Mest sandsynligt skulle du have skrevet det sådan her:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Variabel forvirring
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;
}Advarsel PVS-Studio: V1001 [CWE-563] Variablen 'Størrelse' er tildelt, men bruges ikke ved slutningen af funktionen. Object.cpp 424
Situationen ligner den forrige. Det skal skrives:
this->Size += this->EntrySize;Fragment N38-N47: De glemte at tjekke indekset
Tidligere har vi set på eksempler på diagnostisk udløsning . Dens essens er, at markøren dereferences i begyndelsen, og først derefter kontrolleres. Ung diagnostik er det modsatte i betydningen, men afslører også en masse fejl. Den identificerer situationer, hvor markøren blev tjekket i begyndelsen og derefter glemt at gøre det. Lad os se på sådanne tilfælde fundet inde i 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()); // <=
....
}PVS-Studio advarsel: V1004 [CWE-476] 'Ptr'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 729, 738. TargetTransformInfoImpl.h 738
Variabel ptr kan være lige nullptr, som det fremgår af kontrollen:
if (Ptr != nullptr)Nedenfor er denne pointer dog derefereret uden foreløbig kontrol:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Lad os overveje en anden lignende sag.
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(); // <=
....
}PVS-Studio advarsel: V1004 [CWE-476] 'FD'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 3228, 3231. CGDebugInfo.cpp 3231
Vær opmærksom på skiltet FD. Jeg er sikker på, at problemet er tydeligt synligt, og der kræves ingen speciel forklaring.
Og videre:
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()); // <=
....
}PVS-Studio advarsel: V1004 [CWE-476] 'PtrTy'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 960, 965. InterleavedLoadCombinePass.cpp 965
Hvordan beskytter man sig selv mod sådanne fejl? Vær mere opmærksom på Code-Review og brug PVS-Studio statiske analysator til regelmæssigt at tjekke din kode.
Der er ingen mening i at citere andre kodefragmenter med fejl af denne type. Jeg vil kun efterlade en liste over advarsler i artiklen:
- V1004 [CWE-476] 'Expr'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] 'PI'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] 'StatepointCall'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] 'RV'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] 'TC'-markøren blev brugt usikkert, efter at den blev verificeret mod nullptr. Tjek linjer: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Ikke kritisk, men en defekt (mulig hukommelseslækage)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}PVS-Studio advarsel: [CWE-460] En pointer uden ejer føjes til 'Strategies'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-isel-fuzzer.cpp 58
For at tilføje et element til enden af en beholder som f.eks std::vektor > du kan ikke bare skrive xxx.push_back(nyt X), da der ikke er nogen implicit konvertering fra X* в std::unique_ptr.
En almindelig løsning er at skrive xxx.emplace_back(nyt X)da den kompilerer: metode emplace_back konstruerer et element direkte ud fra dets argumenter og kan derfor bruge eksplicitte konstruktører.
Det er ikke sikkert. Hvis vektoren er fuld, re-allokeres hukommelsen. Hukommelsesomfordelingsoperationen kan mislykkes, hvilket resulterer i, at en undtagelse bliver kastet std::bad_alloc. I dette tilfælde vil markøren gå tabt, og det oprettede objekt vil aldrig blive slettet.
En sikker løsning er at skabe unik_ptrsom vil eje markøren før vektoren forsøger at omallokere hukommelse:
xxx.push_back(std::unique_ptr<X>(new X))Siden C++14 kan du bruge 'std::make_unique':
xxx.push_back(std::make_unique<X>())Denne type defekt er ikke kritisk for LLVM. Hvis hukommelsen ikke kan allokeres, stopper compileren simpelthen. Dog til applikationer med lang , som ikke bare kan afsluttes, hvis hukommelsesallokering mislykkes, dette kan være en rigtig grim fejl.
Så selvom denne kode ikke udgør en praktisk trussel mod LLVM, fandt jeg det nyttigt at tale om dette fejlmønster, og at PVS-Studio-analysatoren har lært at identificere det.
Andre advarsler af denne type:
- V1023 [CWE-460] En pointer uden ejer føjes til 'Passes'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. PassManager.h 546
- V1023 [CWE-460] En pointer uden ejer føjes til 'AAs'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. AliasAnalysis.h 324
- V1023 [CWE-460] En pointer uden ejer føjes til 'Entries'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] En pointer uden ejer føjes til 'AllEdges'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. CFGMST.h 268
- V1023 [CWE-460] En pointer uden ejer føjes til 'VMaps'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. SimpleLoopUswitch.cpp 2012
- V1023 [CWE-460] En pointer uden ejer tilføjes til 'Records'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. FDRLogBuilder.h 30
- V1023 [CWE-460] En pointer uden ejer føjes til 'PendingSubmodules'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. ModuleMap.cpp 810
- V1023 [CWE-460] En pointer uden ejer føjes til 'Objekter'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. DebugMap.cpp 88
- V1023 [CWE-460] En pointer uden ejer føjes til 'Strategies'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 685
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 686
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 688
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 689
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 690
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 691
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 692
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 693
- V1023 [CWE-460] En pointer uden ejer føjes til 'Modifiers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. llvm-stress.cpp 694
- V1023 [CWE-460] En pointer uden ejer føjes til 'Operander'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] En pointer uden ejer føjes til 'Stash'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] En pointer uden ejer føjes til 'Matchers'-beholderen ved hjælp af 'emplace_back'-metoden. En hukommelseslæk vil opstå i tilfælde af en undtagelse. GlobalISelEmitter.cpp 2702
Konklusion
Jeg udstedte 60 advarsler i alt og stoppede så. Er der andre defekter, som PVS-Studio-analysatoren registrerer i LLVM? Ja jeg har. Men da jeg skrev kodefragmenter til artiklen, var det sent på aftenen, eller rettere sagt aften, og jeg besluttede, at det var på tide at kalde det en dag.
Jeg håber, du fandt det interessant og vil prøve PVS-Studio-analysatoren.
Du kan downloade analysatoren og få minestrygernøglen på .
Vigtigst af alt, brug statisk analyse regelmæssigt. Engangstjek, udført af os for at popularisere metoden til statisk analyse og PVS-Studio er ikke et normalt scenarie.
Held og lykke med at forbedre kvaliteten og pålideligheden af din kode!
Hvis du vil dele denne artikel med et engelsktalende publikum, så brug venligst oversættelseslinket: Andrey Karpov. .
Kilde: www.habr.com