Több mint két év telt el az LLVM projekt utolsó kódellenőrzése óta a PVS-Studio analizátorunk segítségével. Győződjön meg arról, hogy a PVS-Studio analizátor továbbra is vezető eszköz a hibák és a lehetséges sebezhetőségek azonosítására. Ehhez ellenőrizzük és új hibákat találunk az LLVM 8.0.0 kiadásban.
Megírandó cikk
Hogy őszinte legyek, nem akartam megírni ezt a cikket. Nem érdekes olyan projektről írni, amelyet már többször ellenőriztünk (
Minden alkalommal, amikor az LLVM új verzióját kiadják vagy frissítik
Nézd, a Clang Static Analyzer új verziója megtanulta megtalálni az új hibákat! Számomra úgy tűnik, hogy a PVS-Studio használatának jelentősége csökken. A Clang több hibát talál, mint korábban, és utoléri a PVS-Studio képességeit. Mit gondolsz erről?
Erre mindig valami ilyesmit szeretnék válaszolni:
Mi sem ülünk tétlenül! Jelentősen továbbfejlesztettük a PVS-Studio analizátor képességeit. Szóval ne aggódj, továbbra is úgy vezetünk, mint korábban.
Sajnos ez rossz válasz. Nincsenek benne bizonyítékok. És ezért írom most ezt a cikket. Tehát az LLVM projektet ismét ellenőrizték, és számos hibát találtak benne. Most bemutatom azokat, amelyek érdekesnek tűntek számomra. A Clang Static Analyzer nem találja ezeket a hibákat (vagy rendkívül kényelmetlen a segítségével). De megtehetjük. Ráadásul mindezeket a hibákat egy este alatt megtaláltam és leírtam.
De a cikk megírása több hetet vett igénybe. Egyszerűen nem tudtam rávenni magam, hogy mindezt szövegbe foglaljam :).
Egyébként, ha érdekli, hogy a PVS-Studio analizátor milyen technológiákat használ a hibák és a lehetséges sebezhetőségek azonosítására, akkor azt javaslom, hogy ismerkedjen meg ezzel.
Új és régi diagnosztika
Mint már említettük, körülbelül két évvel ezelőtt az LLVM projektet ismét ellenőrizték, és a talált hibákat kijavították. Most ez a cikk egy új hibacsomagot mutat be. Miért találtak új hibákat? Ennek 3 oka van:
- Az LLVM projekt fejlődik, megváltoztatja a régi kódot és új kódot ad hozzá. Természetesen új hibák vannak a módosított és írott kódban. Ez egyértelműen azt mutatja, hogy a statikus elemzést rendszeresen, nem pedig alkalmanként kell alkalmazni. Cikkeink jól mutatják a PVS-Studio analizátor képességeit, de ennek semmi köze a kódminőség javításához és a hibajavítás költségeinek csökkentéséhez. Rendszeresen használjon statikus kódelemzőt!
- Véglegesítjük és fejlesztjük a meglévő diagnosztikát. Ezért az analizátor képes azonosítani azokat a hibákat, amelyeket a korábbi vizsgálatok során nem vett észre.
- Új diagnosztika jelent meg a PVS-Studióban, ami 2 éve még nem létezett. Úgy döntöttem, hogy külön részben kiemelem őket, hogy jól látható legyen a PVS-Studio fejlődése.
2 évvel ezelőtti diagnosztikával azonosított hibák
N1 töredék: másolás-beillesztés
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 figyelmeztetés:
Duplán ellenőrizni kell, hogy a név az "avx512.mask.permvar." részkarakterlánccal kezdődik-e. A második ellenőrzésnél nyilván mást akartak írni, de elfelejtették kijavítani a másolt szöveget.
N2 töredék: elírás
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;
....
}
Figyelmeztetés: PVS-Studio: V501 Azonos 'CXNameRange_WantQalifier' részkifejezések találhatók a '|'-tól balra és jobbra. operátor. CIndex.cpp 7245
Elírási hiba miatt ugyanazt a konstanst kétszer használjuk CXNameRange_WantQalifier.
N3 töredék: Összetévesztés a kezelői elsőbbséggel
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio figyelmeztetés:
Véleményem szerint ez egy nagyon szép hiba. Igen, tudom, hogy furcsa elképzeléseim vannak a szépségről :).
Most szerint
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Gyakorlati szempontból egy ilyen feltételnek nincs értelme, mivel le lehet redukálni:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Ez egyértelmű hiba. Valószínűleg a 0/1-et akarták összehasonlítani egy változóval index. A kód javításához zárójeleket kell hozzáadnia a háromtagú operátorhoz:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Mellesleg a hármas operátor nagyon veszélyes és logikai hibákat vált ki. Legyen vele nagyon óvatos, és ne legyen mohó a zárójelekkel. Megnéztem ezt a témát részletesebben
N4, N5 töredék: Null mutató
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 figyelmeztetés:
Ha a mutató LHS nulla, figyelmeztetést kell kiadni. Ehelyett azonban ugyanarra a nullmutatóra hivatkozunk: LHS->getAsString().
Ez egy nagyon tipikus helyzet, amikor egy hiba rejtőzik a hibakezelőben, mivel senki sem teszteli őket. A statikus elemzők ellenőrzik az összes elérhető kódot, függetlenül attól, hogy milyen gyakran használják. Ez egy nagyon jó példa arra, hogy a statikus elemzés hogyan egészíti ki a többi tesztelési és hibavédelmi technikát.
Hasonló mutatókezelési hiba RHS az alábbi kódban engedélyezett: V522 [CWE-476] Megtörténhet az 'RHS' nullmutató hivatkozásának megszüntetése. TGParser.cpp 2186
N6 töredék: A mutató használata mozgás után
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 Figyelmeztetés: V522 [CWE-476] Megtörténhet a 'ProgClone' nullmutató hivatkozásának megszüntetése. Félrefordítás.cpp 601
Az elején egy okos mutató ProgClone megszűnik az objektum birtoklása:
BD.setNewProgram(std::move(ProgClone));
Sőt, most ProgClone egy null mutató. Ezért a nulla mutató hivatkozásának az alábbiakban kell bekövetkeznie:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
De a valóságban ez nem fog megtörténni! Vegye figyelembe, hogy a ciklus valójában nem hajtódik végre.
A tartály elején Rosszul összeállított függvények törölve:
MiscompiledFunctions.clear();
Ezután ennek a tárolónak a mérete hurokállapotban kerül felhasználásra:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Könnyen belátható, hogy a hurok nem indul el. Szerintem ez is egy bug, és a kódot másképp kellene írni.
Úgy tűnik, hogy találkoztunk a hibák híres paritásával! Egyik hiba elfedi a másikat :).
N7 töredék: A mutató használata mozgás után
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 figyelmeztetés: V522 [CWE-476] Megtörténhet a nullmutató „Teszt” hivatkozásának megszüntetése. Félrefordítás.cpp 709
Megint ugyanaz a helyzet. Először az objektum tartalmát mozgatják, majd úgy használják, mintha mi sem történt volna. Egyre gyakrabban látom ezt a helyzetet a programkódban, miután a mozgásszemantika megjelent a C++-ban. Ezért szeretem a C++ nyelvet! Egyre több új módszer létezik a saját láb lelövésére. A PVS-Studio analizátornak mindig lesz munkája :).
N8 töredék: Null mutató
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 figyelmeztetés: V522 [CWE-476] Megtörténhet a nullmutató 'Típus' hivatkozásának megszüntetése. PrettyFunctionDumper.cpp 233
A hibakezelők mellett a hibakereső nyomtatási funkciókat általában nem tesztelik. Éppen egy ilyen eset áll előttünk. A funkció a felhasználóra vár, aki ahelyett, hogy megoldaná a problémáit, kénytelen lesz kijavítani.
korrigálni:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
N9 töredék: Null mutató
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 figyelmeztetés: V522 [CWE-476] Megtörténhet a 'Ty' nullmutató hivatkozásának megszüntetése. SearchableTableEmitter.cpp 614
Szerintem minden világos és nem igényel magyarázatot.
N10 töredék: elírás
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 figyelmeztetés:
Nincs értelme változót hozzárendelni önmagához. Valószínűleg ezt akarták írni:
Identifier->Type = Question->Type;
N11 töredék: Gyanús törés
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 figyelmeztetés:
Az elején van egy nagyon gyanús operátor szünet. Elfelejtettél még valamit ide írni?
N12 töredék: A mutató ellenőrzése a hivatkozás megszüntetése után
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 figyelmeztetés:
mutató Callee Az elején a függvény hívásakor a hivatkozás le van tiltva getTTI.
És akkor kiderül, hogy ennek a mutatónak az egyenlőségét kell ellenőrizni nullptr:
if (!Callee || Callee->isDeclaration())
De már késő…
Részlet N13 - N...: Mutató ellenőrzése a hivatkozás megszüntetése után
Az előző kódrészletben tárgyalt helyzet nem egyedi. Itt jelenik meg:
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 figyelmeztetés: V595 [CWE-476] A 'CalleeFn' mutatót használták, mielőtt ellenőrizték a nullptr ellen. Ellenőrző sorok: 1079, 1081. SimplifyLibCalls.cpp 1079
És itt:
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 figyelmeztetés: V595 [CWE-476] Az 'ND' mutatót használták, mielőtt ellenőrizték a nullptr ellen. Ellenőrző sorok: 532, 534. SemaTemplateInstantiateDecl.cpp 532
És itt:
- V595 [CWE-476] Az 'U' mutatót a nullptr ellenőrzése előtt használták. Ellenőrző sorok: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Az 'ND' mutatót a nullptr ellenőrzése előtt használták. Ellenőrző sorok: 2149, 2151. SemaTemplateInstantiate.cpp 2149
És akkor nem érdekelt a V595-ös számú figyelmeztetések tanulmányozása. Így nem tudom, hogy az itt felsoroltakon kívül van-e több hasonló hiba. Valószínűleg van.
N17, N18 töredék: Gyanús elmozdulás
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio figyelmeztetés:
Lehet, hogy ez nem hiba, és a kód pontosan úgy működik, ahogy tervezték. De ez egyértelműen nagyon gyanús hely, és ellenőrizni kell.
Mondjuk a változót Méret egyenlő 16-tal, majd a kód szerzője azt tervezte, hogy egy változóban kapja meg NImms érték:
1111111111111111111111111111111111111111111111111111111111100000
A valóságban azonban az eredmény a következő lesz:
0000000000000000000000000000000011111111111111111111111111100000
Az a tény, hogy minden számítás a 32 bites előjel nélküli típus használatával történik. És csak ezután lesz ez a 32 bites előjel nélküli típus implicit módon kiterjesztve erre uint64_t. Ebben az esetben a legjelentősebb bitek nullák.
A helyzetet így javíthatja:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Hasonló helyzet: V629 [CWE-190] Fontolja meg az 'Immr << 6' kifejezés ellenőrzését. A 32 bites érték biteltolása, majd a 64 bites típusra történő bővítés. AArch64AddressingModes.h 269
Fragment N19: Hiányzó kulcsszó más?
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 figyelmeztetés:
Itt nincs hiba. Az első akkori blokkja óta if végződik folytatódik, akkor mindegy, van kulcsszó más vagy nem. A kód mindkét esetben ugyanúgy fog működni. Még mindig hiányzott más homályosabbá és veszélyesebbé teszi a kódot. Ha a jövőben folytatódik eltűnik, a kód teljesen másképp kezd el működni. Véleményem szerint jobb hozzátenni más.
N20 töredék: Négy azonos típusú elírás
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 figyelmeztetések:
- V655 [CWE-480] A karakterláncokat összefűzték, de nem használják. Fontolja meg az 'Eredmény + Name.str()' kifejezés vizsgálatát. Szimbólum.cpp 32
- V655 [CWE-480] A karakterláncokat összefűzték, de nem használják. Fontolja meg az 'Eredmény + "(ObjC osztály)" + Name.str()' kifejezés vizsgálatát. Szimbólum.cpp 35
- V655 [CWE-480] A karakterláncokat összefűzték, de nem használják. Fontolja meg a 'Result + "(ObjC Class EH)" + Name.str()' kifejezés ellenőrzését. Szimbólum.cpp 38
- V655 [CWE-480] A karakterláncokat összefűzték, de nem használják. Fontolja meg a 'Result + "(ObjC Ivar)" + Name.str()' kifejezés ellenőrzését. Szimbólum.cpp 41
Véletlenül a + operátort használják a += operátor helyett. Az eredmény olyan tervek, amelyeknek nincs értelme.
N21 töredék: Nem definiált viselkedés
static void getReqFeatures(std::map<StringRef, int> &FeaturesMap,
const std::vector<Record *> &ReqFeatures) {
for (auto &R : ReqFeatures) {
StringRef AsmCondString = R->getValueAsString("AssemblerCondString");
SmallVector<StringRef, 4> Ops;
SplitString(AsmCondString, Ops, ",");
assert(!Ops.empty() && "AssemblerCondString cannot be empty");
for (auto &Op : Ops) {
assert(!Op.empty() && "Empty operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}
Próbálja meg saját maga megtalálni a veszélyes kódot. És ez egy olyan kép, amely eltereli a figyelmet, hogy ne azonnal nézze meg a választ:
PVS-Studio figyelmeztetés:
Probléma sor:
FeaturesMap[Op] = FeaturesMap.size();
Ha elem Op nem található, akkor egy új elem jön létre a térképen, és a térképen lévő elemek száma kerül odaírásra. Csak azt nem tudni, hogy a függvény meghívásra kerül-e méret új elem hozzáadása előtt vagy után.
N22-N24 töredék: Ismételt feladatok
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 figyelmeztetés:
Nem hiszem, hogy itt valódi hiba van. Csak egy felesleges ismételt feladat. De akkor is baklövés.
Hasonlóképpen:
- V519 [CWE-563] A 'B.NDesc' változó egymás után kétszer kap értéket. Talán ez hiba. Ellenőrző sorok: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] A változó egymás után kétszer kap értéket. Talán ez hiba. Ellenőrző sorok: 59, 61. coff2yaml.cpp 61
N25-N27 töredék: További átcsoportosítások
Most nézzük az átcsoportosítás egy kicsit más változatát.
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 figyelmeztetés: V519 [CWE-563] Az „Igazítás” változó egymás után kétszer kap értéket. Talán ez hiba. Ellenőrző sorok: 1158, 1160. LoadStoreVectorizer.cpp 1160
Ez egy nagyon furcsa kód, amely látszólag logikai hibát tartalmaz. Az elején változó Alignment a feltételtől függően érték van hozzárendelve. Aztán a hozzárendelés újra megtörténik, de most minden ellenőrzés nélkül.
Hasonló helyzetek itt is láthatók:
- V519 [CWE-563] Az „Effects” változó egymás után kétszer kap értéket. Talán ez hiba. Ellenőrző sorok: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Az 'ExpectNoDerefChunk' változó egymás után kétszer kap értéket. Talán ez hiba. Ellenőrző sorok: 4970, 4973. SemaType.cpp 4973
N28-as töredék: Mindig hibátlan állapotú
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 figyelmeztetés:
Az ellenőrzésnek nincs értelme. Változó nextByte mindig nem egyenlő az értékkel 0x90, ami az előző ellenőrzésből következik. Ez valamiféle logikai hiba.
Részlet N29 - N...: Mindig igaz/hamis feltételek
Az analizátor számos figyelmeztetést ad ki, hogy a teljes állapot (
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 figyelmeztetés:
A 0xE konstans a 14 decimális érték. Vizsgálat RegNo == 0xe nincs értelme, mert ha RegNo > 13, akkor a függvény befejezi a végrehajtását.
Sok más figyelmeztetés is volt a V547 és V560 azonosítókkal, de ugyanúgy
Mondok egy példát arra, hogy miért unalmas ezeknek a triggereknek a tanulmányozása. Az analizátornak teljesen igaza van, amikor figyelmeztetést adott ki a következő kódra. De ez nem hiba.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio Figyelmeztetés: V547 [CWE-570] A '!HasError' kifejezés mindig hamis. UnwrappedLineParser.cpp 1635
N30 töredék: Gyanús visszatérés
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 figyelmeztetés:
Ez vagy egy hiba, vagy egy speciális technika, amelynek célja, hogy elmagyarázzon valamit a kódot olvasó programozóknak. Ez a kialakítás nem magyaráz meg nekem semmit, és nagyon gyanúsnak tűnik. Jobb nem így írni :).
Fáradt? Aztán itt az ideje, hogy teát vagy kávét főzzön.
Az új diagnosztikával azonosított hibák
Szerintem elég a régi diagnosztika 30 aktiválása. Lássuk most, milyen érdekességeket találhatunk az elemzőben utána megjelent új diagnosztikával
N31 töredék: Elérhetetlen kód
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 figyelmeztetés:
Mint látható, az operátor mindkét ága if a kezelő felhívásával ér véget visszatérés. Ennek megfelelően a konténer CtorDtorsByPriority soha nem lesz tisztázva.
N32 töredék: Elérhetetlen kód
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 figyelmeztetés: V779 [CWE-561] Elérhetetlen kód észlelve. Lehetséges, hogy hiba van jelen. LLParser.cpp 835
Érdekes helyzet. Nézzük először ezt a helyet:
return ParseTypeIdEntry(SummaryID);
break;
Első pillantásra úgy tűnik, hogy itt nincs hiba. Úgy néz ki, mint az operátor szünet van itt egy extra, és egyszerűen törölheti. Azonban nem minden olyan egyszerű.
Az analizátor figyelmeztetést ad a következő sorokra:
Lex.setIgnoreColonInIdentifiers(false);
return false;
És valóban, ez a kód elérhetetlen. Minden eset be kapcsoló a kezelő hívásával ér véget visszatérés. És most értelmetlen egyedül szünet nem tűnik olyan ártalmatlannak! Talán az egyik ágnak ezzel kellene végződnie szünet, nincs bekapcsolva visszatérés?
N33 töredék: Magas bitek véletlenszerű visszaállítása
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 figyelmeztetés:
Felhívjuk figyelmét, hogy a funkció getStubAlignment típust adja vissza aláírás nélküli. Számítsuk ki a kifejezés értékét, feltételezve, hogy a függvény 8-as értéket ad vissza:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Most vegyük észre, hogy a változó DataSize 64 bites előjel nélküli típusa van. Kiderült, hogy a DataSize & 0xFFFFFFF8u művelet végrehajtásakor mind a harminckét magasabb rendű bit nullára áll vissza. Valószínűleg nem ezt akarta a programozó. Gyanítom, hogy ki akarta számolni: DataSize & 0xFFFFFFFFFFFFFFFF8u.
A hiba kijavításához ezt írja be:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Vagy így:
DataSize &= ~(getStubAlignment() - 1ULL);
N34-es töredék: Sikertelen explicit típusú öntvény
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 figyelmeztetés:
Az explicit típusöntést a típusváltozók szorzásakor a túlcsordulás elkerülésére használják int. Az explicit típusú öntvény azonban itt nem véd a túlcsordulás ellen. Először a változók szorzásra kerülnek, majd csak ezután bővül ki a szorzás 32 bites eredménye a típusra.
N35 töredék: Sikertelen másolás-beillesztés
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;
}
....
}
Ez az új érdekes diagnosztika olyan helyzeteket azonosít, amikor egy kódrészletet lemásoltak, és néhány nevet elkezdtek megváltoztatni, de egy helyen nem javították ki.
Felhívjuk figyelmét, hogy a második blokkban megváltoztak Op0 on Op1. De egy helyen nem javították ki. Valószínűleg így kellett volna írni:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
N36 töredék: Változó zavartság
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio figyelmeztetés:
Nagyon veszélyes, ha a függvény argumentumainak ugyanazt a nevet adjuk, mint az osztálytagoknak. Nagyon könnyű összezavarodni. Éppen egy ilyen eset áll előttünk. Ennek a kifejezésnek nincs értelme:
Mode &= Mask;
A függvény argumentuma megváltozik. Ez minden. Ezt az érvet már nem használják. Valószínűleg így kellett volna írnod:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
N37 töredék: Változó zavartság
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;
}
Figyelmeztetés: PVS-Studio: V1001 [CWE-563] A 'Size' változó hozzá van rendelve, de a függvény végére nem kerül felhasználásra. Object.cpp 424
A helyzet hasonló az előzőhöz. Azt kellene írni:
this->Size += this->EntrySize;
N38-N47 töredék: Elfelejtették ellenőrizni az indexet
Korábban a diagnosztikai kiváltási példákat néztük meg
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 figyelmeztetés: V1004 [CWE-476] A 'Ptr' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 729, 738. TargetTransformInfoImpl.h 738
Változó Ptr egyenlő lehet nullptr, amint azt a csekk is bizonyítja:
if (Ptr != nullptr)
Ez alatt a mutató alatt azonban előzetes ellenőrzés nélkül nem hivatkozunk:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Nézzünk egy másik hasonló esetet.
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 figyelmeztetés: V1004 [CWE-476] Az 'FD' mutatót nem biztonságosan használták, miután ellenőrizték, hogy nullptr. Ellenőrző sorok: 3228, 3231. CGDebugInfo.cpp 3231
Ügyeljen a jelre FD. Biztos vagyok benne, hogy a probléma jól látható, és nincs szükség különösebb magyarázatra.
És tovább:
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 figyelmeztetés: V1004 [CWE-476] A 'PtrTy' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr ellen. Ellenőrző sorok: 960, 965. InterleavedLoadCombinePass.cpp 965
Hogyan védheti meg magát az ilyen hibáktól? Legyen figyelmesebb a Code-Review-nál, és használja a PVS-Studio statikus elemzőt a kód rendszeres ellenőrzéséhez.
Nincs értelme más kódrészletekre hivatkozni, amelyek ilyen típusú hibákat tartalmaznak. Csak a figyelmeztetések listáját hagyom meg a cikkben:
- V1004 [CWE-476] Az 'Expr' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] A 'PI' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] A 'StatepointCall' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Az 'RV' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] A 'CalleeFn' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] A 'TC' mutatót nem biztonságosan használták, miután ellenőrizték a nullptr-el szemben. Ellenőrző sorok: 1819, 1824. Driver.cpp 1824
N48-N60 töredék: Nem kritikus, de hiba (lehetséges memóriaszivárgás)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio figyelmeztetés:
Egy elem hozzáadásához egy tároló végéhez pl std::vektor > nem lehet csak úgy írni xxx.push_back(új X), mivel nincs implicit konverzió X* в std::unique_ptr.
Gyakori megoldás az írás xxx.emplace_back(új X)mivel összeállítja: módszer emplace_back közvetlenül az argumentumaiból épít fel egy elemet, ezért használhat explicit konstruktorokat.
Nem biztonságos. Ha a vektor megtelt, a memória újra lefoglalható. Előfordulhat, hogy a memória-újrafoglalási művelet meghiúsul, ami kivételt eredményezhet std::bad_alloc. Ebben az esetben a mutató elvész, és a létrehozott objektum soha nem törlődik.
Biztonságos megoldás az alkotás egyedi_ptramely birtokolja a mutatót, mielőtt a vektor megpróbálja újra lefoglalni a memóriát:
xxx.push_back(std::unique_ptr<X>(new X))
A C++14 óta használhatja az 'std::make_unique'-t:
xxx.push_back(std::make_unique<X>())
Ez a fajta hiba nem kritikus az LLVM számára. Ha a memória nem foglalható le, a fordító egyszerűen leáll. Azonban a hosszú
Tehát bár ez a kód nem jelent gyakorlati veszélyt az LLVM-re, hasznosnak találtam beszélni erről a hibamintáról, és arról, hogy a PVS-Studio analizátor megtanulta azonosítani.
Egyéb ilyen típusú figyelmeztetések:
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a „Passes” tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. PassManager.h 546
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül az 'AAs' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. AliasAnalysis.h 324
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül az "Entries" tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadódik az 'AllEdges' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. CFGMST.h 268
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal kerül hozzáadásra a „VMaps” tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal kerül hozzáadásra a „Records” tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. FDRLogBuilder.h 30
- V1023 [CWE-460] A „PendingSubmodules” tárolóhoz az „emplace_back” metódussal egy tulajdonos nélküli mutató kerül hozzáadásra. Kivétel esetén memóriaszivárgás lép fel. ModuleMap.cpp 810
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadódik az 'Objects' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. DebugMap.cpp 88
- V1023 [CWE-460] Az 'emplace_back' metódussal egy tulajdonos nélküli mutató kerül a 'Strategies' tárolóba. Kivétel esetén memóriaszivárgás lép fel. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 685
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 686
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 688
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 689
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 690
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 691
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 692
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 693
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadásra kerül a 'Módosítók' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. llvm-stress.cpp 694
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadódik az 'Operandusok' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] A tulajdonos nélküli mutató az 'emplace_back' metódussal hozzáadódik a „Stash” tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Egy tulajdonos nélküli mutató az 'emplace_back' metódussal kerül hozzáadásra a 'Matchers' tárolóhoz. Kivétel esetén memóriaszivárgás lép fel. GlobalISelEmitter.cpp 2702
Következtetés
Összesen 60 figyelmeztetést adtam ki, majd abbahagytam. Vannak más hibák, amelyeket a PVS-Studio analizátor észlel az LLVM-ben? Igen van. Amikor azonban kódrészleteket írtam ki a cikkhez, késő este volt, vagy inkább még éjszaka, és úgy döntöttem, ideje napnak nevezni.
Remélem, érdekesnek találtad, és ki szeretnéd próbálni a PVS-Studio analizátort.
Letöltheti az elemzőt és megszerezheti az aknakereső kulcsát a címen
A legfontosabb, hogy rendszeresen használjon statikus elemzést. Egyszeri ellenőrzések, amit a statikus elemzés módszertanának népszerűsítése érdekében végeztünk és a PVS-Studio nem egy normális forgatókönyv.
Sok sikert a kód minőségének és megbízhatóságának javításához!
Ha meg szeretné osztani ezt a cikket egy angolul beszélő közönséggel, kérjük, használja a fordítási linket: Andrey Karpov.
Forrás: will.com