Mer enn to år har gått siden siste kodesjekk av LLVM-prosjektet med vår PVS-Studio-analysator. La oss sørge for at PVS-Studio-analysatoren fortsatt er et ledende verktøy for å identifisere feil og potensielle sårbarheter. For å gjøre dette, vil vi sjekke og finne nye feil i LLVM 8.0.0-versjonen.
Artikkel som skal skrives
For å være ærlig, ville jeg ikke skrive denne artikkelen. Det er ikke interessant å skrive om et prosjekt som vi allerede har sjekket flere ganger (, , ). Det er bedre å skrive om noe nytt, men jeg har ikke noe valg.
Hver gang en ny versjon av LLVM utgis eller oppdateres , mottar vi spørsmål av følgende type i e-posten vår:
Se, den nye versjonen av Clang Static Analyzer har lært å finne nye feil! Det virker for meg som om relevansen av å bruke PVS-Studio synker. Clang finner flere feil enn før og fanger opp med mulighetene til PVS-Studio. Hva tenker du om dette?
Til dette vil jeg alltid svare noe som:
Vi sitter ikke stille heller! Vi har forbedret egenskapene til PVS-Studio-analysatoren betydelig. Så ikke bekymre deg, vi fortsetter å lede som før.
Dessverre er dette et dårlig svar. Det er ingen bevis i det. Og det er derfor jeg skriver denne artikkelen nå. Så LLVM-prosjektet har igjen blitt sjekket og det er funnet en rekke feil i det. Jeg vil nå demonstrere de som virket interessante for meg. Clang Static Analyzer kan ikke finne disse feilene (eller det er ekstremt upraktisk å gjøre det med dens hjelp). Men vi kan. Dessuten fant og skrev jeg ned alle disse feilene på en kveld.
Men å skrive artikkelen tok flere uker. Jeg klarte rett og slett ikke å sette alt dette inn i tekst :).
Forresten, hvis du er interessert i hvilke teknologier som brukes i PVS-Studio-analysatoren for å identifisere feil og potensielle sårbarheter, foreslår jeg at du gjør deg kjent med dette .
Ny og gammel diagnostikk
Som allerede nevnt, for omtrent to år siden ble LLVM-prosjektet igjen kontrollert, og feilene som ble funnet ble rettet. Nå vil denne artikkelen presentere en ny gruppe feil. Hvorfor ble det funnet nye feil? Det er 3 grunner til dette:
- LLVM-prosjektet utvikler seg, endrer gammel kode og legger til ny kode. Naturligvis er det nye feil i den modifiserte og skrevne koden. Dette viser tydelig at statisk analyse bør brukes regelmessig, og ikke av og til. Artiklene våre viser godt egenskapene til PVS-Studio-analysatoren, men dette har ingenting å gjøre med å forbedre kodekvaliteten og redusere kostnadene ved å fikse feil. Bruk en statisk kodeanalysator regelmessig!
- Vi sluttfører og forbedrer eksisterende diagnostikk. Derfor kan analysatoren identifisere feil som den ikke la merke til under tidligere skanninger.
- Ny diagnostikk har dukket opp i PVS-Studio som ikke eksisterte for 2 år siden. Jeg bestemte meg for å fremheve dem i en egen del for å tydelig vise utviklingen av PVS-Studio.
Defekter identifisert ved diagnostikk som eksisterte 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] Det er identiske underuttrykk 'Name.startswith("avx512.mask.permvar.")' til venstre og til høyre for '||' operatør. AutoUpgrade.cpp 73
Det er dobbeltsjekket at navnet begynner med understrengen "avx512.mask.permvar.". I den andre sjekken ville de tydeligvis skrive noe annet, men glemte å rette på den kopierte teksten.
Fragment N2: Skrivefeil
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 Det er identiske underuttrykk 'CXNameRange_WantQualifier' til venstre og høyre for '|' operatør. CIndex.cpp 7245
På grunn av en skrivefeil brukes den samme navngitte konstanten to ganger 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] Kanskje fungerer '?:'-operatøren på en annen måte enn det var forventet. Operatoren '?:' har lavere prioritet enn operatoren '=='. PPCTargetTransformInfo.cpp 404
Etter min mening er dette en veldig vakker feil. Ja, jeg vet at jeg har rare ideer om skjønnhet :).
Nå, ifølge , blir uttrykket evaluert som følger:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Fra et praktisk synspunkt gir en slik tilstand ikke mening, siden den kan reduseres til:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Dette er en klar feil. Mest sannsynlig ønsket de å sammenligne 0/1 med en variabel Index. For å fikse koden må du legge til parenteser rundt den ternære operatoren:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Forresten, den ternære operatøren er veldig farlig og provoserer logiske feil. Vær veldig forsiktig med det og ikke vær grådig med parenteser. Jeg så på dette emnet mer detaljert , i kapittelet "Vær deg for ?: Operatør og skriv den i parentes."
Fragment N4, N5: Nullpeker
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] Frareferanse av null-pekeren 'LHS' kan finne sted. TGParser.cpp 2152
Hvis pekeren LHS er null, bør det gis en advarsel. Imidlertid vil denne samme null-pekeren bli derferert: LHS->getAsString().
Dette er en veldig typisk situasjon når en feil er skjult i en feilbehandler, siden ingen tester dem. Statiske analysatorer sjekker all tilgjengelig kode, uansett hvor ofte den brukes. Dette er et veldig godt eksempel på hvordan statisk analyse utfyller andre test- og feilbeskyttelsesteknikker.
Lignende pekerhåndteringsfeil RHS tillatt i koden rett nedenfor: V522 [CWE-476] Det kan skje at null-pekeren 'RHS' fjernes. TGParser.cpp 2186
Fragment N6: Bruk av pekeren etter flytting
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] Frareferanse av null-pekeren 'ProgClone' kan finne sted. Miscompilation.cpp 601
I begynnelsen en smart peker ProgClone slutter å eie objektet:
BD.setNewProgram(std::move(ProgClone));Faktisk nå ProgClone er en null-peker. Derfor bør en null-peker-derreferanse forekomme like nedenfor:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Men i virkeligheten vil dette ikke skje! Legg merke til at løkken faktisk ikke blir utført.
I begynnelsen av beholderen Feilkompilerte funksjoner fjernet:
MiscompiledFunctions.clear();Deretter brukes størrelsen på denne beholderen i løkketilstanden:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Det er lett å se at loopen ikke starter. Jeg tror dette også er en feil og koden bør skrives annerledes.
Det ser ut til at vi har møtt den berømte pariteten av feil! En feil maskerer en annen :).
Fragment N7: Bruk av pekeren etter flytting
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] Frareferanse av null-pekeren 'Test' kan finne sted. Miscompilation.cpp 709
Samme situasjon igjen. Først flyttes innholdet i objektet, og deretter brukes det som om ingenting hadde skjedd. Jeg ser denne situasjonen oftere og oftere i programkode etter at bevegelsessemantikk dukket opp i C++. Dette er grunnen til at jeg elsker C++-språket! Det er flere og flere nye måter å skyte ditt eget bein på. PVS-Studio-analysatoren vil alltid ha arbeid :).
Fragment N8: Nullpeker
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] Frareferanse av null-pekeren 'Type' kan finne sted. PrettyFunctionDumper.cpp 233
I tillegg til feilbehandlere, blir feilsøkingsutskriftsfunksjoner vanligvis ikke testet. Vi har nettopp en slik sak foran oss. Funksjonen venter på brukeren, som i stedet for å løse problemene sine, vil bli tvunget til å fikse den.
korrigere:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Nullpeker
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] Frareferanse av null-pekeren 'Ty' kan finne sted. SearchableTableEmitter.cpp 614
Jeg tror alt er klart og ikke krever forklaring.
Fragment N10: Skrivefeil
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: Variabelen 'Identifier->Type' er tilordnet seg selv. FormatTokenLexer.cpp 249
Det er ingen vits i å tilordne en variabel til seg selv. Mest sannsynlig ønsket de å skrive:
Identifier->Type = Question->Type;Fragment N11: Mistenkelig brudd
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] Vurder å inspisere "switch"-erklæringen. Det er mulig at den første "case"-operatøren mangler. SystemZAsmParser.cpp 652
Det er en veldig mistenkelig operatør i begynnelsen bryte. Har du glemt å skrive noe annet her?
Fragment N12: Kontroll av en peker etter 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");
....
}PVS-Studio advarsel: [CWE-476] 'Callee'-pekeren ble brukt før den ble verifisert mot nullptr. Sjekk linjer: 172, 174. AMDGPUInline.cpp 172
Peker Callee i begynnelsen er dereferert på tidspunktet funksjonen kalles getTTI.
Og så viser det seg at denne pekeren bør sjekkes for likestilling nullptr:
if (!Callee || Callee->isDeclaration())Men det er for sent...
Fragment N13 - N...: Kontrollerer en peker etter derereferanse
Situasjonen diskutert i forrige 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'-pekeren ble brukt før den ble verifisert mot nullptr. Sjekk 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'-pekeren ble brukt før den ble verifisert mot nullptr. Sjekk linjer: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Og her:
- V595 [CWE-476] 'U'-pekeren ble brukt før den ble verifisert mot nullptr. Sjekk linjer: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND'-pekeren ble brukt før den ble verifisert mot nullptr. Sjekk linjer: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Og så ble jeg uinteressert i å studere advarslene med nummer V595. Så jeg vet ikke om det er flere lignende feil enn de som er oppført her. Mest sannsynlig er det.
Fragment N17, N18: Mistenkelig skifte
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] Vurder å inspisere uttrykket '~(Størrelse - 1) << 1'. Bitskifting av 32-bits verdi med en påfølgende utvidelse til 64-bits typen. AArch64AddressingModes.h 260
Det er kanskje ikke en feil, og koden fungerer akkurat etter hensikten. Men dette er helt klart et veldig mistenkelig sted og må sjekkes.
La oss si variabelen Størrelse er lik 16, og så planla forfatteren av koden å få den i en variabel NImms verdi:
1111111111111111111111111111111111111111111111111111111111100000
Imidlertid vil resultatet i realiteten være:
0000000000000000000000000000000011111111111111111111111111100000
Faktum er at alle beregninger skjer ved å bruke den 32-biters usignerte typen. Og først da vil denne 32-bits usignerte typen implisitt utvides til uint64_t. I dette tilfellet vil de mest signifikante bitene være null.
Du kan fikse situasjonen slik:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Lignende situasjon: V629 [CWE-190] Vurder å inspisere uttrykket 'Immr << 6'. Bitskifting av 32-bits verdi med en påfølgende utvidelse til 64-bits typen. AArch64AddressingModes.h 269
Fragment N19: Manglende nøkkelord 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 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] Vurder å inspisere applikasjonens logikk. Det er mulig at søkeordet «annet» mangler. AMDGPPUAsmParser.cpp 5655
Det er ingen feil her. Siden den daværende blokken av den første if slutter med fortsette, så spiller det ingen rolle, det er et nøkkelord ellers eller ikke. Uansett vil koden fungere på samme måte. Fortsatt savnet ellers gjør koden mer uklar og farlig. Hvis i fremtiden fortsette forsvinner, vil koden begynne å fungere helt annerledes. Etter min mening er det bedre å legge til ellers.
Fragment N20: Fire skrivefeil av 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 ble sammenkoblet, men brukes ikke. Vurder å inspisere uttrykket 'Result + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] Strengene ble sammenkoblet, men brukes ikke. Vurder å inspisere uttrykket 'Result + "(ObjC Class)" + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Strengene ble sammenkoblet, men brukes ikke. Vurder å inspisere "Resultat + "(ObjC Class EH) " + Name.str()-uttrykket. Symbol.cpp 38
- V655 [CWE-480] Strengene ble sammenkoblet, men brukes ikke. Vurder å inspisere uttrykket 'Resultat + "(ObjC IVar)" + Navn.str()'. Symbol.cpp 41
Ved et uhell brukes +-operatoren i stedet for +=-operatoren. Resultatet er design som er blottet for mening.
Fragment N21: Udefinert oppfø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 cannot be empty");
for (auto &Op : Ops) {
assert(!Op.empty() && "Empty operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Prøv å finne den farlige koden selv. Og dette er et bilde for å distrahere oppmerksomheten for ikke å umiddelbart se på svaret:
PVS-Studio advarsel: [CWE-758] Farlig konstruksjon brukes: 'FeaturesMap[Op] = FeaturesMap.size()', der 'FeaturesMap' er av 'map'-klassen. Dette kan føre til udefinert atferd. RISCVCompressInstEmitter.cpp 490
Problemlinje:
FeaturesMap[Op] = FeaturesMap.size();Hvis element Op ikke finnes, opprettes et nytt element i kartet og antall elementer i dette kartet skrives der. Det er bare ukjent om funksjonen vil bli kalt størrelse før eller etter å legge til et nytt element.
Fragment N22-N24: Gjentatte oppdrag
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] Variabelen 'NType' tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 1663, 1664. MachOObjectFile.cpp 1664
Jeg tror ikke det er en reell feil her. Bare en unødvendig gjentatt oppgave. Men fortsatt en tabbe.
Like måte:
- V519 [CWE-563] Variabelen 'B.NDesc' tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Variabelen tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Flere omplasseringer
La oss nå se på en litt annen versjon av omfordeling.
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] Variabelen 'Alignment' tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 1158, 1160. LoadStoreVectorizer.cpp 1160
Dette er veldig merkelig kode som tilsynelatende inneholder en logisk feil. I begynnelsen, variabel Justering en verdi tildeles avhengig av tilstanden. Og så oppstår oppdraget igjen, men nå uten noen sjekk.
Lignende situasjoner kan sees her:
- V519 [CWE-563] Variabelen 'Effekter' tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Variabelen 'ExpectNoDerefChunk' tildeles verdier to ganger etter hverandre. Kanskje dette er en feil. Sjekk linjer: 4970, 4973. SemaType.cpp 4973
Fragment N28: Alltid sann 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] Uttrykket 'nextByte != 0x90' er alltid sant. X86DisassemblerDecoder.cpp 379
Å sjekke gir ingen mening. Variabel nesteByte alltid ikke lik verdien 0x90, som følger av forrige kontroll. Dette er en slags logisk feil.
Fragment N29 - N...: Alltid sanne/falske forhold
Analysatoren utsteder mange advarsler om at hele tilstanden () eller en del av det () er alltid sant eller usant. Ofte er dette ikke reelle feil, men rett og slett slurvete kode, resultatet av makroutvidelse og lignende. Det er imidlertid fornuftig å se på alle disse advarslene, siden det oppstår ekte logiske feil fra tid til annen. For eksempel er denne delen av koden mistenkelig:
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 av betinget uttrykk er alltid falsk: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanten 0xE er verdien 14 i desimal. Undersøkelse RegNo == 0xe gir ikke mening fordi hvis RegNo > 13, vil funksjonen fullføre utførelsen.
Det var mange andre advarsler med ID-ene V547 og V560, men som med , jeg var ikke interessert i å studere disse advarslene. Det var allerede klart at jeg hadde nok materiale til å skrive en artikkel :). Derfor er det ukjent hvor mange feil av denne typen som kan identifiseres i LLVM ved bruk av PVS-Studio.
Jeg skal gi deg et eksempel på hvorfor det er kjedelig å studere disse triggerne. Analysatoren har helt rett i å utstede en advarsel for følgende kode. Men dette er ikke en feil.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}PVS-Studio Advarsel: V547 [CWE-570] Uttrykket '!HasError' er alltid usant. UnwrappedLineParser.cpp 1635
Fragment N30: Mistenkelig retur
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" i en loop. R600OptimizeVectorRegisters.cpp 63
Dette er enten en feil eller en spesifikk teknikk som er ment å forklare noe for programmerere som leser koden. Dette designet forklarer meg ingenting og ser veldig mistenkelig ut. Det er bedre å ikke skrive slik :).
Trett? Da er det på tide å lage te eller kaffe.
Defekter identifisert av ny diagnostikk
Jeg tror 30 aktiveringer av gammel diagnostikk er nok. La oss nå se hvilke interessante ting som kan bli funnet med den nye diagnostikken som dukket opp i analysatoren etter sjekker. I løpet av denne tiden ble totalt 66 generell diagnostikk lagt til C++-analysatoren.
Fragment N31: Uoppnå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] Uoppnåelig kode oppdaget. Det er mulig at det er en feil. ExecutionUtils.cpp 146
Som du kan se, begge grenene av operatøren if avsluttes med en samtale til operatøren retur. Følgelig beholderen CtorDtorsByPriority vil aldri bli klarert.
Fragment N32: Uoppnå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] Uoppnåelig kode oppdaget. Det er mulig at det er en feil. LLParser.cpp 835
Interessant situasjon. La oss først se på dette stedet:
return ParseTypeIdEntry(SummaryID);
break;Ved første øyekast ser det ut til at det ikke er noen feil her. Det ser ut som operatøren bryte det er en ekstra her, og du kan ganske enkelt slette den. Imidlertid er ikke alt så enkelt.
Analysatoren avgir en advarsel på linjene:
Lex.setIgnoreColonInIdentifiers(false);
return false;Og faktisk er denne koden uoppnåelig. Alle saker i bryter avsluttes med et anrop fra operatøren retur. Og nå meningsløs alene bryte ser ikke så ufarlig ut! Kanskje en av grenene skal slutte med bryte, ikke på retur?
Fragment N33: Tilfeldig tilbakestilling av høye biter
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 på bitmasken er mindre enn størrelsen på den første operanden. Dette vil føre til tap av høyere biter. RuntimeDyld.cpp 815
Vær oppmerksom på at funksjonen getStubAlignment returtype usignert. La oss beregne verdien av uttrykket, forutsatt at funksjonen returnerer verdien 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Legg nå merke til at variabelen Datastørrelse har en 64-bits usignert type. Det viser seg at når du utfører DataSize & 0xFFFFFFF8u-operasjonen, vil alle trettito høyordensbiter bli tilbakestilt til null. Mest sannsynlig er det ikke dette programmereren ønsket. Jeg mistenker at han ønsket å beregne: DataSize & 0xFFFFFFFFFFFFFFF8u.
For å fikse feilen, bør du skrive dette:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Eller så:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Mislykket eksplisitt 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] Mulig overløp. Vurder å caste operander av 'NumElts * Scale'-operatoren til typen 'size_t', ikke resultatet. X86ISelLowering.h 1577
Eksplisitt typeavstøpning brukes for å unngå overløp ved multiplisering av typevariabler int. Eksplisitt type støping her beskytter imidlertid ikke mot overløp. Først vil variablene multipliseres, og først da vil 32-bits resultatet av multiplikasjonen utvides 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 ble funnet. Kanskje dette er en skrivefeil og 'Op1'-variabelen bør brukes i stedet for 'Op0'. InstCombineCompares.cpp 5507
Denne nye interessante diagnostikken identifiserer situasjoner der en kodebit har blitt kopiert og noen navn i den har begynt å bli endret, men på ett sted har de ikke korrigert det.
Vær oppmerksom på at de endret seg i den andre blokken Op0 på Op1. Men på ett sted fikset de det ikke. Mest sannsynlig burde det vært skrevet slik:
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'-variabelen er tildelt, men brukes ikke ved slutten av funksjonen. SIModeRegister.cpp 48
Det er veldig farlig å gi funksjonsargumenter de samme navnene som klassemedlemmer. Det er veldig lett å bli forvirret. Vi har nettopp en slik sak foran oss. Dette uttrykket gir ikke mening:
Mode &= Mask;Funksjonsargumentet endres. Det er alt. Dette argumentet brukes ikke lenger. Mest sannsynlig burde du ha skrevet det slik:
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] Variabelen 'Størrelse' er tilordnet, men brukes ikke ved slutten av funksjonen. Object.cpp 424
Situasjonen er lik den forrige. Det skal skrives:
this->Size += this->EntrySize;Fragment N38-N47: De glemte å sjekke indeksen
Tidligere har vi sett på eksempler på diagnostisk utløsning . Dens essens er at pekeren er dereferert i begynnelsen, og først deretter sjekket. Ung diagnostikk er motsatt i betydning, men avslører også mange feil. Den identifiserer situasjoner der pekeren ble sjekket i begynnelsen og deretter glemt å gjøre det. La oss se på slike tilfeller som finnes 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'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 729, 738. TargetTransformInfoImpl.h 738
variabel Ptr kan være lik nullptr, som det fremgår av sjekken:
if (Ptr != nullptr)Men under denne pekeren er det referert uten foreløpig kontroll:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());La oss vurdere en annen lignende sak.
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'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 3228, 3231. CGDebugInfo.cpp 3231
Vær oppmerksom på skiltet FD. Jeg er sikker på at problemet er godt synlig og ingen spesiell forklaring er nødvendig.
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'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 960, 965. InterleavedLoadCombinePass.cpp 965
Hvordan beskytte deg mot slike feil? Vær mer oppmerksom på Code-Review og bruk den statiske analysatoren PVS-Studio for å sjekke koden din regelmessig.
Det er ingen vits i å sitere andre kodefragmenter med feil av denne typen. Jeg vil bare legge igjen en liste over advarsler i artikkelen:
- V1004 [CWE-476] 'Expr'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] 'PI'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] 'StatepointCall'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] 'RV'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] 'TC'-pekeren ble brukt på en usikker måte etter at den ble verifisert mot nullptr. Sjekk linjer: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Ikke kritisk, men en defekt (mulig minnelekkasje)
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 peker uten eier legges til 'Strategies'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-isel-fuzzer.cpp 58
For å legge til et element på slutten av en beholder som std::vektor > du kan ikke bare skrive xxx.push_back(ny X), siden det ikke er noen implisitt konvertering fra X* в std::unique_ptr.
En vanlig løsning er å skrive xxx.emplace_back(ny X)siden den kompilerer: metode emplace_back konstruerer et element direkte fra argumenter og kan derfor bruke eksplisitte konstruktører.
Det er ikke trygt. Hvis vektoren er full, blir minnet re-allokert. Minnetildelingsoperasjonen kan mislykkes, noe som resulterer i at et unntak blir kastet std::bad_alloc. I dette tilfellet vil pekeren gå tapt og det opprettede objektet vil aldri bli slettet.
En trygg løsning er å skape unik_ptrsom vil eie pekeren før vektoren prøver å omfordele minne:
xxx.push_back(std::unique_ptr<X>(new X))Siden C++14 kan du bruke 'std::make_unique':
xxx.push_back(std::make_unique<X>())Denne typen feil er ikke kritisk for LLVM. Hvis minne ikke kan tildeles, vil kompilatoren ganske enkelt stoppe. Imidlertid for applikasjoner med lang , som ikke bare kan avsluttes hvis minnetildeling mislykkes, dette kan være en skikkelig ekkel feil.
Så selv om denne koden ikke utgjør en praktisk trussel mot LLVM, fant jeg det nyttig å snakke om dette feilmønsteret og at PVS-Studio-analysatoren har lært å identifisere det.
Andre advarsler av denne typen:
- V1023 [CWE-460] En peker uten eier legges til 'Passes'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. PassManager.h 546
- V1023 [CWE-460] En peker uten eier legges til 'AAs'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. AliasAnalysis.h 324
- V1023 [CWE-460] En peker uten eier legges til 'Entries'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] En peker uten eier legges til 'AllEdges'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. CFGMST.h 268
- V1023 [CWE-460] En peker uten eier legges til 'VMaps'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. SimpleLoopUswitch.cpp 2012
- V1023 [CWE-460] En peker uten eier legges til 'Records'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. FDRLogBuilder.h 30
- V1023 [CWE-460] En peker uten eier legges til 'PendingSubmodules'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. ModuleMap.cpp 810
- V1023 [CWE-460] En peker uten eier legges til 'Objekter'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. DebugMap.cpp 88
- V1023 [CWE-460] En peker uten eier legges til 'Strategies'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 685
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 686
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 688
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 689
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 690
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 691
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 692
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 693
- V1023 [CWE-460] En peker uten eier legges til 'Modifiers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. llvm-stress.cpp 694
- V1023 [CWE-460] En peker uten eier legges til 'Operander'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] En peker uten eier legges til 'Stash'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] En peker uten eier legges til 'Matchers'-beholderen ved hjelp av 'emplace_back'-metoden. En minnelekkasje vil oppstå i tilfelle et unntak. GlobalISelEmitter.cpp 2702
Konklusjon
Jeg ga ut 60 advarsler totalt og sluttet så. Er det andre feil som PVS-Studio-analysatoren oppdager i LLVM? Ja jeg har. Men da jeg skrev ut kodefragmenter for artikkelen, var det sent på kvelden, eller rettere sagt natt, og jeg bestemte meg for at det var på tide å kalle det en dag.
Jeg håper du fant det interessant og vil prøve PVS-Studio-analysatoren.
Du kan laste ned analysatoren og få nøkkelen til minesveiperen på .
Viktigst av alt, bruk statisk analyse regelmessig. Engangssjekker, utført av oss for å popularisere metodikken for statisk analyse og PVS-Studio er ikke et normalt scenario.
Lykke til med å forbedre kvaliteten og påliteligheten til koden din!
Hvis du vil dele denne artikkelen med et engelsktalende publikum, vennligst bruk oversettelseslenken: Andrey Karpov. .
Kilde: www.habr.com