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 prepoznavanje pogrešaka i potencijalnih ranjivosti. Da bismo to učinili, provjerit ćemo i pronaći nove pogreške u izdanju LLVM 8.0.0.
Članak koji treba napisati
Da budem iskren, nisam htio napisati ovaj članak. Nije zanimljivo pisati o projektu koji smo već nekoliko puta provjerili (, , ). Bolje je pisati o nečem novom, ali nemam izbora.
Svaki put kada se izda ili ažurira nova verzija LLVM-a , na našu e-poštu primamo pitanja sljedeće vrste:
Pogledajte, nova verzija Clang Static Analyzera naučila je pronaći nove pogreške! Čini mi se da je relevantnost korištenja PVS-Studia sve manja. Clang pronalazi više pogrešaka nego prije i sustiže mogućnosti PVS-Studia. Što misliš o ovome?
Na ovo uvijek želim odgovoriti nešto poput:
Ni mi ne sjedimo besposleni! Značajno smo poboljšali mogućnosti analizatora PVS-Studio. Dakle, ne brinite, nastavljamo voditi kao i prije.
Nažalost, ovo je loš odgovor. U njemu nema dokaza. I zato sada pišem ovaj članak. Dakle, projekt LLVM još jednom je provjeren i u njemu su pronađene razne greške. Sada ću pokazati one koji su mi se učinili zanimljivima. Clang Static Analyzer ne može pronaći te pogreške (ili je to vrlo nezgodno učiniti uz njegovu pomoć). Ali možemo. Štoviše, sve sam te pogreške pronašao i zapisao u jednoj večeri.
Ali pisanje članka trajalo je nekoliko tjedana. Jednostavno se nisam mogla natjerati da sve ovo pretočim u tekst :).
Usput, ako vas zanima koje se tehnologije koriste u analizatoru PVS-Studio za prepoznavanje pogrešaka i potencijalnih ranjivosti, predlažem da se upoznate s ovim .
Nova i stara dijagnostika
Kao što je već navedeno, prije otprilike dvije godine LLVM projekt je još jednom provjeren, te su pronađene pogreške ispravljene. Sada će ovaj članak predstaviti novu seriju pogrešaka. Zašto su pronađeni novi bugovi? Postoje 3 razloga za to:
- Projekt LLVM se razvija, mijenja stari kod i dodaje novi kod. Naravno, postoje nove greške u modificiranom i napisanom kodu. Ovo jasno pokazuje da se statička analiza treba koristiti redovito, a ne povremeno. Naši članci dobro pokazuju mogućnosti analizatora PVS-Studio, ali to nema nikakve veze s poboljšanjem kvalitete koda i smanjenjem troškova ispravljanja pogrešaka. Redovito koristite statički analizator koda!
- Dovršavamo i poboljšavamo postojeću dijagnostiku. Stoga analizator može identificirati greške koje nije primijetio tijekom prethodnih skeniranja.
- U PVS-Studiu se pojavila nova dijagnostika koja nije postojala prije 2 godine. Odlučio sam ih istaknuti u zasebnom odjeljku kako bih jasno pokazao razvoj PVS-Studia.
Nedostaci 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
....
}Upozorenje PVS-Studio: [CWE-570] Postoje identični podizrazi 'Name.startswith("avx512.mask.permvar.")' lijevo i desno od '||' operater. AutoUpgrade.cpp 73
Duplo se provjerava da naziv počinje podnizom "avx512.mask.permvar.". U drugoj provjeri očito su htjeli nešto drugo napisati, ali su zaboravili ispraviti ispisani tekst.
Fragment N2: Tipfeler
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 tipfelera, ista imenovana konstanta korištena je dva puta CXNameRange_WantQualifier.
Fragment N3: Zabuna s prvenstvom operatora
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Upozorenje PVS-Studio: [CWE-783] Možda operator '?:' radi na drugačiji način od očekivanog. Operator '?:' ima niži prioritet od operatora '=='. PPCTargetTransformInfo.cpp 404
Po mom mišljenju, ovo je vrlo lijepa pogreška. Da, znam da imam čudne ideje o ljepoti :).
Sada, prema , izraz se vrednuje na sljedeći način:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0S praktičnog stajališta takav uvjet nema smisla jer se može svesti na:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Ovo je očita pogreška. Najvjerojatnije su htjeli usporediti 0/1 s varijablom indeks. Za popravak koda morate dodati zagrade oko ternarnog operatora:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Usput, ternarni operator je vrlo opasan i izaziva logičke pogreške. Budite vrlo oprezni s tim i nemojte biti pohlepni sa zagradama. Pogledao sam ovu temu detaljnije , u poglavlju “Čuvajte se ?: Operator i stavite ga u zagrade.”
Ulomak N4, N5: Nul 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;
}
....
}Upozorenje PVS-Studio: [CWE-476] Može doći do dereferenciranja nultog pokazivača 'LHS'. TGParser.cpp 2152
Ako pokazivač LHS je ništavan, treba izdati upozorenje. Međutim, umjesto toga, isti će nulti pokazivač biti dereferenciran: LHS->getAsString().
Ovo je vrlo tipična situacija kada se greška krije u rukovatelju greškama, jer ih nitko ne testira. Statički analizatori provjeravaju sav dohvatljiv kod, bez obzira koliko se često koristio. Ovo je vrlo dobar primjer kako statička analiza nadopunjuje druga ispitivanja i tehnike zaštite od pogrešaka.
Slična pogreška rukovanja pokazivačem RHS dopušteno u kodu ispod: V522 [CWE-476] Može doći do dereferenciranja nultog pokazivača 'RHS'. TGParser.cpp 2186
Fragment N6: Korištenje pokazivača nakon pomicanja
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] Može doći do dereferenciranja nultog pokazivača 'ProgClone'. Pogrešna kompilacija.cpp 601
Na početku pametna kazaljka ProgClone prestaje vlasništvo nad objektom:
BD.setNewProgram(std::move(ProgClone));Zapravo, sada ProgClone je nulti pokazivač. Stoga bi se dereferencija nultog pokazivača trebala pojaviti odmah ispod:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);No, u stvarnosti se to neće dogoditi! Imajte na umu da se petlja zapravo ne izvršava.
Na početku kontejnera Pogrešno kompilirane funkcije očišćeno:
MiscompiledFunctions.clear();Zatim se veličina ovog spremnika koristi u stanju petlje:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {Lako je vidjeti da se petlja ne pokreće. Mislim da je i ovo greška i kod bi trebao biti drugačije napisan.
Čini se da smo naišli na taj famozni paritet grešaka! Jedna greška prikriva drugu :).
Fragment N7: Korištenje pokazivača nakon pomicanja
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;
}
....
}Upozorenje PVS-Studio: V522 [CWE-476] Može doći do dereferenciranja nultog pokazivača 'Test'. Pogrešna kompilacija.cpp 709
Opet ista situacija. Prvo se sadržaj objekta pomiče, a zatim se koristi kao da se ništa nije dogodilo. Ovu situaciju sve češće vidim u programskom kodu nakon što se semantika kretanja pojavila u C++. Zbog toga volim jezik C++! Sve je više novih načina da odbijete vlastitu nogu. PVS-Studio analizator će uvijek imati posla :).
Fragment N8: Nul 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);
}Upozorenje PVS-Studio: V522 [CWE-476] Može doći do dereferenciranja nultog pokazivača 'Vrsta'. PrettyFunctionDumper.cpp 233
Osim rukovatelja pogreškama, funkcije ispisa za otklanjanje pogrešaka obično se ne testiraju. Pred nama je upravo takav slučaj. Funkcija čeka korisnika, koji će umjesto rješavanja svojih problema biti prisiljen popraviti je.
ispraviti:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: Nul 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());
....
}Upozorenje PVS-Studio: V522 [CWE-476] Može doći do dereferenciranja nultog pokazivača 'Ty'. SearchableTableEmitter.cpp 614
Mislim da je sve jasno i ne zahtijeva objašnjenje.
Fragment N10: Tipfeler
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;
}Upozorenje PVS-Studio: Varijabla 'Identifier->Type' dodijeljena je sama sebi. FormatTokenLexer.cpp 249
Nema smisla dodjeljivati varijablu samoj sebi. Najvjerojatnije su htjeli napisati:
Identifier->Type = Question->Type;Ulomak N11: Sumnjivi lom
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Upozorenje PVS-Studio: [CWE-478] Razmislite o provjeri naredbe 'switch'. Moguće je da prvi 'case' operator nedostaje. SystemZAsmParser.cpp 652
Na početku je vrlo sumnjiv operater razbiti. Jeste li zaboravili napisati još nešto ovdje?
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");
....
}Upozorenje PVS-Studio: [CWE-476] Pokazivač 'Callee' korišten je prije nego što je provjeren u odnosu na nullptr. Provjerite linije: 172, 174. AMDGPUInline.cpp 172
pokazivač Callee na početku se dereferencira u trenutku pozivanja funkcije getTTI.
I onda se ispostavi da ovaj pokazivač treba provjeriti na jednakost nullptr:
if (!Callee || Callee->isDeclaration())Ali prekasno je…
Fragment N13 - N...: Provjera pokazivača nakon dereferenciranja
Situacija opisana 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()) { // <=
....
}Upozorenje PVS-Studio: V595 [CWE-476] Pokazivač 'CalleeFn' korišten je prije nego što je provjeren 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()); // <=
....
}Upozorenje PVS-Studio: V595 [CWE-476] Pokazivač 'ND' korišten je prije nego što je provjeren u odnosu na nullptr. Provjerite retke: 532, 534. SemaTemplateInstantiateDecl.cpp 532
I ovdje:
- V595 [CWE-476] Pokazivač 'U' korišten je prije nego što je provjeren u odnosu na nullptr. Provjerite retke: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Pokazivač 'ND' korišten je prije nego što je provjeren u odnosu na nullptr. Provjerite retke: 2149, 2151. SemaTemplateInstantiate.cpp 2149
A onda sam postao nezainteresiran za proučavanje upozorenja s brojem V595. Tako da ne znam ima li još sličnih grešaka osim ovdje navedenih. Najvjerojatnije postoji.
Ulomak 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;
....
}Upozorenje PVS-Studio: [CWE-190] Razmislite o provjeri izraza '~(Size - 1) << 1'. Bitni pomak 32-bitne vrijednosti s naknadnim proširenjem na 64-bitni tip. AArch64AddressingModes.h 260
Možda nije greška i kôd radi točno onako kako je zamišljeno. Ali ovo je očito vrlo sumnjivo mjesto i treba ga provjeriti.
Recimo varijabla Veličina je jednak 16, a onda je autor koda planirao da ga dobije u varijabli NImms vrijednost:
1111111111111111111111111111111111111111111111111111111111100000
Međutim, u stvarnosti će rezultat biti:
0000000000000000000000000000000011111111111111111111111111100000
Činjenica je da se svi izračuni odvijaju korištenjem 32-bitnog tipa bez predznaka. I tek tada će se ovaj 32-bitni tip bez predznaka implicitno proširiti na uint64_t. U ovom slučaju, najvažniji bitovi bit će nula.
Situaciju možete popraviti ovako:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Slična situacija: V629 [CWE-190] Razmislite o provjeri izraza 'Immr << 6'. Bitni pomak 32-bitne vrijednosti s 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");
}
....
}Upozorenje PVS-Studio: [CWE-670] Razmislite o provjeri logike aplikacije. Moguće je da nedostaje ključna riječ 'else'. AMDGPUAsmParser.cpp 5655
Ovdje nema greške. Od tadašnjeg bloka prve if završava sa nastaviti, onda nema veze, postoji ključna riječ drugo ili ne. U svakom slučaju kod će raditi isto. Još uvijek propušteno drugo čini kôd 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.
Ulomak N20: Četiri istovrsne tiskarske pogreške
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 provjeri izraza "Rezultat + naziv.str()". Simbol.cpp 32
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o provjeri izraza 'Rezultat + "(ObjC klasa)" + Name.str()'. Simbol.cpp 35
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o provjeri izraza 'Rezultat + "(ObjC klasa EH) " + Name.str()'. Simbol.cpp 38
- V655 [CWE-480] Nizovi su spojeni, ali se ne koriste. Razmislite o provjeri izraza 'Rezultat + "(ObjC IVar)" + Name.str()'. Simbol.cpp 41
Slučajno se koristi operator + umjesto operatora +=. Rezultat su dizajni koji su lišeni smisla.
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 da ne pogledate odmah odgovor:
Upozorenje PVS-Studio: [CWE-758] Koristi se opasna konstrukcija: 'FeaturesMap[Op] = FeaturesMap.size()', gdje je 'FeaturesMap' klase 'map'. To može dovesti do nedefiniranog ponašanja. RISCVCompressInstEmitter.cpp 490
Problemska linija:
FeaturesMap[Op] = FeaturesMap.size();Ako element Op nije pronađen, tada se u mapi stvara novi element i tamo se upisuje broj elemenata u ovoj mapi. Samo je nepoznato hoće li funkcija biti pozvana Veličina prije ili nakon dodavanja novog elementa.
Ulomak 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;
}
....
}Upozorenje PVS-Studio: [CWE-563] Varijabli 'NType' dodjeljuju se vrijednosti dvaput uzastopno. Možda je ovo greška. Provjerite linije: 1663, 1664. MachOObjectFile.cpp 1664
Mislim da ovdje nema prave greške. Samo nepotrebno ponavljanje zadatka. Ali ipak greška.
Također:
- V519 [CWE-563] Varijabli 'B.NDesc' dodjeljuju se vrijednosti dvaput uzastopno. Možda je ovo greška. Provjerite linije: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Varijabli se dodjeljuju vrijednosti dva puta uzastopno. Možda je ovo greška. Provjerite linije: 59, 61. coff2yaml.cpp 61
Ulomak N25-N27: Više preraspodjela
Sada pogledajmo nešto 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;
....
}PVS-Studio upozorenje: V519 [CWE-563] Varijabli 'Alignment' se dodjeljuju vrijednosti dvaput uzastopno. Možda je ovo greška. Provjerite linije: 1158, 1160. LoadStoreVectorizer.cpp 1160
Ovo je vrlo čudan kod koji očito sadrži logičku pogrešku. U početku promjenjivo Poravnanje vrijednost se dodjeljuje ovisno o stanju. I onda se ponovno javlja dodjela, ali sada bez ikakve provjere.
Slične situacije možete vidjeti ovdje:
- V519 [CWE-563] Varijabli 'Effects' se dodjeljuju vrijednosti dva puta uzastopno. Možda je ovo greška. Provjerite retke: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Varijabli 'ExpectNoDerefChunk' dodjeljuju se vrijednosti dvaput uzastopno. Možda je ovo greška. Provjerite linije: 4970, 4973. SemaType.cpp 4973
Ulomak N28: Uvijek 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;
}
....
}Upozorenje PVS-Studio: [CWE-571] Izraz 'nextByte != 0x90' je uvijek istinit. X86DisassemblerDecoder.cpp 379
Provjeravanje nema smisla. Varijabilna nextByte uvijek nije jednaka vrijednosti 0x90, što proizlazi iz prethodne provjere. Ovo je neka vrsta logičke pogreške.
Fragment N29 - N...: Uvijek istiniti/netočni uvjeti
Analizator izdaje mnoga upozorenja da je cjelokupno stanje () ili njegov dio () uvijek je istinito ili lažno. Često se ne radi o stvarnim pogreškama, već jednostavno o neurednom kodu, rezultatu makro ekspanzije i slično. Međutim, ima smisla pogledati sva ova upozorenja, budući da se s vremena na vrijeme događaju prave logičke pogreške. 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;
....
}Upozorenje PVS-Studio: [CWE-570] Dio uvjetnog izraza uvijek je lažan: RegNo == 0xe. ARMDisassembler.cpp 939
Konstanta 0xE je vrijednost 14 u decimalnom obliku. Ispitivanje Reg. broj == 0xe nema smisla jer ako Reg br > 13, tada će funkcija završiti svoje izvršenje.
Bilo je mnogo drugih upozorenja s ID-ovima V547 i V560, ali kao i kod , nisam bio zainteresiran za proučavanje ovih upozorenja. Već je bilo jasno da imam dovoljno materijala za pisanje članka :). Stoga je nepoznato koliko se grešaka ovog tipa može identificirati u LLVM-u pomoću PVS-Studio.
Dat ću vam primjer zašto je proučavanje ovih okidača dosadno. Analizator je apsolutno u pravu kada izdaje upozorenje za sljedeći kôd. 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();
}
....
}Upozorenje PVS-Studio: [CWE-670] Bezuvjetni 'povratak' unutar petlje. R600OptimizeVectorRegisters.cpp 63
Ovo je ili pogreška ili posebna tehnika koja ima za cilj objasniti nešto programerima koji čitaju kod. Ovaj dizajn mi ništa ne objašnjava i djeluje vrlo sumnjivo. Bolje da ne pisem tako :).
Umoran? Tada je vrijeme za kuhanje čaja ili kave.
Kvarovi utvrđeni novom dijagnostikom
Mislim da je dovoljno 30 aktivacija stare dijagnostike. Pogledajmo sada što se zanimljivo može pronaći s novom dijagnostikom koja se pojavila u analizatoru nakon provjere. Tijekom tog vremena C++ analizatoru dodano je ukupno 66 dijagnostika opće namjene.
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();
}Upozorenje PVS-Studio: [CWE-561] Otkriven nedostupan kod. Moguće je da postoji greška. ExecutionUtils.cpp 146
Kao što vidite, obje grane operatera if završava pozivom operateru povratak. Sukladno tome, spremnik 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;
}Upozorenje PVS-Studio: 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 čini se da tu nema greške. Izgleda kao operater razbiti ovdje postoji jedan dodatni i možete ga jednostavno izbrisati. Međutim, nije sve tako jednostavno.
Analizator izdaje upozorenje u redovima:
Lex.setIgnoreColonInIdentifiers(false);
return false;I doista, ovaj kod je nedostupan. Svi slučajevi u prebaciti završava pozivom operatera povratak. A sada sam besmislen razbiti ne izgleda tako bezopasno! Možda bi jedna od grana trebala završiti s razbiti, ne na povratak?
Fragment N33: Nasumično ponovno postavljanje 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);
....
}Upozorenje PVS-Studio: Veličina maske bita manja je od veličine prvog operanda. To će uzrokovati gubitak viših bitova. RuntimeDyld.cpp 815
Napominjemo da 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 varijabla Veličina podataka ima 64-bitni tip bez predznaka. Ispada da će se prilikom izvođenja operacije DataSize & 0xFFFFFFF8u sva trideset i dva bita višeg reda vratiti na nulu. Najvjerojatnije to nije ono što je programer želio. Sumnjam da je htio izračunati: DataSize & 0xFFFFFFFFFFFFFF8u.
Da biste ispravili grešku, trebali biste napisati ovo:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);Ili:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: neuspješno eksplicitno pretvaranje 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);
....
}Upozorenje PVS-Studio: [CWE-190] Moguće prelijevanje. Razmislite o pretvaranju operanda operatora 'NumElts * Scale' u tip 'size_t', a ne u rezultat. X86ISelLowering.h 1577
Eksplicitno pretvaranje tipa koristi se kako bi se izbjeglo prelijevanje pri množenju varijabli tipa int. Međutim, eksplicitno pretvaranje tipa ovdje ne štiti od preljeva. Prvo će se varijable umnožiti, a tek onda će se 32-bitni rezultat množenja proširiti na tip .
Fragment N35: Neuspjelo kopiranje-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 pri upisu i varijablu 'Op1' treba koristiti umjesto 'Op0'. InstCombineCompares.cpp 5507
Ova nova zanimljiva dijagnostika identificira situacije u kojima je dio koda kopiran i neka su se imena u njemu počela mijenjati, ali na jednom mjestu to nisu ispravili.
Napominjemo da su se u drugom bloku promijenili Op0 na Op1. Ali na jednom mjestu to nisu popravili. Najvjerojatnije je trebalo ovako pisati:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Varijabilna zabuna
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Upozorenje PVS-Studio: [CWE-563] Varijabla 'Mode' je dodijeljena, ali se ne koristi do kraja funkcije. SIModeRegister.cpp 48
Vrlo je opasno dati argumentima funkcije ista imena kao članovima klase. Vrlo se lako zbuniti. Pred nama je upravo takav slučaj. Ovaj izraz nema smisla:
Mode &= Mask;Argument funkcije se mijenja. To je sve. Ovaj se argument više ne koristi. Najvjerojatnije ste to trebali napisati ovako:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Varijabilna zabuna
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. Objekt.cpp 424
Situacija je slična prethodnoj. Trebalo bi napisati:
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 suprotnog značenja, ali također otkriva puno pogrešaka. Identificira situacije u kojima je pokazivač provjeren na početku, a zatim zaboravljen. Pogledajmo takve slučajeve 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] Pokazivač 'Ptr' korišten je nesigurno nakon što je provjeren protiv nullptr. Provjerite linije: 729, 738. TargetTransformInfoImpl.h 738
Promjenjiva Ptr mogu biti jednaki nullptr, što dokazuje ček:
if (Ptr != nullptr)Međutim, ispod ovog pokazivača dereferencira se bez prethodne 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(); // <=
....
}Upozorenje PVS-Studio: V1004 [CWE-476] Pokazivač 'FD' korišten je nesigurno nakon što je provjeren protiv nullptr. Provjerite linije: 3228, 3231. CGDebugInfo.cpp 3231
Obratite pozornost na znak FD. Siguran sam da je problem jasno vidljiv i da 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()); // <=
....
}Upozorenje PVS-Studio: V1004 [CWE-476] Pokazivač 'PtrTy' korišten je nesigurno nakon što je provjeren protiv nullptr. Provjerite linije: 960, 965. InterleavedLoadCombinePass.cpp 965
Kako se zaštititi od takvih grešaka? Budite pažljiviji na Code-Review i koristite PVS-Studio statički analizator za redovitu provjeru koda.
Nema smisla citirati druge fragmente koda s greškama ove vrste. U članku ću ostaviti samo popis upozorenja:
- V1004 [CWE-476] Pokazivač 'Expr' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite retke: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Pokazivač 'PI' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Pokazivač 'StatepointCall' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Pokazivač 'RV' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Pokazivač 'CalleeFn' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Pokazivač 'TC' korišten je nesigurno nakon što je provjeren u odnosu na nullptr. Provjerite linije: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Nije kritično, ali kvar (moguće curenje memorije)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Upozorenje PVS-Studio: [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Strategije' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-isel-fuzzer.cpp 58
Za dodavanje elementa na kraj spremnika poput std::vektor > ne možeš samo pisati xxx.push_back(novi X), budući da nema implicitne konverzije iz X* в std::unique_ptr.
Uobičajeno rješenje je pisanje xxx.emplace_back(novi X)budući da sastavlja: metodu emplace_back konstruira element izravno iz argumenata i stoga može koristiti eksplicitne konstruktore.
Nije sigurno. Ako je vektor pun, tada se memorija ponovno dodjeljuje. Operacija preraspodjele memorije možda neće uspjeti, što će rezultirati izbacivanjem iznimke std::bad_alloc. U tom slučaju, pokazivač će biti izgubljen i stvoreni objekt nikada neće biti izbrisan.
Sigurno rješenje je stvarati jedinstveni_ptrkoji će posjedovati pokazivač prije nego što vektor pokuša preraspodijeliti 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 se jednostavno zaustaviti. Međutim, za aplikacije s dugim , koji se ne može samo prekinuti ako dodjela memorije ne uspije, ovo može biti stvarno gadna pogreška.
Dakle, iako ovaj kod ne predstavlja praktičnu prijetnju LLVM-u, smatrao sam korisnim govoriti o ovom obrascu pogreške i da ga je PVS-Studio analizator naučio identificirati.
Ostala upozorenja ove vrste:
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Prolazi' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. PassManager.h 546
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u 'AAs' spremnik metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. AliasAnalysis.h 324
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Entries' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'AllEdges' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. CFGMST.h 268
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'VMaps' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Zapisi' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. FDRLogBuilder.h 30
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'PendingSubmodules' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. ModuleMap.cpp 810
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Objekti' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. DebugMap.cpp 88
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Strategije' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 685
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 686
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 688
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 689
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 690
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 691
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 692
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 693
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Modifikatori' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. llvm-stres.cpp 694
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Operandi' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Stash' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Pokazivač bez vlasnika dodaje se u spremnik 'Matchers' metodom 'emplace_back'. U slučaju iznimke dogodit će se curenje memorije. GlobalISelEmitter.cpp 2702
Zaključak
Ukupno sam izrekao 60 opomena i onda stao. Postoje li drugi nedostaci koje PVS-Studio analizator otkriva u LLVM-u? Da imam. Međutim, kada sam ispisivao fragmente koda za članak, bila je kasna večer, točnije čak noć, i odlučio sam da je vrijeme da završim.
Nadam se da vam je bilo zanimljivo i da ćete htjeti isprobati PVS-Studio analizator.
Možete preuzeti analizator i dobiti ključ minolovca na .
Što je najvažnije, redovito koristite statičku analizu. Jednokratni čekovi, koje provodimo kako bismo popularizirali metodologiju statičke analize i PVS-Studio nisu normalan scenarij.
Sretno u poboljšanju kvalitete i pouzdanosti vašeg koda!
Ako želite podijeliti ovaj članak s publikom koja govori engleski, upotrijebite vezu za prijevod: Andrey Karpov. .
Izvor: www.habr.com