Minili sta več kot dve leti od zadnjega preverjanja kode projekta LLVM z našim analizatorjem PVS-Studio. Poskrbimo, da bo analizator PVS-Studio še vedno vodilno orodje za prepoznavanje napak in potencialnih ranljivosti. Da bi to naredili, bomo preverili in našli nove napake v izdaji LLVM 8.0.0.
Članek, ki bo napisan
Če sem iskren, tega članka nisem hotel napisati. Ni zanimivo pisati o projektu, ki smo ga že večkrat preverili (, , ). Bolje je pisati o nečem novem, a nimam izbire.
Vsakič, ko je izdana ali posodobljena nova različica LLVM , na našo pošto prejemamo vprašanja naslednje vrste:
Poglejte, nova različica Clang Static Analyzer se je naučila najti nove napake! Zdi se mi, da se pomen uporabe PVS-Studio zmanjšuje. Clang najde več napak kot prej in dohiti zmožnosti PVS-Studia. Kaj mislis o tem?
Na to vedno želim odgovoriti nekaj takega:
Tudi mi ne sedimo brez dela! Bistveno smo izboljšali zmogljivosti analizatorja PVS-Studio. Torej brez skrbi, še naprej vodimo kot doslej.
Na žalost je to slab odgovor. V njej ni nobenih dokazov. In zato zdaj pišem ta članek. Torej, projekt LLVM je bil še enkrat preverjen in v njem so bile odkrite različne napake. Zdaj bom prikazal tiste, ki so se mi zdele zanimive. Clang Static Analyzer ne more najti teh napak (ali pa je to zelo neprijetno storiti z njegovo pomočjo). Ampak lahko. Še več, vse te napake sem našel in zapisal v enem večeru.
Toda pisanje članka je trajalo več tednov. Enostavno se nisem mogel prisiliti, da bi vse to spravil v besedilo :).
Mimogrede, če vas zanima, katere tehnologije se uporabljajo v analizatorju PVS-Studio za prepoznavanje napak in potencialnih ranljivosti, potem predlagam, da se seznanite s tem .
Nova in stara diagnostika
Kot smo že omenili, je bil pred približno dvema letoma projekt LLVM ponovno preverjen in ugotovljene napake so bile odpravljene. Zdaj bo ta članek predstavil novo skupino napak. Zakaj so bile odkrite nove napake? Za to obstajajo 3 razlogi:
- Projekt LLVM se razvija, spreminja staro kodo in dodaja novo kodo. Seveda so v spremenjeni in napisani kodi nove napake. To jasno dokazuje, da je treba statično analizo uporabljati redno in ne občasno. Naši članki dobro prikazujejo zmogljivosti analizatorja PVS-Studio, vendar to nima nobene zveze z izboljšanjem kakovosti kode in zmanjšanjem stroškov odpravljanja napak. Redno uporabljajte statični analizator kode!
- Dokončujemo in izboljšujemo obstoječo diagnostiko. Zato lahko analizator prepozna napake, ki jih med prejšnjimi pregledi ni opazil.
- V PVS-Studio se je pojavila nova diagnostika, ki pred dvema letoma ni obstajala. Odločil sem se, da jih izpostavim v ločenem razdelku, da bi jasno prikazal razvoj PVS-Studio.
Napake, ugotovljene z diagnostiko, ki je obstajala pred 2 leti
Fragment N1: Kopiraj-Prilepi
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
....
}Opozorilo PVS-Studio: [CWE-570] Obstajata enaka podizraza 'Name.startswith("avx512.mask.permvar.")' levo in desno od '||' operater. AutoUpgrade.cpp 73
Dvakrat se preveri, ali se ime začne s podnizom "avx512.mask.permvar.". Pri drugem preverjanju so očitno želeli napisati nekaj drugega, a so pozabili popraviti prepisano besedilo.
Fragment N2: Tiskarska napaka
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;
....
}Opozorilo PVS-Studio: V501 Obstajata enaka podizraza 'CXNameRange_WantQualifier' levo in desno od '|' operater. CIndex.cpp 7245
Zaradi tipkarske napake je istoimenska konstanta uporabljena dvakrat CXNameRange_WantQualifier.
Fragment N3: Zmeda s prednostjo operaterja
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Opozorilo PVS-Studio: [CWE-783] Morda operater '?:' deluje na drugačen način, kot je bilo pričakovano. Operator '?:' ima nižjo prednost kot operator '=='. PPCTargetTransformInfo.cpp 404
Po mojem mnenju je to zelo lepa napaka. Ja, vem, da imam čudne predstave o lepoti :).
Zdaj, glede na , je izraz ovrednoten na naslednji način:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0S praktičnega vidika takšen pogoj ni smiseln, saj ga je mogoče zmanjšati na:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())To je očitna napaka. Najverjetneje so želeli primerjati 0/1 s spremenljivko Kazalo. Če želite popraviti kodo, morate okoli ternarnega operaterja dodati oklepaje:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Mimogrede, ternarni operater je zelo nevaren in povzroča logične napake. Pri tem bodite zelo previdni in ne bodite požrešni z oklepaji. To temo sem si podrobneje ogledal , v poglavju »Pazite na ?: Operator in ga zapišite v oklepaje.«
Fragment N4, N5: ničelni kazalec
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;
}
....
}Opozorilo PVS-Studio: [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'LHS'. TGParser.cpp 2152
Če kazalec LHS ni, je treba izdati opozorilo. Vendar bo namesto tega ta isti ničelni kazalec dereferenciran: LHS->getAsString().
To je zelo tipična situacija, ko se napaka skriva v obdelovalniku napak, saj jih nihče ne testira. Statični analizatorji preverijo vso dosegljivo kodo, ne glede na to, kako pogosto se uporablja. To je zelo dober primer, kako statična analiza dopolnjuje druge tehnike testiranja in zaščite pred napakami.
Podobna napaka pri ravnanju s kazalcem RHS dovoljeno v kodi spodaj: V522 [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'RHS'. TGParser.cpp 2186
Fragment N6: Uporaba kazalca po premikanju
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);
}
....
}Opozorilo PVS-Studio: V522 [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'ProgClone'. Miscompilation.cpp 601
Na začetku pametna kazalka ProgClone preneha lastništvo predmeta:
BD.setNewProgram(std::move(ProgClone));Pravzaprav zdaj ProgClone je ničelni kazalec. Zato bi se moralo dereferenciranje ničelnega kazalca pojaviti tik spodaj:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Toda v resnici se to ne bo zgodilo! Upoštevajte, da se zanka dejansko ne izvede.
Na začetku posode MiscompiledFunctions očiščeno:
MiscompiledFunctions.clear();Nato se velikost tega vsebnika uporabi v pogoju zanke:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Preprosto je videti, da se zanka ne zažene. Mislim, da je tudi to napaka in bi morala biti koda napisana drugače.
Zdi se, da smo naleteli na tisto famozno pariteto napak! Ena napaka prikrije drugo :).
Fragment N7: Uporaba kazalca po premikanju
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;
}
....
}Opozorilo PVS-Studio: V522 [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'Test'. Miscompilation.cpp 709
Spet ista situacija. Sprva se vsebina predmeta premakne, nato pa se uporabi, kot da se ni nič zgodilo. To situacijo vedno pogosteje vidim v programski kodi, potem ko se je v C++ pojavila semantika gibanja. Zato obožujem jezik C++! Vedno več je novih načinov, kako si ustreliti nogo. Analizator PVS-Studio bo vedno imel delo :).
Fragment N8: ničelni kazalec
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);
}Opozorilo PVS-Studio: V522 [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'Vrsta'. PrettyFunctionDumper.cpp 233
Poleg obdelovalcev napak običajno niso preizkušene niti funkcije izpisa za odpravljanje napak. Pred nami je prav tak primer. Funkcija čaka na uporabnika, ki bo namesto reševanja svojih težav prisiljen le-to popraviti.
Pravilno:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: ničelni kazalec
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());
....
}Opozorilo PVS-Studio: V522 [CWE-476] Lahko pride do dereferenciranja ničelnega kazalca 'Ty'. SearchableTableEmitter.cpp 614
Mislim, da je vse jasno in ne zahteva razlage.
Fragment N10: Tiskarska napaka
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;
}Opozorilo PVS-Studio: Spremenljivka 'Identifier->Type' je dodeljena sama sebi. FormatTokenLexer.cpp 249
Nima smisla dodeljevati spremenljivke same sebi. Najverjetneje so želeli napisati:
Identifier->Type = Question->Type;Fragment N11: Sumljiv zlom
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Opozorilo PVS-Studio: [CWE-478] Razmislite o pregledu izjave 'switch'. Možno je, da prvi operator 'case' manjka. SystemZAsmParser.cpp 652
Na začetku je zelo sumljiv operater odmor. Si še kaj pozabil tukaj napisati?
Fragment N12: Preverjanje kazalca po dereferenciranju
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");
....
}Opozorilo PVS-Studio: [CWE-476] Kazalec 'Callee' je bil uporabljen, preden je bil preverjen glede na nullptr. Preverite vrstice: 172, 174. AMDGPUInline.cpp 172
Kazalec Callee na začetku se dereferencira v času klica funkcije getTTI.
In potem se izkaže, da je treba ta kazalec preveriti glede enakosti nullptr:
if (!Callee || Callee->isDeclaration())Ampak je prepozno…
Fragment N13 - N...: Preverjanje kazalca po dereferenciranju
Situacija, obravnavana v prejšnjem fragmentu kode, ni edinstvena. Pojavi se tukaj:
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()) { // <=
....
}Opozorilo PVS-Studio: V595 [CWE-476] Kazalec 'CalleeFn' je bil uporabljen, preden je bil preverjen glede na nullptr. Preverite vrstice: 1079, 1081. SimplifyLibCalls.cpp 1079
In tukaj:
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()); // <=
....
}Opozorilo PVS-Studio: V595 [CWE-476] Kazalec 'ND' je bil uporabljen, preden je bil preverjen glede na nullptr. Preverite vrstice: 532, 534. SemaTemplateInstantiateDecl.cpp 532
In tukaj:
- V595 [CWE-476] Kazalec 'U' je bil uporabljen, preden je bil preverjen glede na nullptr. Preverite vrstice: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Kazalec 'ND' je bil uporabljen, preden je bil preverjen glede na nullptr. Preverite vrstice: 2149, 2151. SemaTemplateInstantiate.cpp 2149
In potem mi je postalo nezainteresirano študirati opozorila s številko V595. Zato ne vem, ali obstaja še več podobnih napak poleg tukaj navedenih. Najverjetneje obstaja.
Fragment N17, N18: Sumljiv premik
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Opozorilo PVS-Studio: [CWE-190] Razmislite o pregledu izraza '~(Size - 1) << 1'. Bitni premik 32-bitne vrednosti z naknadno razširitvijo na 64-bitni tip. AArch64AddressingModes.h 260
Morda ne gre za napako in koda deluje točno tako, kot je predvideno. Toda to je očitno zelo sumljivo mesto in ga je treba preveriti.
Recimo spremenljivka Velikosti je enako 16, nato pa je avtor kode načrtoval, da ga dobi v spremenljivki NImms vrednost:
1111111111111111111111111111111111111111111111111111111111100000
Vendar pa bo v resnici rezultat:
0000000000000000000000000000000011111111111111111111111111100000
Dejstvo je, da se vsi izračuni izvajajo z uporabo 32-bitnega nepredznačenega tipa. In šele takrat bo ta 32-bitni nepodpisani tip implicitno razširjen na uint64_t. V tem primeru bodo najpomembnejši biti enaki nič.
Situacijo lahko popravite takole:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Podobna situacija: V629 [CWE-190] Razmislite o pregledu izraza 'Immr << 6'. Bitni premik 32-bitne vrednosti z naknadno razširitvijo na 64-bitni tip. AArch64AddressingModes.h 269
Fragment N19: Manjka ključna beseda ostalo?
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");
}
....
}Opozorilo PVS-Studio: [CWE-670] Razmislite o pregledu logike aplikacije. Možno je, da ključna beseda 'drugo' manjka. AMDGPUAsmParser.cpp 5655
Tukaj ni nobene napake. Od takratnega bloka prvega if konča z naprej, potem ni pomembno, obstaja ključna beseda ostalo ali ne. V vsakem primeru bo koda delovala enako. Še vedno pogrešan ostalo naredi kodo bolj nejasno in nevarno. Če v prihodnosti naprej izgine, bo koda začela delovati popolnoma drugače. Po mojem mnenju je bolje dodati ostalo.
Fragment N20: Štiri tipkarske napake istega tipa
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;
}Opozorila PVS-Studio:
- V655 [CWE-480] Nizi so bili povezani, vendar se ne uporabljajo. Razmislite o pregledu izraza 'Result + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] Nizi so bili povezani, vendar se ne uporabljajo. Razmislite o pregledu izraza 'Result + "(ObjC Class)" + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Nizi so bili povezani, vendar se ne uporabljajo. Razmislite o pregledu izraza 'Rezultat + "(ObjC Class EH) " + Name.str()'. Symbol.cpp 38
- V655 [CWE-480] Nizi so bili povezani, vendar se ne uporabljajo. Razmislite o pregledu izraza 'Rezultat + "(ObjC IVar)" + Name.str()'. Symbol.cpp 41
Po naključju je namesto operatorja += uporabljen operator +. Rezultat so dizajni brez pomena.
Fragment N21: Nedefinirano vedenje
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();
}
}
}Poskusite sami najti nevarno kodo. In to je slika za odvračanje pozornosti, da ne bi takoj pogledali odgovora:
Opozorilo PVS-Studio: [CWE-758] Uporabljena je nevarna konstrukcija: 'FeaturesMap[Op] = FeaturesMap.size()', kjer je 'FeaturesMap' razreda 'map'. To lahko povzroči nedefinirano vedenje. RISCVCompressInstEmitter.cpp 490
Problemska vrstica:
FeaturesMap[Op] = FeaturesMap.size();Če element Op ni najden, potem se ustvari nov element v zemljevidu in tam je zapisano število elementov v tem zemljevidu. Samo ni znano, ali bo funkcija poklicana velikost pred ali po dodajanju novega elementa.
Fragment N22-N24: Ponovljene naloge
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;
}
....
}Opozorilo PVS-Studio: [CWE-563] Spremenljivki 'NType' so vrednosti dodeljene dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 1663, 1664. MachOObjectFile.cpp 1664
Mislim, da tukaj ni prave napake. Samo nepotrebna ponavljajoča se naloga. Ampak še vedno napaka.
Enako:
- V519 [CWE-563] Spremenljivki 'B.NDesc' so vrednosti dodeljene dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Spremenljivki se vrednosti dodelijo dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Več predodelitev
Zdaj pa poglejmo nekoliko drugačno različico prerazporeditve.
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;
....
}Opozorilo PVS-Studio: V519 [CWE-563] Spremenljivki 'Alignment' so vrednosti dodeljene dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 1158, 1160. LoadStoreVectorizer.cpp 1160
To je zelo čudna koda, ki očitno vsebuje logično napako. Na začetku spremenljivo Poravnava vrednost je dodeljena glede na stanje. In potem se dodelitev ponovi, vendar zdaj brez preverjanja.
Podobne situacije si lahko ogledate tukaj:
- V519 [CWE-563] Spremenljivki 'Effects' so vrednosti dodeljene dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Spremenljivki 'ExpectNoDerefChunk' so vrednosti dodeljene dvakrat zaporedoma. Morda je to napaka. Preverite vrstice: 4970, 4973. SemaType.cpp 4973
Fragment N28: Vedno resnično stanje
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;
}
....
}Opozorilo PVS-Studio: [CWE-571] Izraz 'nextByte != 0x90' je vedno resničen. X86DisassemblerDecoder.cpp 379
Preverjanje nima smisla. Spremenljivka nextByte vedno ni enaka vrednosti 0x90, kar izhaja iz prejšnjega preverjanja. To je neke vrste logična napaka.
Fragment N29 - N...: pogoji vedno res/false
Analizator izda veliko opozoril, da celotno stanje () ali njen del () je vedno resnično ali napačno. Pogosto ne gre za prave napake, temveč preprosto za površno kodo, ki je posledica razširitve makra ipd. Vendar je smiselno preučiti vsa ta opozorila, saj se občasno pojavijo resnične logične napake. Na primer, ta del kode je sumljiv:
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;
....
}Opozorilo PVS-Studio: [CWE-570] Del pogojnega izraza je vedno napačen: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanta 0xE je decimalna vrednost 14. Pregled Reg št == 0xe nima smisla, ker če Reg št. > 13, potem bo funkcija zaključila svojo izvedbo.
Bilo je še veliko drugih opozoril z ID-jema V547 in V560, a kot pri , me ta opozorila niso zanimala. Bilo je že jasno, da imam dovolj materiala za pisanje članka :). Zato ni znano, koliko napak te vrste je mogoče identificirati v LLVM z uporabo PVS-Studio.
Dal vam bom primer, zakaj je preučevanje teh sprožilcev dolgočasno. Analizator ima popolnoma prav, ko je izdal opozorilo za naslednjo kodo. Vendar to ni napaka.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Opozorilo PVS-Studio: V547 [CWE-570] Izraz '!HasError' je vedno napačen. UnwrappedLineParser.cpp 1635
Fragment N30: Sumljiva vrnitev
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();
}
....
}Opozorilo PVS-Studio: [CWE-670] Brezpogojna 'vrnitev' znotraj zanke. R600OptimizeVectorRegisters.cpp 63
To je napaka ali posebna tehnika, ki je namenjena razložitvi nekaj programerjem, ki berejo kodo. Ta oblika mi ne pojasni ničesar in izgleda zelo sumljivo. Bolje da ne pišeš tako :).
Utrujeni? Potem je čas za pripravo čaja ali kave.
Napake, odkrite z novo diagnostiko
Mislim, da je 30 aktivacij stare diagnostike dovolj. Poglejmo zdaj, kaj zanimivega lahko najdemo z novo diagnostiko, ki se je pojavila v analizatorju po tem preverjanja. V tem času je bilo analizatorju C++ dodanih skupaj 66 splošnih diagnostik.
Fragment N31: nedosegljiva koda
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();
}Opozorilo PVS-Studio: [CWE-561] Zaznana nedosegljiva koda. Možno je, da je prisotna napaka. ExecutionUtils.cpp 146
Kot lahko vidite, obe veji operaterja if konča s klicem operaterja vrnitev. V skladu s tem posoda CtorDtorsByPriority nikoli ne bo očiščen.
Fragment N32: nedosegljiva koda
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;
}Opozorilo PVS-Studio: V779 [CWE-561] Zaznana nedosegljiva koda. Možno je, da je prisotna napaka. LLParser.cpp 835
Zanimiva situacija. Poglejmo najprej to mesto:
return ParseTypeIdEntry(SummaryID);
break;Na prvi pogled se zdi, da tu ni nobene napake. Izgleda kot operater odmor tukaj je dodaten in ga lahko preprosto izbrišete. Vendar pa ni vse tako preprosto.
Analizator izda opozorilo v vrsticah:
Lex.setIgnoreColonInIdentifiers(false);
return false;In res, ta koda je nedosegljiva. Vsi primeri v stikalo konča s klicem operaterja vrnitev. In zdaj nesmiselno sam odmor ne izgleda tako neškodljivo! Morda bi se ena od vej morala končati z odmorvendar ne na vrnitev?
Fragment N33: Naključna ponastavitev visokih bitov
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);
....
}Opozorilo PVS-Studio: Velikost bitne maske je manjša od velikosti prvega operanda. To bo povzročilo izgubo višjih bitov. RuntimeDyld.cpp 815
Upoštevajte, da funkcija getStubAlignment vrne vrsto nepodpisani. Izračunajmo vrednost izraza ob predpostavki, da funkcija vrne vrednost 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Zdaj opazite, da spremenljivka DataSize ima 64-bitni nepodpisani tip. Izkazalo se je, da bo pri izvajanju operacije DataSize & 0xFFFFFFF8u vseh dvaintrideset bitov višjega reda ponastavljenih na nič. Najverjetneje to ni tisto, kar je programer želel. Sumim, da je hotel izračunati: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Če želite odpraviti napako, morate napisati tole:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Ali tako:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: neuspešna eksplicitna pretvorba tipa
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);
....
}Opozorilo PVS-Studio: [CWE-190] Možno prelivanje. Razmislite o pretvorbi operandov operatorja 'NumElts * Scale' v tip 'size_t', ne v rezultat. X86ISelLowering.h 1577
Eksplicitno prelivanje tipa se uporablja za preprečevanje prelivanja pri množenju spremenljivk tipa int. Vendar eksplicitno prelivanje tipov tukaj ne ščiti pred prelivanjem. Najprej se pomnožijo spremenljivke in šele nato se 32-bitni rezultat množenja razširi na tip .
Fragment N35: Neuspešno kopiranje in lepljenje
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] Najdena sta bila dva podobna fragmenta kode. Morda je to tipkarska napaka in je treba namesto 'Op1' uporabiti spremenljivko 'Op0'. InstCombineCompares.cpp 5507
Ta nova zanimiva diagnostika identificira situacije, ko je bil del kode kopiran in so se nekatera imena v njem začela spreminjati, vendar na enem mestu tega niso popravili.
Upoštevajte, da so se v drugem bloku spremenili Op0 o Op1. Toda na enem mestu tega niso popravili. Najverjetneje bi moralo biti napisano takole:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Spremenljiva zmeda
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Opozorilo PVS-Studio: [CWE-563] Spremenljivka 'Mode' je dodeljena, vendar ni uporabljena do konca funkcije. SIModeRegister.cpp 48
Zelo nevarno je dati argumentom funkcije enaka imena kot članom razreda. Zelo enostavno se je zmešati. Pred nami je prav tak primer. Ta izraz nima smisla:
Mode &= Mask;Argument funkcije se spremeni. To je vse. Ta argument se ne uporablja več. Najverjetneje bi morali napisati takole:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Spremenljiva zmeda
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;
}Opozorilo PVS-Studio: V1001 [CWE-563] Spremenljivka 'Velikost' je dodeljena, vendar ni uporabljena do konca funkcije. Object.cpp 424
Situacija je podobna prejšnji. Moralo bi biti napisano:
this->Size += this->EntrySize;Fragment N38-N47: Pozabili so preveriti indeks
Prej smo si ogledali primere diagnostičnega proženja . Njegovo bistvo je, da se kazalec na začetku dereferencira in šele nato preveri. Mlada diagnostika je nasproten po pomenu, vendar razkriva tudi veliko napak. Identificira situacije, v katerih je bil kazalec na začetku preverjen in nato na to pozabljen. Oglejmo si takšne primere, ki jih najdemo v 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()); // <=
....
}Opozorilo PVS-Studio: V1004 [CWE-476] Kazalec 'Ptr' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 729, 738. TargetTransformInfoImpl.h 738
Spremenljivka Ptr lahko enaka nullptr, kar dokazuje ček:
if (Ptr != nullptr)Vendar pa je spodaj ta kazalec dereferenciran brez predhodnega preverjanja:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Poglejmo še en podoben primer.
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(); // <=
....
}Opozorilo PVS-Studio: V1004 [CWE-476] Kazalec 'FD' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 3228, 3231. CGDebugInfo.cpp 3231
Bodite pozorni na znak FD. Prepričan sem, da je problem jasno viden in ni potrebna posebna razlaga.
In še:
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()); // <=
....
}Opozorilo PVS-Studio: V1004 [CWE-476] Kazalec 'PtrTy' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 960, 965. InterleavedLoadCombinePass.cpp 965
Kako se zaščititi pred takimi napakami? Bodite bolj pozorni na Code-Review in uporabljajte statični analizator PVS-Studio za redno preverjanje kode.
Nima smisla navajati drugih fragmentov kode s tovrstnimi napakami. V članku bom pustil le seznam opozoril:
- V1004 [CWE-476] Kazalec 'Expr' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Kazalec 'PI' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Kazalec 'StatepointCall' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Kazalec 'RV' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Kazalec 'CalleeFn' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Kazalec 'TC' je bil uporabljen nevarno, potem ko je bil preverjen glede na nullptr. Preverite vrstice: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Ni kritično, vendar je napaka (možno uhajanje pomnilnika)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Opozorilo PVS-Studio: [CWE-460] Kazalec brez lastnika je dodan vsebniku »Strategije« z metodo »emplace_back«. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-isel-fuzzer.cpp 58
Če želite dodati element na konec vsebnika, kot je std::vector > ne moreš samo pisati xxx.push_back(nov X), ker ni implicitne pretvorbe iz X* в std::unique_ptr.
Pogosta rešitev je pisanje xxx.emplace_back(nov X)saj sestavlja: metoda emplace_back sestavi element neposredno iz njegovih argumentov in zato lahko uporablja eksplicitne konstruktorje.
Ni varno. Če je vektor poln, se pomnilnik ponovno dodeli. Operacija ponovne dodelitve pomnilnika morda ne uspe, kar povzroči izjemo std::bad_alloc. V tem primeru bo kazalec izgubljen in ustvarjeni predmet ne bo nikoli izbrisan.
Varna rešitev je ustvarjanje edinstven_ptrki bo lastnik kazalca, preden bo vektor poskušal znova dodeliti pomnilnik:
xxx.push_back(std::unique_ptr<X>(new X))Od C++14 lahko uporabite 'std::make_unique':
xxx.push_back(std::make_unique<X>())Ta vrsta napake ni kritična za LLVM. Če pomnilnika ni mogoče dodeliti, se bo prevajalnik preprosto ustavil. Vendar pa za aplikacije z dolgim , ki se ne more kar prekiniti, če dodelitev pomnilnika ne uspe, je to lahko resnično zoprna napaka.
Torej, čeprav ta koda ne predstavlja praktične grožnje za LLVM, se mi je zdelo koristno govoriti o tem vzorcu napake in o tem, da se ga je analizator PVS-Studio naučil prepoznati.
Druga opozorila te vrste:
- V1023 [CWE-460] Kazalec brez lastnika je dodan v vsebnik 'Passes' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. PassManager.h 546
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'AA' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. AliasAnalysis.h 324
- V1023 [CWE-460] Kazalec brez lastnika je dodan v vsebnik 'Entries' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'AllEdges' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. CFGMST.h 268
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'VMaps' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Zapisi' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. FDRLogBuilder.h 30
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'PendingSubmodules' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. ModuleMap.cpp 810
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Predmeti' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. DebugMap.cpp 88
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Strategije' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 685
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 686
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 688
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 689
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 690
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 691
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 692
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 693
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Modifikatorji' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. llvm-stres.cpp 694
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Operandov' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Stash' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Kazalec brez lastnika je dodan vsebniku 'Matchers' z metodo 'emplace_back'. V primeru izjeme bo prišlo do puščanja pomnilnika. GlobalISelEmitter.cpp 2702
Zaključek
Skupaj sem izrekel 60 opozoril in potem nehal. Ali obstajajo druge napake, ki jih analizator PVS-Studio zazna v LLVM? Ja, jaz imam. Vendar, ko sem pisal fragmente kode za članek, je bil pozen večer ali bolje rečeno celo noč, in odločil sem se, da je čas, da to pokličem.
Upam, da se vam je zdelo zanimivo in boste želeli preizkusiti analizator PVS-Studio.
Analizator lahko prenesete in dobite ključ minolovca na .
Predvsem pa redno uporabljajte statično analizo. Enkratni čeki, ki smo jih izvedli z namenom popularizacije metodologije statične analize in PVS-Studio, niso običajen scenarij.
Vso srečo pri izboljšanju kakovosti in zanesljivosti kode!
Če želite ta članek deliti z angleško govorečim občinstvom, uporabite povezavo za prevod: Andrey Karpov. .
Vir: www.habr.com