Prošlo je više od dvije godine od posljednje provjere koda LLVM projekta pomoću našeg PVS-Studio analizatora. Pobrinimo se da PVS-Studio analizator i dalje bude vodeći alat za identifikaciju grešaka i potencijalnih ranjivosti. Da bismo to učinili, provjerit ćemo i pronaći nove greške u izdanju LLVM 8.0.0.
Članak treba napisati
Da budem iskren, nisam želio da pišem ovaj članak. Nije zanimljivo pisati o projektu koji smo već nekoliko puta provjerili (, , ). Bolje je pisati o nečemu novom, ali nemam izbora.
Svaki put kada se objavi ili ažurira nova verzija LLVM-a , na našu poštu primamo pitanja sljedećeg tipa:
Gledajte, nova verzija Clang Static Analyzer je naučila da pronađe nove greške! Čini mi se da je relevantnost korištenja PVS-Studio sve manja. Clang pronalazi više grešaka nego ranije i sustiže mogućnosti PVS-Studio. Šta mislite o ovome?
Na ovo uvek želim da odgovorim nešto poput:
Ni mi ne sedimo besposleni! Značajno smo poboljšali mogućnosti PVS-Studio analizatora. Zato ne brinite, nastavljamo da vodimo kao i do sada.
Nažalost, ovo je loš odgovor. U tome nema dokaza. I zato sada pišem ovaj članak. Dakle, LLVM projekat je još jednom provjeren i u njemu su pronađene razne greške. Sada ću pokazati one koji su mi se činili zanimljivi. Clang Static Analyzer ne može pronaći ove greške (ili je krajnje nezgodno to učiniti uz njegovu pomoć). Ali možemo. Štaviše, sve ove greške sam pronašao i zapisao u jednoj večeri.
Ali pisanje članka je trajalo nekoliko sedmica. Jednostavno se nisam mogao natjerati da sve ovo stavim u tekst :).
Usput, ako vas zanima koje se tehnologije koriste u analizatoru PVS-Studio za identifikaciju grešaka i potencijalnih ranjivosti, onda predlažem da se upoznate s ovim .
Nova i stara dijagnostika
Kao što je već navedeno, prije otprilike dvije godine LLVM projekat je još jednom provjeren, a pronađene greške su ispravljene. Sada će ovaj članak predstaviti novu grupu grešaka. Zašto su pronađene nove greške? Za to postoje 3 razloga:
- LLVM projekat se razvija, mijenja stari kod i dodaje novi kod. Naravno, ima novih grešaka u modifikovanom i pisanom kodu. Ovo jasno pokazuje da statičku analizu treba koristiti redovno, a ne povremeno. Naši članci dobro pokazuju mogućnosti analizatora PVS-Studio, ali to nema nikakve veze sa poboljšanjem kvaliteta koda i smanjenjem troškova ispravljanja grešaka. Redovno koristite statički analizator koda!
- Dovršavamo i unapređujemo postojeću dijagnostiku. Stoga analizator može identificirati greške koje nije primijetio tokom prethodnih skeniranja.
- U PVS-Studiju se pojavila nova dijagnostika koja nije postojala prije 2 godine. Odlučio sam da ih istaknem u posebnom dijelu kako bih jasno prikazao razvoj PVS-Studija.
Defekti utvrđeni dijagnostikom koji su postojali prije 2 godine
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 upozorenje: [CWE-570] Postoje identični podizrazi 'Name.startswith("avx512.mask.permvar.")' lijevo i desno od '||' operater. AutoUpgrade.cpp 73
Dvostruko se provjerava da ime počinje podnizom "avx512.mask.permvar.". U drugoj provjeri očito su htjeli nešto drugo napisati, ali su zaboravili ispraviti kopirani tekst.
Fragment N2: Tipska greška
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;
....
}Upozorenje PVS-Studio: V501 Postoje identični podizrazi 'CXNameRange_WantQualifier' lijevo i desno od '|' operater. CIndex.cpp 7245
Zbog greške u kucanju ista imenovana konstanta se koristi dva puta CXNameRange_WantQualifier.
Fragment N3: Zabuna s prioritetom operatora
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}PVS-Studio upozorenje: [CWE-783] Možda operator '?:' radi na drugačiji način nego što se očekivalo. Operator '?:' ima niži prioritet od operatora '=='. PPCTargetTransformInfo.cpp 404
Po mom mišljenju, ovo je veoma lepa greška. Da, znam da imam čudne ideje o lepoti :).
Sada, prema , izraz se procjenjuje na sljedeći način:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Sa praktične tačke gledišta, takav uslov nema smisla, jer se može svesti na:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Ovo je jasna greška. Najvjerovatnije su htjeli uporediti 0/1 sa varijablom indeks. Da popravite kod, morate dodati zagrade oko ternarnog operatora:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Inače, ternarni operator je vrlo opasan i izaziva logičke greške. Budite veoma oprezni s tim i nemojte biti pohlepni sa zagradama. Pogledao sam ovu temu detaljnije , u poglavlju “Čuvajte se ?: Operator i stavite ga u zagrade.”
Fragment N4, N5: Null pokazivač
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 upozorenje: [CWE-476] Dereferenciranje nultog pokazivača 'LHS' može se dogoditi. TGParser.cpp 2152
Ako je pokazivač LHS je null, treba izdati upozorenje. Međutim, umjesto toga, ovaj isti null pokazivač će biti dereferenciran: LHS->getAsString().
Ovo je vrlo tipična situacija kada je greška skrivena u rukovaocu greškama, jer ih niko ne testira. Statički analizatori provjeravaju sav dostupan kod, bez obzira koliko se često koristi. Ovo je vrlo dobar primjer kako statička analiza nadopunjuje druge tehnike testiranja i zaštite od grešaka.
Slična greška u rukovanju pokazivačem RHS dozvoljeno u kodu ispod: V522 [CWE-476] Dereferenciranje nulte pokazivača 'RHS' može se dogoditi. TGParser.cpp 2186
Fragment N6: Upotreba pokazivača nakon kretanja
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 Upozorenje: V522 [CWE-476] Dereferenciranje nulte pokazivača 'ProgClone' može se dogoditi. Miscompilation.cpp 601
Na početku pametan pokazivač ProgClone prestaje vlasništvo nad objektom:
BD.setNewProgram(std::move(ProgClone));U stvari, sada ProgClone je nulti pokazivač. Stoga bi se nulta dereferenca pokazivača trebala pojaviti odmah ispod:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Ali, u stvarnosti, to se neće dogoditi! Imajte na umu da se petlja zapravo ne izvršava.
Na početku kontejnera MiscompiledFunctions očišćeno:
MiscompiledFunctions.clear();Zatim se veličina ovog kontejnera koristi u stanju petlje:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Lako je vidjeti da petlja ne počinje. Mislim da je i ovo greška i kod bi trebao biti napisan drugačije.
Čini se da smo naišli na taj famozni paritet grešaka! Jedna greška maskira drugu :).
Fragment N7: Upotreba pokazivača nakon kretanja
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 upozorenje: V522 [CWE-476] Dereferenciranje nultog pokazivača 'Test' može se dogoditi. Miscompilation.cpp 709
Opet ista situacija. Prvo se pomera sadržaj objekta, a zatim se koristi kao da se ništa nije dogodilo. Ovu situaciju sve češće viđam u programskom kodu nakon što se u C++ pojavila semantika pokreta. Zbog toga volim C++ jezik! Sve je više novih načina da sebi odbijete nogu. PVS-Studio analizator će uvijek imati posla :).
Fragment N8: Null pokazivač
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 upozorenje: V522 [CWE-476] Dereferenciranje nulte pokazivača 'Type' može se dogoditi. PrettyFunctionDumper.cpp 233
Osim rukovatelja greškama, funkcije ispisa za otklanjanje grešaka obično se ne testiraju. Pred nama je upravo takav slučaj. Funkcija čeka korisnika, koji će, umjesto da rješava svoje probleme, biti primoran da je popravi.
Tačno:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Null pokazivač
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 upozorenje: V522 [CWE-476] Može doći do dereferenciranja nulte pokazivača 'Ty'. SearchableTableEmitter.cpp 614
Mislim da je sve jasno i ne zahteva objašnjenje.
Fragment N10: Tipska greška
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 upozorenje: Varijabla 'Identifier->Type' je dodijeljena samoj sebi. FormatTokenLexer.cpp 249
Nema smisla dodijeliti varijablu samoj sebi. Najvjerovatnije su htjeli napisati:
Identifier->Type = Question->Type;Fragment N11: Sumnjiv prekid
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 upozorenje: [CWE-478] Razmislite o pregledu izjave 'switch'. Moguće je da nedostaje prvi operator 'case'. SystemZAsmParser.cpp 652
Na početku je vrlo sumnjiv operater pauza. Da li ste zaboravili da napišete još nešto ovde?
Fragment N12: Provjera pokazivača nakon dereferenciranja
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 upozorenje: [CWE-476] 'Callee' pokazivač je korišten prije nego što je verificiran u odnosu na nullptr. Kontrolni redovi: 172, 174. AMDGPUInline.cpp 172
Pointer Callee na početku je dereferenciran u trenutku kada je funkcija pozvana getTTI.
A onda se ispostavlja da ovaj pokazivač treba provjeriti na jednakost nullptr:
if (!Callee || Callee->isDeclaration())Ali prekasno je…
Fragment N13 - N...: Provjera pokazivača nakon dereferenciranja
Situacija o kojoj se govori u prethodnom fragmentu koda nije jedinstvena. Ovdje se pojavljuje:
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 upozorenje: V595 [CWE-476] 'CalleeFn' pokazivač je korišten prije nego što je verifikovan u odnosu na nullptr. Provjerite linije: 1079, 1081. SimplifyLibCalls.cpp 1079
i ovdje:
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 upozorenje: V595 [CWE-476] 'ND' pokazivač je korišten prije nego što je verifikovan u odnosu na nullptr. Kontrolni redovi: 532, 534. SemaTemplateInstantiateDecl.cpp 532
i ovdje:
- V595 [CWE-476] 'U' pokazivač je korišten prije nego što je verificiran u odnosu na nullptr. Kontrolni redovi: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND' pokazivač je korišten prije nego što je verificiran prema nullptr. Kontrolni redovi: 2149, 2151. SemaTemplateInstantiate.cpp 2149
A onda sam postao nezainteresovan za proučavanje upozorenja sa brojem V595. Tako da ne znam ima li još sličnih grešaka osim ovdje navedenih. Najvjerovatnije postoji.
Fragment N17, N18: Sumnjiv pomak
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}PVS-Studio upozorenje: [CWE-190] Razmislite o pregledu izraza '~(Size - 1) << 1'. Pomeranje bita 32-bitne vrednosti sa naknadnim proširenjem na 64-bitni tip. AArch64AddressingModes.h 260
Možda nije greška i kod radi tačno kako je predviđeno. Ali ovo je očigledno vrlo sumnjivo mjesto i treba ga provjeriti.
Recimo varijabla veličina je jednak 16, a onda je autor koda planirao da ga dobije u promenljivoj NImms vrednost:
1111111111111111111111111111111111111111111111111111111111100000
Međutim, u stvarnosti rezultat će biti:
0000000000000000000000000000000011111111111111111111111111100000
Činjenica je da se svi proračuni odvijaju koristeći 32-bitni neoznačeni tip. I tek tada će ovaj 32-bitni nepotpisani tip biti implicitno proširen na uint64_t. U ovom slučaju najvažniji bitovi će biti nula.
Situaciju možete popraviti ovako:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Slična situacija: V629 [CWE-190] Razmislite o pregledu izraza 'Immr << 6'. Pomeranje bita 32-bitne vrednosti sa naknadnim proširenjem na 64-bitni tip. AArch64AddressingModes.h 269
Fragment N19: Nedostaje ključna riječ drugo?
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 upozorenje: [CWE-670] Razmislite o inspekciji logike aplikacije. Moguće je da nedostaje ključna riječ 'else'. AMDGPUAsmParser.cpp 5655
Ovdje nema greške. Od tada-bloka prvog if završava sa nastaviti, onda nije bitno, postoji ključna riječ drugo ili ne. U svakom slučaju, kod će raditi isto. Još promašen drugo čini kod nejasnijim i opasnijim. Ako u budućnosti nastaviti nestane, kod će početi raditi potpuno drugačije. Po mom mišljenju bolje je dodati drugo.
Fragment N20: Četiri greške u kucanju iste vrste
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 upozorenja:
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o pregledu izraza 'Rezultat + Name.str()'. Symbol.cpp 32
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o pregledu izraza 'Rezultat + "(ObjC Class)" + Name.str()'. Symbol.cpp 35
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o pregledu izraza 'Rezultat + "(ObjC Class EH) " + Name.str()'. Symbol.cpp 38
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o pregledu izraza 'Rezultat + "(ObjC IVar)" + Name.str()'. Symbol.cpp 41
Slučajno se koristi operator + umjesto operatora +=. Rezultat su dizajni koji su lišeni značenja.
Fragment N21: Nedefinirano ponašanje
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();
}
}
}Pokušajte sami pronaći opasnu šifru. A ovo je slika za odvlačenje pažnje kako ne biste odmah pogledali odgovor:
PVS-Studio upozorenje: [CWE-758] Koristi se opasna konstrukcija: 'FeaturesMap[Op] = FeaturesMap.size()', gdje je 'FeaturesMap' klase 'map'. Ovo može dovesti do nedefinisanog ponašanja. RISCVCompressInstEmitter.cpp 490
Linija problema:
FeaturesMap[Op] = FeaturesMap.size();If element Op nije pronađen, tada se kreira novi element u mapi i tamo se upisuje broj elemenata u ovoj mapi. Samo je nepoznato da li će funkcija biti pozvana veličina prije ili nakon dodavanja novog elementa.
Fragment N22-N24: Ponovljeni zadaci
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 upozorenje: [CWE-563] Varijabli 'NType' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolne linije: 1663, 1664. MachOObjectFile.cpp 1664
Mislim da ovde nema prave greške. Samo nepotreban ponovljeni zadatak. Ali ipak greška.
Slično tome:
- V519 [CWE-563] Varijabli 'B.NDesc' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolni redovi: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Varijabli se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolni redovi: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Još preimenovanja
Pogledajmo sada malo drugačiju verziju preraspodjele.
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;
....
}Upozorenje PVS-Studio: V519 [CWE-563] Varijabli 'Alignment' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolne linije: 1158, 1160. LoadStoreVectorizer.cpp 1160
Ovo je vrlo čudan kod koji očigledno sadrži logičku grešku. Na početku, varijabilna Poravnanje vrijednost se dodjeljuje ovisno o stanju. A onda se dodjela ponovo javlja, ali sada bez ikakve provjere.
Slične situacije možete vidjeti ovdje:
- V519 [CWE-563] Varijabli 'Effects' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolni redovi: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Varijabli 'ExpectNoDerefChunk' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Kontrolni redovi: 4970, 4973. SemaType.cpp 4973
Fragment N28: Uvek istinito 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;
}
....
}PVS-Studio upozorenje: [CWE-571] Izraz 'nextByte != 0x90' je uvijek istinit. X86DisassemblerDecoder.cpp 379
Provjeravanje nema smisla. Varijabilna nextByte uvijek nije jednak vrijednosti 0x90, što slijedi iz prethodne provjere. Ovo je neka vrsta logičke greške.
Fragment N29 - N...: Uvek tačni/netačni uslovi
Analizator izdaje mnoga upozorenja da je cijelo stanje () ili njegov dio () je uvijek tačno ili netačno. Često to nisu prave greške, već jednostavno neuredan kod, rezultat proširenja makroa i slično. Međutim, ima smisla pogledati sva ova upozorenja, jer se prave logičke greške s vremena na vrijeme događaju. Na primjer, ovaj dio koda je sumnjiv:
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 upozorenje: [CWE-570] Dio uslovnog izraza je uvijek lažan: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanta 0xE je vrijednost 14 u decimalnom obliku. Ispitivanje RegNo == 0xe nema smisla jer ako RegNo > 13, tada će funkcija završiti svoje izvršavanje.
Bilo je mnogo drugih upozorenja sa ID-ovima V547 i V560, ali kao i sa , nisam bio zainteresiran za proučavanje ovih upozorenja. Već je bilo jasno da imam dovoljno materijala da napišem članak :). Stoga je nepoznato koliko grešaka ovog tipa može biti identificirano u LLVM-u pomoću PVS-Studio.
Dat ću vam primjer zašto je proučavanje ovih okidača dosadno. Analizator je potpuno u pravu kada je izdao upozorenje za sljedeći kod. Ali ovo nije greška.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Upozorenje PVS-Studio: V547 [CWE-570] Izraz '!HasError' je uvijek lažan. UnwrappedLineParser.cpp 1635
Fragment N30: Sumnjiv povratak
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 upozorenje: [CWE-670] Bezuslovni 'povratak' unutar petlje. R600OptimizeVectorRegisters.cpp 63
Ovo je ili greška ili specifična tehnika koja ima za cilj da objasni nešto programerima koji čitaju kod. Ovaj dizajn mi ništa ne objašnjava i izgleda vrlo sumnjivo. Bolje je ne pisati tako :).
Umoran? Onda je vrijeme za skuhanje čaja ili kafe.
Defekti otkriveni novom dijagnostikom
Mislim da je dovoljno 30 aktivacija stare dijagnostike. Pogledajmo sada koje se zanimljive stvari mogu pronaći s novom dijagnostikom koja se kasnije pojavila u analizatoru provjere. Tokom ovog vremena, ukupno 66 dijagnostičkih uređaja opšte namene je dodato C++ analizatoru.
Fragment N31: Nedostupan kod
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 upozorenje: [CWE-561] Otkriven nedostupan kod. Moguće je da postoji greška. ExecutionUtils.cpp 146
Kao što vidite, obje grane operatora if završava pozivom operateru povratak. Prema tome, kontejner CtorDtorsByPriority nikada neće biti očišćena.
Fragment N32: Nedostupan kod
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 upozorenje: V779 [CWE-561] Otkriven nedostupan kod. Moguće je da postoji greška. LLParser.cpp 835
Zanimljiva situacija. Pogledajmo prvo ovo mjesto:
return ParseTypeIdEntry(SummaryID);
break;Na prvi pogled se čini da tu nema greške. Izgleda kao operater pauza postoji još jedan ovdje i možete ga jednostavno izbrisati. Međutim, nije sve tako jednostavno.
Analizator izdaje upozorenje na redovima:
Lex.setIgnoreColonInIdentifiers(false);
return false;I zaista, ovaj kod je nedostižan. Svi slučajevi u prekidač završava pozivom operatera povratak. A sada besmisleno sama pauza ne izgleda tako bezazleno! Možda bi jedna od grana trebala završiti sa pauzane na povratak?
Fragment N33: Nasumično resetovanje visokih bitova
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 upozorenje: Veličina bitne maske je manja od veličine prvog operanda. To će uzrokovati gubitak viših bitova. RuntimeDyld.cpp 815
Imajte na umu da je funkcija getStubAlignment vraća tip nepotpisan. Izračunajmo vrijednost izraza, pod pretpostavkom da funkcija vraća vrijednost 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Sada primijetite da je varijabla DataSize ima 64-bitni neoznačeni tip. Ispostavilo se da će prilikom izvođenja operacije DataSize & 0xFFFFFFF8u, sva trideset dva bita visokog reda biti vraćena na nulu. Najvjerovatnije to nije ono što je programer želio. Pretpostavljam da je htio izračunati: DataSize & 0xFFFFFFFFFFFFFFF8u.
Da biste ispravili grešku, trebali biste napisati ovo:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Ili tako:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: Neuspješno prebacivanje eksplicitnog 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);
....
}PVS-Studio upozorenje: [CWE-190] Moguće prelijevanje. Razmislite o pretvaranju operanada 'NumElts * Scale' operatora na tip 'size_t', a ne rezultat. X86ISelLowering.h 1577
Eksplicitno uvođenje tipova se koristi da bi se izbjeglo prelijevanje pri množenju varijabli tipa Int. Međutim, eksplicitno uvođenje tipa ovdje ne štiti od prelijevanja. Prvo će se varijable pomnožiti, a tek onda će se 32-bitni rezultat množenja proširiti na tip .
Fragment N35: Neuspješno kopiranje i lijepljenje
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] Pronađena su dva slična fragmenta koda. Možda je ovo greška u kucanju i treba koristiti varijablu 'Op1' umjesto 'Op0'. InstCombineCompares.cpp 5507
Ova nova zanimljiva dijagnostika identifikuje situacije u kojima je deo koda kopiran i neka imena u njemu su počela da se menjaju, ali na jednom mestu to nisu ispravili.
Napominjemo da su se u drugom bloku promijenili Op0 na Op1. Ali na jednom mjestu to nisu popravili. Najvjerovatnije je trebalo ovako napisati:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Varijabilna konfuzija
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};PVS-Studio upozorenje: [CWE-563] Varijabla 'Mode' je dodijeljena, ali se ne koristi do kraja funkcije. SIModeRegister.cpp 48
Vrlo je opasno argumentima funkcije dati ista imena kao i članovima klase. Lako se zbuniti. Pred nama je upravo takav slučaj. Ovaj izraz nema smisla:
Mode &= Mask;Argument funkcije se mijenja. To je sve. Ovaj argument se više ne koristi. Najvjerovatnije ste to trebali napisati ovako:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Varijabilna konfuzija
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;
}Upozorenje PVS-Studio: V1001 [CWE-563] Varijabla 'Size' je dodijeljena, ali se ne koristi do kraja funkcije. Object.cpp 424
Situacija je slična prethodnoj. Trebalo bi da bude napisano:
this->Size += this->EntrySize;Fragment N38-N47: Zaboravili su provjeriti indeks
Prethodno smo pogledali primjere dijagnostičkog pokretanja . Njegova suština je da se pokazivač na početku dereferencira, a tek onda provjerava. Mlada dijagnostika je suprotan po značenju, ali otkriva i mnogo grešaka. Identificira situacije u kojima je pokazivač na početku provjeren, a zatim zaboravljen. Pogledajmo takve slučajeve koji se nalaze unutar LLVM-a.
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 upozorenje: V1004 [CWE-476] 'Ptr' pokazivač je korišten nesigurno nakon što je verifikovan u odnosu na nullptr. Kontrolni redovi: 729, 738. TargetTransformInfoImpl.h 738
Promjenjivo Ptr mogu biti jednaki nullptr, o čemu svjedoči provjera:
if (Ptr != nullptr)Međutim, ispod ovog pokazivača se dereferencira bez preliminarne provjere:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Razmotrimo još jedan sličan slučaj.
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 upozorenje: V1004 [CWE-476] 'FD' pokazivač je korišćen nesigurno nakon što je verifikovan u odnosu na nullptr. Kontrolni redovi: 3228, 3231. CGDebugInfo.cpp 3231
Obratite pažnju na znak FD. Siguran sam da je problem jasno vidljiv i nije potrebno posebno objašnjenje.
I dalje:
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 upozorenje: V1004 [CWE-476] 'PtrTy' pokazivač je nebezbedno korišćen nakon što je verifikovan u odnosu na nullptr. Kontrolni redovi: 960, 965. InterleavedLoadCombinePass.cpp 965
Kako se zaštititi od ovakvih grešaka? Budite pažljiviji na Code-Review i koristite PVS-Studio statički analizator da redovno provjeravate svoj kod.
Nema smisla citirati druge fragmente koda s greškama ovog tipa. U članku ću ostaviti samo listu upozorenja:
- V1004 [CWE-476] 'Expr' pokazivač je korišten nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] 'PI' pokazivač je korišten nesigurno nakon što je verificiran prema nullptr. Kontrolne linije: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Pokazivač 'StatepointCall' korišten je nesigurno nakon što je verificiran prema nullptr. Kontrolni redovi: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] 'RV' pokazivač je korišten nesigurno nakon što je provjeren u odnosu na nullptr. Kontrolne linije: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn' pokazivač je korišten nesigurno nakon što je provjeren u odnosu na nullptr. Kontrolne linije: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] 'TC' pokazivač je korišten nesigurno nakon što je provjeren u odnosu na nullptr. Kontrolni redovi: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Nije kritično, ali je kvar (moguće curenje memorije)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}PVS-Studio upozorenje: [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Strategije' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-isel-fuzzer.cpp 58
Da biste dodali element na kraj kontejnera kao što je std::vector > ne možeš samo da pišeš xxx.push_back(novi X), budući da ne postoji implicitna konverzija iz X* в std::unique_ptr.
Uobičajeno rješenje je pisanje xxx.emplace_back (novi X)pošto kompajlira: metod emplace_back konstruiše element direktno iz njegovih argumenata i stoga može koristiti eksplicitne konstruktore.
Nije sigurno. Ako je vektor pun, tada se memorija ponovo dodjeljuje. Operacija preraspodjele memorije možda neće uspjeti, što rezultira izbacivanjem izuzetka std::bad_alloc. U tom slučaju, pokazivač će biti izgubljen i kreirani objekt nikada neće biti obrisan.
Sigurno rješenje je stvaranje unique_ptrkoji će posjedovati pokazivač prije nego što vektor pokuša ponovno dodijeliti memoriju:
xxx.push_back(std::unique_ptr<X>(new X))Od C++14, možete koristiti 'std::make_unique':
xxx.push_back(std::make_unique<X>())Ova vrsta kvara nije kritična za LLVM. Ako se memorija ne može dodijeliti, kompajler će jednostavno stati. Međutim, za aplikacije sa dugim , koji se ne može samo prekinuti ako alokacija memorije ne uspije, ovo može biti prava gadna greška.
Dakle, iako ovaj kod ne predstavlja praktičnu prijetnju LLVM-u, smatrao sam korisnim govoriti o ovom obrascu greške i da je PVS-Studio analizator naučio da ga identificira.
Ostala upozorenja ove vrste:
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Prolazi' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. PassManager.h 546
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'AAs' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. AliasAnalysis.h 324
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Entries' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'AllEdges' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. CFGMST.h 268
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'VMaps' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'Records' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. FDRLogBuilder.h 30
- V1023 [CWE-460] Pointer bez vlasnika se dodaje u 'PendingSubmodules' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. ModuleMap.cpp 810
- V1023 [CWE-460] Pointer bez vlasnika se dodaje u kontejner 'Objekti' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. DebugMap.cpp 88
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Strategije' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 685
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 686
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 688
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 689
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 690
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 691
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 692
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 693
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u kontejner 'Modifikatori' metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. llvm-stress.cpp 694
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'Operands' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'Stash' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Pokazivač bez vlasnika se dodaje u 'Matchers' kontejner metodom 'emplace_back'. Doći će do curenja memorije u slučaju izuzetka. GlobalISelEmitter.cpp 2702
zaključak
Izdao sam ukupno 60 upozorenja i onda prestao. Postoje li drugi nedostaci koje PVS-Studio analizator otkriva u LLVM-u? Da imam. Međutim, kada sam ispisivao fragmente koda za članak, bilo je kasno navečer, odnosno čak noć, i odlučio sam da je vrijeme da to nazovem danom.
Nadam se da vam je bilo zanimljivo i da ćete htjeti isprobati PVS-Studio analizator.
Možete preuzeti analizator i dobiti ključ minolovca na .
Ono što je najvažnije, redovno koristite statičku analizu. Jednokratne provjere, koje sprovodimo u cilju popularizacije metodologije statičke analize i PVS-Studio nisu normalan scenario.
Sretno u poboljšanju kvalitete i pouzdanosti vašeg koda!
Ako želite da podijelite ovaj članak sa publikom koja govori engleski, koristite link za prijevod: Andrey Karpov. .
izvor: www.habr.com