Meira en tvö ár eru liðin frá síðustu kóðaathugun LLVM verkefnisins með því að nota PVS-Studio greiningartækið okkar. Við skulum ganga úr skugga um að PVS-Studio greiningartækið sé enn leiðandi tæki til að bera kennsl á villur og hugsanlega veikleika. Til að gera þetta munum við athuga og finna nýjar villur í LLVM 8.0.0 útgáfunni.
Grein sem á að skrifa
Satt að segja vildi ég ekki skrifa þessa grein. Það er ekki áhugavert að skrifa um verkefni sem við höfum þegar skoðað nokkrum sinnum (
Í hvert skipti sem ný útgáfa af LLVM er gefin út eða uppfærð
Sjáðu, nýja útgáfan af Clang Static Analyzer hefur lært að finna nýjar villur! Mér sýnist að mikilvægi þess að nota PVS-Studio fari minnkandi. Clang finnur fleiri villur en áður og nær getu PVS-Studio. Hvað finnst þér um þetta?
Við þessu vil ég alltaf svara einhverju eins og:
Við sitjum heldur ekki auðum höndum! Við höfum bætt verulega getu PVS-Studio greiningartækisins. Svo ekki hafa áhyggjur, við höldum áfram að leiða eins og áður.
Því miður er þetta slæmt svar. Það eru engar sannanir í því. Og þess vegna skrifa ég þessa grein núna. Svo hefur LLVM verkefnið enn og aftur verið athugað og margvíslegar villur hafa fundist í því. Ég mun nú sýna þá sem mér þóttu áhugaverðir. Clang Static Analyzer getur ekki fundið þessar villur (eða það er mjög óþægilegt að gera það með hjálp þess). En við getum. Þar að auki fann ég og skrifaði niður allar þessar villur á einu kvöldi.
En það tók nokkrar vikur að skrifa greinina. Ég bara gat ekki stillt mig um að setja þetta allt í texta :).
Við the vegur, ef þú hefur áhuga á hvaða tækni er notuð í PVS-Studio greiningartækinu til að bera kennsl á villur og hugsanlega veikleika, þá legg ég til að þú kynnir þér þetta
Ný og gömul greining
Eins og áður hefur komið fram, fyrir um tveimur árum var LLVM verkefnið aftur athugað og villurnar sem fundust voru leiðréttar. Nú mun þessi grein kynna nýja lotu af villum. Hvers vegna fundust nýjar villur? Það eru 3 ástæður fyrir þessu:
- LLVM verkefnið er að þróast, breytir gömlum kóða og bætir við nýjum kóða. Auðvitað eru nýjar villur í breyttum og skrifaðum kóða. Þetta sýnir greinilega að nota ætti kyrrstöðugreiningu reglulega en ekki einstaka sinnum. Greinar okkar sýna vel getu PVS-Studio greiningartækisins, en þetta hefur ekkert að gera með að bæta kóða gæði og draga úr kostnaði við að laga villur. Notaðu kyrrstöðugreiningartæki reglulega!
- Við erum að leggja lokahönd á og bæta núverandi greiningar. Þess vegna getur greiningartækið greint villur sem hann tók ekki eftir við fyrri skannanir.
- Nýjar greiningar hafa birst í PVS-Studio sem voru ekki til fyrir 2 árum. Ég ákvað að varpa ljósi á þá í sérstökum kafla til að sýna skýrt þróun PVS-Studio.
Gallar greindir með greiningu sem voru fyrir 2 árum síðan
Brot N1: Copy-Paste
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}
PVS-Stúdíó viðvörun:
Tvöfalt hakað við að nafnið byrji á undirstrengnum "avx512.mask.permvar.". Í seinni athuguninni vildu þeir greinilega skrifa eitthvað annað en gleymdu að leiðrétta afritaðan texta.
Brot N2: Innsláttarvilla
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;
....
}
Viðvörun PVS-Studio: V501 Það eru eins undirtjáningar 'CXNameRange_WantQualifier' vinstra megin og hægra megin við '|' rekstraraðili. CIndex.cpp 7245
Vegna innsláttarvillu er samnefndi fasti notaður tvisvar CXNameRange_WantQualifier.
Brot N3: Rugl við forgang rekstraraðila
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Stúdíó viðvörun:
Að mínu mati er þetta mjög falleg mistök. Já, ég veit að ég hef undarlegar hugmyndir um fegurð :).
Nú, skv
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Frá hagnýtu sjónarhorni er slíkt ástand ekki skynsamlegt, þar sem hægt er að minnka það í:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Þetta er augljós mistök. Líklegast vildu þeir bera 0/1 saman við breytu Index. Til að laga kóðann þarftu að bæta við sviga utan um þrískiptinguna:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Við the vegur, ternary rekstraraðili er mjög hættulegur og vekur rökfræðilegar villur. Vertu mjög varkár með það og ekki vera gráðugur með sviga. Ég skoðaði þetta efni nánar
Brot N4, N5: Núllbendill
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-Stúdíó viðvörun:
Ef bendillinn LHS er ógild, ætti að gefa út viðvörun. Hins vegar verður þessum sama núllbendingu vísað frá: LHS->getAsString().
Þetta er mjög dæmigert ástand þegar villa er falin í villumeðferð, þar sem enginn prófar þær. Statískir greiningartæki athuga allan kóða sem hægt er að nálgast, sama hversu oft hann er notaður. Þetta er mjög gott dæmi um hvernig kyrrstöðugreining bætir við aðrar prófanir og villuvarnaraðferðir.
Svipuð villa meðhöndlun bendils RHS leyfilegt í kóðanum rétt fyrir neðan: V522 [CWE-476] Frávísun á núllbendlinum 'RHS' gæti átt sér stað. TGParser.cpp 2186
Brot N6: Notaðu bendilinn eftir flutning
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 Viðvörun: V522 [CWE-476] Frávísun á núllbendlinum 'ProgClone' gæti átt sér stað. Miscompilation.cpp 601
Í upphafi snjall bendill ProgClone hættir að eiga hlutinn:
BD.setNewProgram(std::move(ProgClone));
Reyndar núna ProgClone er núllbending. Þess vegna ætti núllbending tilvísun að eiga sér stað rétt fyrir neðan:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
En í raun og veru mun þetta ekki gerast! Athugaðu að lykkjan er í raun ekki keyrð.
Í upphafi gámsins Missamdar aðgerðir hreinsað:
MiscompiledFunctions.clear();
Næst er stærð þessa íláts notuð í lykkjuástandinu:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Það er auðvelt að sjá að lykkjan byrjar ekki. Ég held að þetta sé líka villa og ætti að skrifa kóðann öðruvísi.
Það virðist sem við höfum lent í þessum fræga jöfnuði villna! Ein mistök hylja aðra :).
Brot N7: Notaðu bendilinn eftir flutning
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 viðvörun: V522 [CWE-476] Frávísun á núllbendlinum 'Test' gæti átt sér stað. Miscompilation.cpp 709
Sama ástandið aftur. Í fyrstu er innihald hlutarins fært til og síðan er það notað eins og ekkert hafi í skorist. Ég sé þetta ástand oftar og oftar í forritakóða eftir að merkingarfræði hreyfingar birtist í C++. Þess vegna elska ég C++ tungumálið! Það eru fleiri og fleiri nýjar leiðir til að skjóta af sér fótinn. PVS-Studio greiningartækið mun alltaf hafa vinnu :).
Brot N8: Núllbendill
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 viðvörun: V522 [CWE-476] Frávísun á núllbendilinn 'Type' gæti átt sér stað. PrettyFunctionDumper.cpp 233
Auk villumeðferðaraðila eru villuleitarútprentunaraðgerðir venjulega ekki prófaðar. Við höfum einmitt svona mál fyrir okkur. Aðgerðin bíður eftir notandanum, sem, í stað þess að leysa vandamál sín, neyðist til að laga það.
Rétt:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Brot N9: Núllbendill
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 viðvörun: V522 [CWE-476] Frávísun á núllbendilinn 'Ty' gæti átt sér stað. SearchableTableEmitter.cpp 614
Ég held að allt sé skýrt og þarfnast ekki skýringa.
Brot N10: Innsláttarvilla
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-Stúdíó viðvörun:
Það þýðir ekkert að úthluta breytu fyrir sig. Líklegast vildu þeir skrifa:
Identifier->Type = Question->Type;
Brot N11: Grunsamlegt brot
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-Stúdíó viðvörun:
Það er mjög grunsamlegur rekstraraðili í upphafi brjóta. Gleymdirðu að skrifa eitthvað annað hérna?
Brot N12: Athugun á bendili eftir frávísun
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-Stúdíó viðvörun:
Bendill Callee í upphafi er vísað frá á þeim tíma sem fallið er kallað getTTI.
Og þá kemur í ljós að þetta ábending ætti að vera athugað með tilliti til jafnræðis nullptr:
if (!Callee || Callee->isDeclaration())
En það er of seint…
Brot N13 - N...: Athugun á bendili eftir frávísun
Staðan sem fjallað var um í fyrra kóðabrotinu er ekki einsdæmi. Það birtist hér:
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 viðvörun: V595 [CWE-476] 'CalleeFn' bendillinn var notaður áður en hann var staðfestur gegn nullptr. Athugaðu línur: 1079, 1081. SimplifyLibCalls.cpp 1079
Og hér:
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 viðvörun: V595 [CWE-476] 'ND' bendillinn var notaður áður en hann var staðfestur gegn nullptr. Athugaðu línur: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Og hér:
- V595 [CWE-476] 'U' bendillinn var notaður áður en hann var staðfestur gegn nullptr. Athugaðu línur: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND' bendillinn var notaður áður en hann var staðfestur gegn nullptr. Athugaðu línur: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Og svo varð ég áhugalaus um að kynna mér viðvaranirnar með númerinu V595. Svo ég veit ekki hvort það eru fleiri svipaðar villur fyrir utan þær sem taldar eru upp hér. Líklegast er það.
Brot N17, N18: Grunsamleg vakt
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Stúdíó viðvörun:
Það er kannski ekki galli og kóðinn virkar nákvæmlega eins og hann er ætlaður. En þetta er greinilega mjög grunsamlegur staður og þarf að athuga.
Segjum breytuna Size er jafnt og 16, og þá ætlaði höfundur kóðans að fá hann í breytu NImms gildi:
1111111111111111111111111111111111111111111111111111111111100000
Hins vegar verður niðurstaðan í raun og veru:
0000000000000000000000000000000011111111111111111111111111100000
Staðreyndin er sú að allir útreikningar eiga sér stað með því að nota 32-bita ómerkta gerð. Og aðeins þá verður þessi 32-bita óundirrituðu gerð óbeint stækkuð í uint64_t. Í þessu tilviki verða mikilvægustu bitarnir núll.
Þú getur lagað ástandið svona:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Svipaðar aðstæður: V629 [CWE-190] Íhugaðu að skoða 'Immr << 6' tjáninguna. Bitabreyting á 32 bita gildinu með síðari stækkun yfir í 64 bita gerð. AArch64AddressingModes.h 269
Brot N19: Leitarorð vantar annars?
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-Stúdíó viðvörun:
Hér er engin mistök. Síðan þáverandi blokk fyrsta if endar með áfram, þá skiptir það ekki máli, það er lykilorð annars eða ekki. Hvort heldur sem er mun kóðinn virka eins. Enn saknað annars gerir kóðann óljósari og hættulegri. Ef í framtíðinni áfram hverfur mun kóðinn byrja að virka allt öðruvísi. Að mínu mati er betra að bæta við annars.
Brot N20: Fjórar innsláttarvillur af sömu gerð
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-Stúdíó viðvaranir:
- V655 [CWE-480] Strengir voru samtengdir en eru ekki notaðir. Íhugaðu að skoða 'Result + Name.str()' tjáninguna. Symbol.cpp 32
- V655 [CWE-480] Strengir voru samtengdir en eru ekki notaðir. Íhugaðu að skoða 'Result + "(ObjC Class)" + Name.str()' tjáningu. Symbol.cpp 35
- V655 [CWE-480] Strengir voru samtengdir en eru ekki notaðir. Íhugaðu að skoða 'Result + "(ObjC Class EH) " + Name.str()' tjáningu. Symbol.cpp 38
- V655 [CWE-480] Strengir voru samtengdir en eru ekki notaðir. Íhugaðu að skoða 'Result + "(ObjC IVar)" + Name.str()' tjáningu. Symbol.cpp 41
Fyrir tilviljun er + stjórnandinn notaður í stað += stjórnandans. Niðurstaðan er hönnun sem er merkingarlaus.
Brot N21: Óskilgreind hegðun
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();
}
}
}
Reyndu sjálfur að finna hættulega kóðann. Og þetta er mynd til að dreifa athyglinni til að horfa ekki strax á svarið:
PVS-Stúdíó viðvörun:
Vandamálslína:
FeaturesMap[Op] = FeaturesMap.size();
Ef frumefni Op finnst ekki, þá er nýr þáttur búinn til í kortinu og fjöldi þátta í þessu korti skrifaður þar. Það er bara óþekkt hvort aðgerðin verður kölluð stærð fyrir eða eftir að nýjum þætti er bætt við.
Brot N22-N24: Endurtekin verkefni
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-Stúdíó viðvörun:
Ég held að hér sé ekki um raunveruleg mistök að ræða. Bara óþarfa endurtekið verkefni. En samt klúður.
Sömuleiðis:
- V519 [CWE-563] 'B.NDesc' breytunni er úthlutað gildum tvisvar í röð. Kannski er þetta mistök. Athugaðu línur: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Breytunni er úthlutað gildum tvisvar í röð. Kannski er þetta mistök. Athugaðu línur: 59, 61. coff2yaml.cpp 61
Brot N25-N27: Fleiri endurúthlutun
Nú skulum við skoða aðeins aðra útgáfu af endurúthlutun.
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 viðvörun: V519 [CWE-563] 'Alignment' breytunni er úthlutað gildum tvisvar í röð. Kannski er þetta mistök. Athugaðu línur: 1158, 1160. LoadStoreVectorizer.cpp 1160
Þetta er mjög undarlegur kóði sem virðist innihalda rökræna villu. Í upphafi, breyt Alignment gildi er úthlutað eftir aðstæðum. Og svo kemur úthlutunin aftur, en núna án þess að athuga það.
Svipaðar aðstæður má sjá hér:
- V519 [CWE-563] 'Áhrif' breytunni er úthlutað gildum tvisvar í röð. Kannski er þetta mistök. Athugaðu línur: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] 'ExpectNoDerefChunk' breytunni er úthlutað gildum tvisvar í röð. Kannski er þetta mistök. Athugaðu línur: 4970, 4973. SemaType.cpp 4973
Brot N28: Alltaf satt ástand
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-Stúdíó viðvörun:
Það er ekki skynsamlegt að athuga. Breytilegt nextBæti alltaf ekki jafnt gildinu 0x90, sem leiðir af fyrri athugun. Þetta er einhvers konar rökvilla.
Brot N29 - N...: Alltaf satt/ósatt skilyrði
Greiningartækið gefur út margar viðvaranir um að allt ástandið (
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-Stúdíó viðvörun:
Fastinn 0xE er gildið 14 í aukastaf. Próf RegNo == 0xe meikar ekki sens því ef RegNo > 13, þá mun aðgerðin ljúka framkvæmd sinni.
Það voru margar aðrar viðvaranir með auðkenni V547 og V560, en eins og með
Ég skal gefa þér dæmi um hvers vegna það er leiðinlegt að rannsaka þessar kveikjur. Greiningartækið hefur alveg rétt fyrir sér í að gefa út viðvörun fyrir eftirfarandi kóða. En þetta eru ekki mistök.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio Viðvörun: V547 [CWE-570] Tjáning '!HasError' er alltaf ósönn. UnwrappedLineParser.cpp 1635
Brot N30: Grunsamleg endurkoma
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-Stúdíó viðvörun:
Þetta er annað hvort villa eða ákveðin tækni sem er ætluð til að útskýra eitthvað fyrir forriturum sem lesa kóðann. Þessi hönnun útskýrir ekkert fyrir mér og lítur mjög grunsamlega út. Það er betra að skrifa ekki svona :).
Þreyttur? Þá er komið að því að búa til te eða kaffi.
Gallar greindir með nýrri greiningu
Ég held að 30 virkjanir af gömlum greiningartækjum séu nóg. Við skulum nú sjá hvaða áhugaverða hluti er að finna með nýju greiningunum sem birtust í greiningartækinu á eftir
Brot N31: Kóði sem ekki er hægt að ná í
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-Stúdíó viðvörun:
Eins og þú sérð, bæði útibú rekstraraðila if endar með því að hringja í símafyrirtækið aftur. Samkvæmt því er gámurinn CtorDtorsBy Priority verður aldrei hreinsað.
Brot N32: Kóði sem ekki er hægt að ná í
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 viðvörun: V779 [CWE-561] Kóði sem ekki er hægt að ná í fannst. Hugsanlegt er að villa sé til staðar. LLParser.cpp 835
Áhugaverð staða. Við skulum líta á þennan stað fyrst:
return ParseTypeIdEntry(SummaryID);
break;
Við fyrstu sýn virðist sem engin villa sé hér. Það lítur út eins og rekstraraðilinn brjóta það er auka einn hérna og þú getur einfaldlega eytt honum. Hins vegar er ekki allt svo einfalt.
Greiningartækið gefur út viðvörun á línunum:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Og svo sannarlega er ekki hægt að nálgast þennan kóða. Öll mál í skipta endar með símtali frá símafyrirtækinu aftur. Og nú tilgangslaus einn brjóta lítur ekki svo skaðlaust út! Kannski ætti ein af greinunum að enda með brjótaen ekki á aftur?
Brot N33: Tilviljunarkennd endurstilling á háum bitum
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-Stúdíó viðvörun:
Vinsamlegast athugaðu að aðgerðin getStubAlignment skilar gerð óundirritaðir. Við skulum reikna út gildi tjáningarinnar, að því gefnu að fallið skili gildinu 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Taktu nú eftir því að breyt Gagnastærð hefur 64 bita óundirritaða gerð. Það kemur í ljós að þegar DataSize & 0xFFFFFFF8u aðgerðin er framkvæmd, verða allir þrjátíu og tveir hástigsbitarnir núllstilltir. Líklegast er þetta ekki það sem forritarinn vildi. Mig grunar að hann hafi viljað reikna: DataSize & 0xFFFFFFFFFFFFFFF8u.
Til að laga villuna ættir þú að skrifa þetta:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Eða svo:
DataSize &= ~(getStubAlignment() - 1ULL);
Brot N34: Misheppnuð skýr gerð steypa
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-Stúdíó viðvörun:
Skýr gerð steypu er notuð til að forðast yfirfall þegar tegundarbreytur eru margfaldaðar int. Hins vegar, skýr gerð steypa hér verndar ekki gegn yfirfalli. Fyrst verða breyturnar margfaldaðar og aðeins þá verður 32-bita niðurstaða margföldunarinnar stækkuð í tegundina
Brot N35: Mistókst að afrita og líma
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;
}
....
}
Þessi nýja áhugaverða greining greinir aðstæður þar sem bútur af kóða hefur verið afritaður og byrjað að breyta sumum nöfnum í honum, en á einum stað hafa þeir ekki leiðrétt það.
Vinsamlegast athugaðu að í seinni blokkinni breyttust þau Op0 á Op1. En á einum stað redduðu þeir því ekki. Líklegast hefði átt að skrifa svona:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Brot N36: Breytilegt rugl
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Stúdíó viðvörun:
Það er mjög hættulegt að gefa fallrökum sömu nöfnum og bekkjarmeðlimum. Það er mjög auðvelt að ruglast. Við höfum einmitt svona mál fyrir okkur. Þessi tjáning meikar ekki sens:
Mode &= Mask;
Aðgerðarröksemdin breytist. Það er allt og sumt. Þessi rök eru ekki lengur notuð. Líklega hefðir þú átt að skrifa þetta svona:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Brot N37: Breytilegt rugl
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;
}
Viðvörun PVS-Studio: V1001 [CWE-563] 'Stærð' breytunni er úthlutað en er ekki notuð í lok fallsins. Object.cpp 424
Staðan er svipuð og sú fyrri. Það ætti að skrifa:
this->Size += this->EntrySize;
Brot N38-N47: Þeir gleymdu að athuga vísitöluna
Áður skoðuðum við dæmi um sjúkdómsgreiningu
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 viðvörun: V1004 [CWE-476] 'Ptr' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 729, 738. TargetTransformInfoImpl.h 738
Breytilegt Ptr geta verið jafnir nullptr, eins og ávísunin sýnir:
if (Ptr != nullptr)
Hins vegar er vísað frá þessum bendili fyrir neðan án bráðabirgðaathugunar:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Við skulum íhuga annað svipað mál.
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 viðvörun: V1004 [CWE-476] 'FD' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 3228, 3231. CGDebugInfo.cpp 3231
Gefðu gaum að merkinu FD. Ég er viss um að vandamálið sést vel og engin sérstök útskýring er nauðsynleg.
Og ennfremur:
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 viðvörun: V1004 [CWE-476] 'PtrTy' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 960, 965. InterleavedLoadCombinePass.cpp 965
Hvernig á að vernda þig gegn slíkum mistökum? Vertu vakandi fyrir Code-Review og notaðu PVS-Studio kyrrstöðugreiningartækið til að athuga kóðann þinn reglulega.
Það þýðir ekkert að vitna í önnur kóðabrot með villum af þessu tagi. Ég mun aðeins skilja eftir lista yfir viðvaranir í greininni:
- V1004 [CWE-476] 'Expr' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] 'PI' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] 'StatepointCall' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] 'RV' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] 'TC' bendillinn var notaður á óöruggan hátt eftir að hann var staðfestur gegn nullptr. Athugaðu línur: 1819, 1824. Driver.cpp 1824
Brot N48-N60: Ekki mikilvægt, en galli (mögulegur minnisleki)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Stúdíó viðvörun:
Til að bæta þætti við lok íláts eins og std::vektor > þú getur ekki bara skrifað xxx.push_back(nýtt X), þar sem engin óbein umbreyting er frá X* в std::einstakt_ptr.
Algeng lausn er að skrifa xxx.emplace_back(nýtt X)þar sem það safnar saman: aðferð emplace_back smíðar frumefni beint úr rökum sínum og getur því notað skýra smiði.
Það er ekki öruggt. Ef vektorinn er fullur, þá er minni úthlutað aftur. Endurúthlutunaraðgerð minni gæti mistekist, sem leiðir til þess að undantekningu er kastað std::bad_alloc. Í þessu tilviki mun bendillinn glatast og hlutnum sem búið er til verður aldrei eytt.
Örugg lausn er að búa til einstakt_ptrsem mun eiga bendilinn áður en vigur reynir að endurúthluta minni:
xxx.push_back(std::unique_ptr<X>(new X))
Þar sem C++14 geturðu notað 'std::make_unique':
xxx.push_back(std::make_unique<X>())
Þessi tegund galla er ekki mikilvæg fyrir LLVM. Ef ekki er hægt að úthluta minni mun þýðandinn einfaldlega hætta. Hins vegar fyrir umsóknir með langa
Svo, þó að þessi kóði stafi ekki hagnýt ógn við LLVM, fannst mér gagnlegt að tala um þetta villumynstur og að PVS-Studio greiningartækið hafi lært að bera kennsl á það.
Aðrar viðvaranir af þessu tagi:
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Passes' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. PassManager.h 546
- V1023 [CWE-460] Bendi án eiganda er bætt við 'AAs' ílátið með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. AliasAnalysis.h 324
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Entries' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Bendill án eiganda er bætt við 'AllEdges' ílátið með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. CFGMST.h 268
- V1023 [CWE-460] Bendill án eiganda er bætt við 'VMaps' ílátið með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Records' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. FDRLogBuilder.h 30
- V1023 [CWE-460] Bendill án eiganda er bætt við 'PendingSubmodules' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. ModuleMap.cpp 810
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Objects' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. DebugMap.cpp 88
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Strategies' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 685
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 686
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 688
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 689
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 690
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 691
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 692
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 693
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Modifiers' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. llvm-stress.cpp 694
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Operands' gáminn með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Bendill án eiganda er bætt við 'Stash' ílátið með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Bendi án eiganda er bætt við 'Matchers' ílátið með 'emplace_back' aðferðinni. Minningarleki mun eiga sér stað ef undantekning er á. GlobalISelEmitter.cpp 2702
Ályktun
Ég gaf út 60 viðvaranir alls og hætti svo. Eru aðrir gallar sem PVS-Studio greiningartækið greinir í LLVM? Já ég hef. Hins vegar, þegar ég var að skrifa út kóðabrot fyrir greinina, var það seint á kvöldin, eða öllu heldur nótt, og ég ákvað að það væri kominn tími til að kalla þetta dag.
Ég vona að þér hafi fundist það áhugavert og vilja prófa PVS-Studio greiningartækið.
Þú getur hlaðið niður greiningartækinu og fengið jarðsprengjulykilinn á
Mikilvægast er að nota stöðuga greiningu reglulega. Einskiptiskoðanir, framkvæmt af okkur í því skyni að auka vinsældir aðferðafræði truflanagreiningar og PVS-Studio eru ekki eðlileg atburðarás.
Gangi þér vel í að bæta gæði og áreiðanleika kóðans þíns!
Ef þú vilt deila þessari grein með enskumælandi áhorfendum, vinsamlegast notaðu þýðingartengilinn: Andrey Karpov.
Heimild: www.habr.com