Kanë kaluar më shumë se dy vjet nga kontrolli i fundit i kodit të projektit LLVM duke përdorur analizuesin tonë PVS-Studio. Le të sigurohemi që analizuesi PVS-Studio është ende një mjet kryesor për identifikimin e gabimeve dhe dobësive të mundshme. Për ta bërë këtë, ne do të kontrollojmë dhe gjejmë gabime të reja në lëshimin e LLVM 8.0.0.
Artikull për t'u shkruar
Për të qenë i sinqertë, nuk doja ta shkruaja këtë artikull. Nuk është interesante të shkruash për një projekt që ne e kemi kontrolluar tashmë disa herë (
Sa herë që lëshohet ose përditësohet një version i ri i LLVM
Shikoni, versioni i ri i Clang Static Analyzer ka mësuar të gjejë gabime të reja! Më duket se rëndësia e përdorimit të PVS-Studio po zvogëlohet. Clang gjen më shumë gabime se më parë dhe arrin me aftësitë e PVS-Studio. Cfare mendon per kete?
Për këtë unë gjithmonë dua të përgjigjem diçka si:
As ne nuk rrimë kot! Ne kemi përmirësuar ndjeshëm aftësitë e analizuesit PVS-Studio. Pra, mos u shqetësoni, ne vazhdojmë të udhëheqim si më parë.
Fatkeqësisht, kjo është një përgjigje e keqe. Nuk ka prova në të. Dhe kjo është arsyeja pse po shkruaj këtë artikull tani. Pra, projekti LLVM është kontrolluar edhe një herë dhe në të janë gjetur një sërë gabimesh. Tani do të demonstroj ato që më dukeshin interesante. Clang Static Analyzer nuk mund t'i gjejë këto gabime (ose është jashtëzakonisht e papërshtatshme ta bësh këtë me ndihmën e tij). Por ne mundemi. Për më tepër, i gjeta dhe i shkrova të gjitha këto gabime në një mbrëmje.
Por shkrimi i artikullit zgjati disa javë. Thjesht nuk mund ta përballoja veten për t'i dhënë të gjitha këto në tekst :).
Nga rruga, nëse jeni të interesuar se cilat teknologji përdoren në analizuesin PVS-Studio për të identifikuar gabimet dhe dobësitë e mundshme, atëherë unë sugjeroj të njiheni me këtë
Diagnostifikimi i ri dhe i vjetër
Siç u përmend tashmë, rreth dy vjet më parë projekti LLVM u kontrollua edhe një herë, dhe gabimet e gjetura u korrigjuan. Tani ky artikull do të paraqesë një grup të ri gabimesh. Pse u gjetën defekte të reja? Ka 3 arsye për këtë:
- Projekti LLVM po evoluon, ndryshon kodin e vjetër dhe shton kod të ri. Natyrisht, ka gabime të reja në kodin e modifikuar dhe të shkruar. Kjo tregon qartë se analiza statike duhet të përdoret rregullisht, dhe jo herë pas here. Artikujt tanë tregojnë mirë aftësitë e analizuesit PVS-Studio, por kjo nuk ka të bëjë fare me përmirësimin e cilësisë së kodit dhe uljen e kostos së rregullimit të gabimeve. Përdorni rregullisht një analizues të kodit statik!
- Ne jemi duke finalizuar dhe përmirësuar diagnostikimin ekzistues. Prandaj, analizuesi mund të identifikojë gabime që nuk i ka vërejtur gjatë skanimeve të mëparshme.
- Në PVS-Studio janë shfaqur diagnostifikime të reja që nuk ekzistonin 2 vjet më parë. Vendosa t'i theksoj ato në një seksion të veçantë për të treguar qartë zhvillimin e PVS-Studio.
Defekte të identifikuara nga diagnostifikimi që ekzistonin 2 vite më parë
Fragmenti 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
....
}
Paralajmërim PVS-Studio:
Kontrollohet dy herë që emri të fillojë me nënvargun "avx512.mask.permvar.". Në kontrollin e dytë, ata padyshim donin të shkruanin diçka tjetër, por harruan të korrigjonin tekstin e kopjuar.
Fragmenti N2: gabim shtypi
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;
....
}
Paralajmërim PVS-Studio: V501 Ka nën-shprehje identike 'CXNameRange_WantQualifier' në të majtë dhe në të djathtë të '|' operatori. CIndex.cpp 7245
Për shkak të një gabimi shtypi, e njëjta konstante me emër përdoret dy herë CXNameRange_WantQualifier.
Fragmenti N3: Ngatërrim me përparësinë e operatorit
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
Paralajmërim PVS-Studio:
Për mendimin tim, ky është një gabim shumë i bukur. Po, e di që kam ide të çuditshme për bukurinë :).
Tani, sipas
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Nga pikëpamja praktike, një kusht i tillë nuk ka kuptim, pasi mund të reduktohet në:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Ky është një gabim i qartë. Me shumë mundësi, ata donin të krahasonin 0/1 me një ndryshore indeks. Për të rregulluar kodin, duhet të shtoni kllapa rreth operatorit tresh:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Nga rruga, operatori tresh është shumë i rrezikshëm dhe provokon gabime logjike. Jini shumë të kujdesshëm me të dhe mos u bëni lakmitarë me kllapa. Unë e shikova këtë temë në mënyrë më të detajuar
Fragmenti N4, N5: Treguesi null
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;
}
....
}
Paralajmërim PVS-Studio:
Nëse treguesi LHS është nul, duhet të lëshohet një paralajmërim. Sidoqoftë, në vend të kësaj, i njëjti tregues null do të çreferencohet: LHS->getAsString().
Kjo është një situatë shumë tipike kur një gabim fshihet në një mbajtës gabimi, pasi askush nuk i teston ato. Analizuesit statikë kontrollojnë të gjithë kodin e arritshëm, pavarësisht sa shpesh përdoret. Ky është një shembull shumë i mirë se si analiza statike plotëson teknikat e tjera të testimit dhe mbrojtjes nga gabimet.
Gabim i ngjashëm në trajtimin e treguesit HRH lejohet në kodin pak më poshtë: V522 [CWE-476] Mund të ndodhë çreferencimi i treguesit null 'RHS'. TGParser.cpp 2186
Fragmenti N6: Përdorimi i treguesit pas lëvizjes
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);
}
....
}
Paralajmërim PVS-Studio: V522 [CWE-476] Mund të ndodhë çreferencimi i treguesit null 'ProgClone'. Kompilim i gabuar.cpp 601
Në fillim një tregues i zgjuar ProgClone pushon së zotëruari objektin:
BD.setNewProgram(std::move(ProgClone));
Në fakt, tani ProgClone është një tregues null. Prandaj, një dereferencim i treguesit null duhet të ndodhë pikërisht më poshtë:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Por, në realitet, kjo nuk do të ndodhë! Vini re se cikli nuk është ekzekutuar në të vërtetë.
Në fillim të enës Funksionet e përpiluara gabimisht pastruar:
MiscompiledFunctions.clear();
Më pas, madhësia e kësaj kontejneri përdoret në gjendjen e ciklit:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Është e lehtë të shihet se cikli nuk fillon. Unë mendoj se ky është gjithashtu një gabim dhe kodi duhet të shkruhet ndryshe.
Duket se kemi hasur në atë barazinë e famshme të gabimeve! Një gabim maskon një tjetër :).
Fragmenti N7: Përdorimi i treguesit pas lëvizjes
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;
}
....
}
Paralajmërim PVS-Studio: V522 [CWE-476] Mund të ndodhë çreferencimi i treguesit null 'Test'. Kompilim i gabuar.cpp 709
Përsëri e njëjta situatë. Fillimisht, përmbajtja e objektit zhvendoset dhe më pas përdoret sikur asgjë të mos kishte ndodhur. E shoh këtë situatë gjithnjë e më shpesh në kodin e programit pasi semantika e lëvizjes u shfaq në C++. Kjo është arsyeja pse unë e dua gjuhën C++! Ka gjithnjë e më shumë mënyra të reja për të gjuajtur këmbën tuaj. Analizuesi PVS-Studio do të ketë gjithmonë punë :).
Fragmenti N8: Treguesi null
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);
}
Paralajmërim PVS-Studio: V522 [CWE-476] Mund të ndodhë mosreferencimi i treguesit null 'Type'. PrettyFunctionDumper.cpp 233
Përveç trajtuesve të gabimeve, funksionet e printimit të korrigjimit zakonisht nuk testohen. Ne kemi vetëm një rast të tillë para nesh. Funksioni është në pritje të përdoruesit, i cili në vend që të zgjidhë problemet e tij, do të detyrohet ta rregullojë atë.
saktë:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragmenti N9: Treguesi null
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());
....
}
Paralajmërim PVS-Studio: V522 [CWE-476] Mund të ndodhë mosreferencimi i treguesit null 'Ty'. SearchableTableEmitter.cpp 614
Unë mendoj se gjithçka është e qartë dhe nuk kërkon shpjegim.
Fragmenti N10: gabim shtypi
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;
}
Paralajmërim PVS-Studio:
Nuk ka kuptim t'i caktoni një variabël vetes. Me shumë mundësi ata donin të shkruanin:
Identifier->Type = Question->Type;
Fragmenti N11: Thyerje e dyshimtë
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}
Paralajmërim PVS-Studio:
Ka një operator shumë të dyshimtë në fillim pushim. Keni harruar të shkruani diçka tjetër këtu?
Fragmenti N12: Kontrollimi i një treguesi pas çreferencimit
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");
....
}
Paralajmërim PVS-Studio:
Tregues Callee në fillim çreferencohet në momentin kur thirret funksioni merrniTTI.
Dhe pastaj rezulton se ky tregues duhet të kontrollohet për barazi nullptr:
if (!Callee || Callee->isDeclaration())
Por është shumë vonë…
Fragmenti N13 - N...: Kontrollimi i një treguesi pas çreferencimit
Situata e diskutuar në fragmentin e kodit të mëparshëm nuk është unike. Këtu shfaqet:
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()) { // <=
....
}
Paralajmërim PVS-Studio: V595 [CWE-476] Treguesi 'CalleeFn' u përdor përpara se të verifikohej kundër nullptr. Kontrolloni linjat: 1079, 1081. SimplifyLibCalls.cpp 1079
Dhe këtu:
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()); // <=
....
}
Paralajmërim PVS-Studio: V595 [CWE-476] Treguesi 'ND' u përdor përpara se të verifikohej kundër nullptr. Kontrolloni linjat: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Dhe këtu:
- V595 [CWE-476] Treguesi 'U' u përdor përpara se të verifikohej kundër nullptr. Kontrolloni linjat: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Treguesi 'ND' u përdor përpara se të verifikohej kundër nullptr. Kontrolloni linjat: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Dhe pastaj u bëra i painteresuar për të studiuar paralajmërimet me numrin V595. Pra, nuk e di nëse ka më shumë gabime të ngjashme përveç atyre të listuara këtu. Me shumë mundësi ka.
Fragmenti N17, N18: Zhvendosje e dyshimtë
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
Paralajmërim PVS-Studio:
Mund të mos jetë një gabim dhe kodi funksionon saktësisht siç synohet. Por ky është padyshim një vend shumë i dyshimtë dhe duhet kontrolluar.
Le të themi variablin Masat është e barabartë me 16, dhe më pas autori i kodit planifikoi ta merrte atë në një ndryshore NImms vlera:
1111111111111111111111111111111111111111111111111111111111100000
Sidoqoftë, në realitet rezultati do të jetë:
0000000000000000000000000000000011111111111111111111111111100000
Fakti është se të gjitha llogaritjet ndodhin duke përdorur llojin e panënshkruar 32-bit. Dhe vetëm atëherë, ky lloj 32-bitësh i panënshkruar do të zgjerohet në mënyrë implicite uint64_t. Në këtë rast, pjesët më të rëndësishme do të jenë zero.
Ju mund ta rregulloni situatën si kjo:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Situatë e ngjashme: V629 [CWE-190] Merrni parasysh inspektimin e shprehjes 'Immr << 6'. Zhvendosja e bitit të vlerës 32-bit me një zgjerim të mëvonshëm në llojin 64-bit. AArch64AddressingModes.h 269
Fragmenti N19: Mungon fjala kyçe tjetër?
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");
}
....
}
Paralajmërim PVS-Studio:
Këtu nuk ka asnjë gabim. Që nga blloku i atëhershëm i të parit if përfundon me vazhdoj, atëherë nuk ka rëndësi, ka një fjalë kyçe tjetër ose jo. Sido që të jetë, kodi do të funksionojë njësoj. Ende i munguar tjetër e bën kodin më të paqartë dhe më të rrezikshëm. Nëse në të ardhmen vazhdoj zhduket, kodi do të fillojë të funksionojë krejtësisht ndryshe. Për mendimin tim është më mirë të shtohet tjetër.
Fragmenti N20: Katër gabime shtypi të të njëjtit lloj
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;
}
Paralajmërimet e PVS-Studio:
- V655 [CWE-480] Vargjet u lidhën por nuk përdoren. Konsideroni të inspektoni shprehjen 'Rezultati + Emri.str()'. Simboli.cpp 32
- V655 [CWE-480] Vargjet u lidhën por nuk përdoren. Konsideroni të inspektoni shprehjen "Rezultati + "(Klasa ObjC)" + Emri.str()". Simboli.cpp 35
- V655 [CWE-480] Vargjet u lidhën por nuk përdoren. Merrni parasysh të inspektoni shprehjen "Rezultati + "(ObjC Class EH) " + Emri.str()". Simboli.cpp 38
- V655 [CWE-480] Vargjet u lidhën por nuk përdoren. Konsideroni të inspektoni shprehjen "Rezultati + "(ObjC IVar)" + Emri.str()". Simboli.cpp 41
Rastësisht, operatori + përdoret në vend të operatorit +=. Rezultati është dizajne që nuk kanë kuptim.
Fragmenti N21: Sjellje e papërcaktuar
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();
}
}
}
Mundohuni të gjeni vetë kodin e rrezikshëm. Dhe kjo është një foto për të shpërqendruar vëmendjen për të mos parë menjëherë përgjigjen:
Paralajmërim PVS-Studio:
Linja e problemit:
FeaturesMap[Op] = FeaturesMap.size();
Nëse elementi Op nuk gjendet, pastaj krijohet një element i ri në hartë dhe aty shkruhet numri i elementeve në këtë hartë. Nuk dihet nëse funksioni do të thirret madhësi para ose pas shtimit të një elementi të ri.
Fragmenti N22-N24: Detyra të përsëritura
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;
}
....
}
Paralajmërim PVS-Studio:
Unë nuk mendoj se ka një gabim të vërtetë këtu. Vetëm një detyrë e panevojshme e përsëritur. Por ende një gabim.
Në mënyrë të ngjashme:
- V519 [CWE-563] Variablës 'B.NDesc' i caktohen vlera dy herë radhazi. Ndoshta ky është një gabim. Linjat e kontrollit: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Variablës i janë caktuar vlera dy herë radhazi. Ndoshta ky është një gabim. Linjat e kontrollit: 59, 61. coff2yaml.cpp 61
Fragmenti N25-N27: Më shumë ricaktime
Tani le të shohim një version paksa të ndryshëm të ricaktimit.
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;
....
}
Paralajmërim PVS-Studio: V519 [CWE-563] Variablës 'Alignment' i caktohen vlera dy herë radhazi. Ndoshta ky është një gabim. Kontrolloni linjat: 1158, 1160. LoadStoreVetorizer.cpp 1160
Ky është një kod shumë i çuditshëm që me sa duket përmban një gabim logjik. Në fillim, e ndryshueshme Bashkim caktohet një vlerë në varësi të gjendjes. Dhe pastaj caktimi ndodh përsëri, por tani pa asnjë kontroll.
Situata të ngjashme mund të shihen këtu:
- V519 [CWE-563] Variablës 'Efektet' i caktohen vlera dy herë radhazi. Ndoshta ky është një gabim. Kontrolloni linjat: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Ndryshores 'ExpectNoDerefChunk' i caktohen vlera dy herë radhazi. Ndoshta ky është një gabim. Kontrolloni linjat: 4970, 4973. SemaType.cpp 4973
Fragmenti N28: Gjendje gjithmonë e vërtetë
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;
}
....
}
Paralajmërim PVS-Studio:
Kontrollimi nuk ka kuptim. E ndryshueshme NextByte gjithmonë jo e barabartë me vlerën 0x90, që rrjedh nga kontrolli i mëparshëm. Ky është një lloj gabimi logjik.
Fragmenti N29 - N...: Kushtet gjithmonë të vërteta/false
Analizuesi lëshon shumë paralajmërime se e gjithë gjendja (
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;
....
}
Paralajmërim PVS-Studio:
Konstanta 0xE është vlera 14 në dhjetor. Ekzaminimi RegNo == 0xe nuk ka kuptim sepse nëse RegNo > 13, atëherë funksioni do të përfundojë ekzekutimin e tij.
Kishte shumë paralajmërime të tjera me ID V547 dhe V560, por si me
Unë do t'ju jap një shembull se pse studimi i këtyre shkaktarëve është i mërzitshëm. Analizuesi ka absolutisht të drejtë në lëshimin e një paralajmërimi për kodin e mëposhtëm. Por ky nuk është një gabim.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
Paralajmërim PVS-Studio: V547 [CWE-570] Shprehja '!HasError' është gjithmonë e rreme. UnwrappedLineParser.cpp 1635
Fragmenti N30: Kthimi i dyshimtë
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();
}
....
}
Paralajmërim PVS-Studio:
Ky është ose një gabim ose një teknikë specifike që synon t'u shpjegojë diçka programuesve që lexojnë kodin. Ky dizajn nuk më shpjegon asgjë dhe duket shumë i dyshimtë. Më mirë të mos shkruash kështu :).
E lodhur? Pastaj është koha për të bërë çaj ose kafe.
Defektet e identifikuara nga diagnostifikimi i ri
Mendoj se mjaftojnë 30 aktivizime të diagnostifikimit të vjetër. Le të shohim tani se çfarë gjërash interesante mund të gjenden me diagnostifikimin e ri që u shfaq në analizuesin më pas
Fragment N31: Kod i paarritshëm
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();
}
Paralajmërim PVS-Studio:
Siç mund ta shihni, të dy degët e operatorit if përfundon me një telefonatë për operatorin kthim. Prandaj, enë CtorDtorsByPriority nuk do të pastrohet kurrë.
Fragment N32: Kod i paarritshëm
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;
}
Paralajmërim PVS-Studio: V779 [CWE-561] U zbulua kod i paarritshëm. Është e mundur që të ketë një gabim. LLParser.cpp 835
Situatë interesante. Le të shohim së pari këtë vend:
return ParseTypeIdEntry(SummaryID);
break;
Në pamje të parë, duket se këtu nuk ka asnjë gabim. Duket si operatori pushim ka një shtesë këtu, dhe ju thjesht mund ta fshini atë. Megjithatë, jo gjithçka kaq e thjeshtë.
Analizuesi lëshon një paralajmërim në linjat:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Dhe me të vërtetë, ky kod është i paarritshëm. Të gjitha rastet në kaloni përfundon me një telefonatë nga operatori kthim. Dhe tani e pakuptimtë vetëm pushim nuk duket aq i padëmshëm! Ndoshta një nga degët duhet të përfundojë me pushim, jo në kthim?
Fragmenti N33: Rivendosja e rastësishme e biteve të larta
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);
....
}
Paralajmërim PVS-Studio:
Ju lutemi vini re se funksioni getStubAlignment lloji i kthimit i pa firmosur. Le të llogarisim vlerën e shprehjes, duke supozuar se funksioni kthen vlerën 8:
~(getStubAlignment() - 1)
~ (8u-1)
0xFFFFFFFF8u
Tani vini re se ndryshorja Madhësia e të Dhënave ka një tip 64-bit të panënshkruar. Rezulton se kur kryeni operacionin DataSize & 0xFFFFFFF8u, të tridhjetë e dy bitet e rendit të lartë do të rivendosen në zero. Me shumë mundësi, kjo nuk është ajo që dëshironte programuesi. Dyshoj se ka dashur të llogarisë: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Për të rregulluar gabimin, duhet të shkruani këtë:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Ose kështu:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragmenti N34: Dështo cast i tipit eksplicit
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);
....
}
Paralajmërim PVS-Studio:
Hedhja e tipit eksplicit përdoret për të shmangur tejmbushjen kur shumëzohen variablat e tipit int. Megjithatë, hedhja e tipit të qartë këtu nuk mbron nga tejmbushja. Së pari, variablat do të shumëzohen dhe vetëm atëherë rezultati 32-bit i shumëzimit do të zgjerohet në llojin
Fragmenti N35: Kopjo-ngjitja e dështuar
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;
}
....
}
Ky diagnostikim i ri interesant identifikon situatat kur një pjesë e kodit është kopjuar dhe disa emra në të kanë filluar të ndryshohen, por në një vend nuk e kanë korrigjuar atë.
Ju lutemi vini re se në bllokun e dytë ato ndryshuan Op0 mbi Op1. Por në një vend ata nuk e rregulluan atë. Me shumë mundësi duhet të ishte shkruar kështu:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragment N36: Konfuzion i ndryshueshëm
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
Paralajmërim PVS-Studio:
Është shumë e rrezikshme t'u jepen argumenteve të funksioneve emrat e njëjtë si anëtarët e klasës. Është shumë e lehtë të ngatërrohesh. Ne kemi vetëm një rast të tillë para nesh. Kjo shprehje nuk ka kuptim:
Mode &= Mask;
Argumenti i funksionit ndryshon. Kjo eshte e gjitha. Ky argument nuk përdoret më. Me shumë mundësi duhet ta kishit shkruar kështu:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragment N37: Konfuzion i ndryshueshëm
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;
}
Paralajmërim PVS-Studio: V1001 [CWE-563] Ndryshorja 'Size' është caktuar por nuk përdoret deri në fund të funksionit. Objekti.cpp 424
Situata është e ngjashme me atë të mëparshme. Duhet të shkruhet:
this->Size += this->EntrySize;
Fragmenti N38-N47: Harruan të kontrollonin indeksin
Më parë, ne shikuam shembuj të nxitjes diagnostikuese
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()); // <=
....
}
Paralajmërim PVS-Studio: V1004 [CWE-476] Treguesi 'Ptr' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 729, 738. TargetTransformInfoImpl.h 738
Ndryshore PTR mund të jetë i barabartë nullptr, siç dëshmohet nga kontrolli:
if (Ptr != nullptr)
Sidoqoftë, poshtë këtij treguesi nuk referohet pa kontroll paraprak:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Le të shqyrtojmë një rast tjetër të ngjashëm.
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(); // <=
....
}
Paralajmërim PVS-Studio: V1004 [CWE-476] Treguesi 'FD' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 3228, 3231. CGDebugInfo.cpp 3231
Kushtojini vëmendje shenjës FD. Jam i sigurt se problemi është qartë i dukshëm dhe nuk kërkohet shpjegim i veçantë.
Dhe më tej:
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()); // <=
....
}
Paralajmërim PVS-Studio: V1004 [CWE-476] Treguesi 'PtrTy' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 960, 965. InterleavedLoadCombinePass.cpp 965
Si të mbroheni nga gabime të tilla? Jini më të vëmendshëm në Code-Review dhe përdorni analizuesin statik PVS-Studio për të kontrolluar rregullisht kodin tuaj.
Nuk ka kuptim të citohen fragmente të tjera kodi me gabime të këtij lloji. Unë do të lë vetëm një listë paralajmërimesh në artikull:
- V1004 [CWE-476] Treguesi 'Expr' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrollo linjat: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Treguesi 'PI' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Treguesi 'StatepointCall' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Treguesi 'RV' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Treguesi 'CalleeFn' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Treguesi 'TC' u përdor në mënyrë të pasigurt pasi u verifikua kundër nullptr. Kontrolloni linjat: 1819, 1824. Driver.cpp 1824
Fragmenti N48-N60: Jo kritik, por një defekt (rrjedhje e mundshme e kujtesës)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
Paralajmërim PVS-Studio:
Për të shtuar një element në fund të një enë si std:: vektor > thjesht nuk mund të shkruash xxx.push_back (X i ri), pasi nuk ka konvertim të nënkuptuar nga X* в std::unique_ptr.
Një zgjidhje e zakonshme është të shkruani xxx.emplace_back (X i ri)meqenëse përpilon: metodë vend_mbrapa ndërton një element direkt nga argumentet dhe për këtë arsye mund të përdorë konstruktorë të qartë.
Nuk është e sigurt. Nëse vektori është i plotë, atëherë memoria rialokohet. Operacioni i rishpërndarjes së kujtesës mund të dështojë, duke rezultuar në një përjashtim std::bad_alloc. Në këtë rast, treguesi do të humbasë dhe objekti i krijuar nuk do të fshihet kurrë.
Një zgjidhje e sigurt është krijimi unike_ptri cili do të zotërojë treguesin përpara se vektori të përpiqet të rialokojë kujtesën:
xxx.push_back(std::unique_ptr<X>(new X))
Që nga C++14, mund të përdorni 'std::make_unique':
xxx.push_back(std::make_unique<X>())
Ky lloj defekti nuk është kritik për LLVM. Nëse memoria nuk mund të ndahet, përpiluesi thjesht do të ndalojë. Megjithatë, për aplikimet me të gjata
Pra, megjithëse ky kod nuk përbën një kërcënim praktik për LLVM, e pashë të dobishme të flisja për këtë model gabimi dhe që analizuesi PVS-Studio ka mësuar ta identifikojë atë.
Paralajmërime të tjera të këtij lloji:
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Passes" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. PassManager.h 546
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin 'AA' me metodën 'emplace_back'. Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. AliasAnalysis.h 324
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Hyrjet" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "AllEdges" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. CFGMST.h 268
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin 'VMaps' me metodën 'emplace_back'. Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Records" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. FDRLogBuilder.h 30
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "PendingSubmodules" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. ModuleMap.cpp 810
- V1023 [CWE-460] Një tregues pa pronar shtohet në kontejnerin 'Objects' me metodën 'emplace_back'. Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. DebugMap.cpp 88
- V1023 [CWE-460] Një tregues pa pronar shtohet në kontejnerin "Strategies" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 685
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 686
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 688
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 689
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 690
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 691
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 692
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 693
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Modifikuesit" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. llvm-stress.cpp 694
- V1023 [CWE-460] Një tregues pa pronar shtohet në kontejnerin 'Operands' me metodën 'emplace_back'. Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin "Stash" me metodën "emplace_back". Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Një tregues pa zotërues shtohet në kontejnerin 'Matchers' me metodën 'emplace_back'. Një rrjedhje memorie do të ndodhë në rast të një përjashtimi. GlobalISelEmitter.cpp 2702
Përfundim
Kam dhënë gjithsej 60 paralajmërime dhe më pas kam ndaluar. A ka defekte të tjera që analizuesi PVS-Studio zbulon në LLVM? Po, kam. Megjithatë, kur po shkruaja fragmente kodi për artikullin, ishte vonë në mbrëmje, ose më mirë edhe natë, dhe vendosa që ishte koha ta quaja atë një ditë.
Shpresoj se ju dukej interesante dhe do të dëshironi të provoni analizuesin PVS-Studio.
Mund të shkarkoni analizuesin dhe të merrni çelësin e minaveheerit në
Më e rëndësishmja, përdorni rregullisht analizën statike. Kontrolle një herë, të kryera nga ne për të popullarizuar metodologjinë e analizës statike dhe PVS-Studio nuk janë një skenar normal.
Fat i mirë në përmirësimin e cilësisë dhe besueshmërisë së kodit tuaj!
Nëse dëshironi ta ndani këtë artikull me një audiencë anglishtfolëse, ju lutemi përdorni lidhjen e përkthimit: Andrey Karpov.
Burimi: www.habr.com