Bi urte baino gehiago igaro dira gure PVS-Studio analizatzailea erabiliz LLVM proiektuaren azken kodea egiaztatu zenetik. Ziurta dezagun PVS-Studio analizatzailea akatsak eta ahultasun potentzialak identifikatzeko tresna nagusia dela oraindik. Horretarako, akats berriak egiaztatu eta aurkituko ditugu LLVM 8.0.0 bertsioan.
Idatzi beharreko artikulua
Egia esateko, ez nuen artikulu hau idatzi nahi. Ez da interesgarria jada hainbat aldiz egiaztatu dugun proiektu bati buruz idaztea (
LLVM-ren bertsio berri bat askatzen edo eguneratzen den bakoitzean
Begira, Clang Static Analyzer-en bertsio berriak errore berriak aurkitzen ikasi du! PVS-Studio erabiltzearen garrantzia gutxitzen ari dela iruditzen zait. Clang-ek lehen baino akats gehiago aurkitzen ditu eta PVS-Studio-ren gaitasunak lortzen ditu. Zer deritzozu honi buruz?
Honi beti honelako zerbait erantzun nahi diot:
Gu ere ez gara alferrik eserita! PVS-Studio analizatzailearen gaitasunak nabarmen hobetu ditugu. Beraz, ez kezkatu, lehen bezala lideratzen jarraitzen dugu.
Zoritxarrez, erantzun txarra da. Bertan ez dago frogarik. Eta horregatik idazten ari naiz artikulu hau orain. Beraz, LLVM proiektua berriro ere egiaztatu da eta hainbat akats aurkitu dira bertan. Interesgarriak iruditu zaizkidanak erakutsiko ditut orain. Clang Static Analyzer-ek ezin ditu akats hauek aurkitu (edo oso deserosoa da bere laguntzarekin hori egitea). Baina ahal dugu. Gainera, akats horiek guztiak aurkitu eta idatzi ditut arratsalde batean.
Baina artikulua idazteak hainbat aste behar izan zituen. Ezin nuen hau guztia testuan jartzera eraman :).
Bide batez, PVS-Studio analizatzailean zer teknologia erabiltzen diren interesatzen bazaizu akatsak eta ahultasun potentzialak identifikatzeko, orduan hau ezagutzea gomendatzen dizut.
Diagnostiko berriak eta zaharrak
Esan bezala, duela bi urte inguru LLVM proiektua berriro ere egiaztatu zen, eta aurkitutako akatsak zuzendu ziren. Orain artikulu honek akats sorta berri bat aurkeztuko du. Zergatik aurkitu ziren akats berriak? 3 arrazoi daude horretarako:
- LLVM proiektua eboluzionatzen ari da, kode zaharra aldatuz eta kode berria gehituz. Jakina, akats berriak daude aldatutako eta idatzitako kodean. Horrek argi erakusten du analisi estatikoa aldizka erabili behar dela, eta ez noizean behin. Gure artikuluek ondo erakusten dute PVS-Studio analizatzailearen gaitasunak, baina horrek ez du zerikusirik kodearen kalitatea hobetzearekin eta akatsak konpontzearen kostua murriztearekin. Erabili kode estatikoko analizatzaile bat aldizka!
- Dauden diagnostikoak amaitzen eta hobetzen ari gara. Hori dela eta, analizatzaileak aurreko analisietan nabaritu ez dituen akatsak identifikatu ditzake.
- Duela 2 urte existitzen ez ziren diagnostiko berriak agertu dira PVS-Studioan. Aparteko atal batean nabarmentzea erabaki nuen PVS-Studio-ren garapena argi eta garbi erakusteko.
Duela 2 urte zeuden diagnostikoen bidez identifikatutako akatsak
N1 zatia: Kopiatu-Itsatsi
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-Studioko abisua:
Bikoiztu egiten da izena "avx512.mask.permvar." azpikatearekin hasten dela. Bigarren egiaztapenean, jakina, beste zerbait idatzi nahi zuten, baina kopiatutako testua zuzentzea ahaztu zitzaien.
N2 zatia: akatsa
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;
....
}
Abisua PVS-Studio: V501 "CXNameRange_WantQualifier" azpi-esamolde berdinak daude '|'-ren ezkerraldean eta eskuinean. operadorea. CIndex.cpp 7245
Akats bat dela eta, bi aldiz erabiltzen da izen bereko konstantea CXNameRange_WantQualifier.
N3 zatia: operadorearen lehentasunarekin nahastea
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studioko abisua:
Nire ustez, hau oso akats ederra da. Bai, badakit edertasunaren inguruan ideia arraroak ditudala :).
Orain, arabera
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Ikuspegi praktikotik, baldintza horrek ez du zentzurik, honela murriztu baitaiteke:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Hau akats argia da. Seguruenik, 0/1 aldagai batekin alderatu nahi izan dute Index. Kodea konpontzeko parentesiak gehitu behar dituzu operadore ternarioaren inguruan:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Bide batez, operadore ternarioa oso arriskutsua da eta akats logikoak eragiten ditu. Kontuz ibili harekin eta ez izan zikoitz parentesiekin. Gai hau zehatzago aztertu nuen
N4, N5 zatia: Erakusle nulua
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-Studioko abisua:
Erakuslea bada LHS nulua da, abisua eman behar da. Hala ere, horren ordez, erakusle nulu hori bera deserreferentziatuko da: LHS->getAsString().
Egoera oso tipikoa da errore bat akatsen kudeatzaile batean ezkutatzen denean, inork ez baitu probatzen. Analizatzaile estatikoek eskura daitekeen kode guztia egiaztatzen dute, zenbateraino erabiltzen den. Hau oso adibide ona da analisi estatikoak beste probak eta akatsak babesteko teknikak nola osatzen dituen.
Erakusleak maneiatzeko antzeko errorea RHS beheko kodean onartzen da: V522 [CWE-476] 'RHS' erakusle nuluaren deserreferentziak gerta daitezke. TGParser.cpp 2186
N6 zatia: Erakuslea erabiltzea mugitu ondoren
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 Abisua: V522 [CWE-476] 'ProgClone' erakusle nuluaren deserreferentziak gerta daitezke. Konpilazio okerra.cpp 601
Hasieran erakusle adimenduna ProgClone objektuaren jabe izateari uzten dio:
BD.setNewProgram(std::move(ProgClone));
Izan ere, orain ProgClone erakusle nulua da. Beraz, erakusle nuluaren deserreferentzia bat behean gertatu beharko litzateke:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Baina, egia esan, hau ez da gertatuko! Kontuan izan begizta ez dela benetan exekutatzen.
Edukiontziaren hasieran Gaizki konpilatutako funtzioak garbitu:
MiscompiledFunctions.clear();
Ondoren, edukiontzi honen tamaina begizta egoeran erabiltzen da:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Erraz ikusten da begizta ez dela hasten. Uste dut hau ere akats bat dela eta kodea bestela idatzi behar dela.
Badirudi akatsen parekotasun famatu horrekin topatu dugula! Akats batek beste bat ezkutatzen du :).
N7 zatia: Erakuslea erabiltzea mugitu ondoren
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 abisua: V522 [CWE-476] 'Test' erakusle nuluaren deserreferentziak gerta daitezke. Konpilazio okerra.cpp 709
Egoera bera berriro. Hasieran, objektuaren edukia mugitzen da, eta gero ezer gertatuko ez balitz bezala erabiltzen da. Egoera hau gero eta sarriago ikusten dut programa-kodean mugimendu-semantika C++-n agertu ondoren. Horregatik maite dut C++ hizkuntza! Gero eta modu berri gehiago daude zure hanka tiro egiteko. PVS-Studio analizatzaileak beti izango du lana :).
N8 zatia: Erakusle nulua
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 abisua: V522 [CWE-476] 'Mota' erakusle nuluaren deserreferentziak gerta daitezke. PrettyFunctionDumper.cpp 233
Errore-kudeatzaileez gain, arazketa-inprimatze-funtzioak normalean ez dira probatzen. Horrelako kasu bat besterik ez dugu aurrean. Funtzioa erabiltzailearen zain dago, eta honek, bere arazoak konpondu beharrean, konpontzera behartuta egongo da.
zuzentzeko:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
N9 zatia: Erakusle nulua
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 abisua: V522 [CWE-476] 'Ty' erakusle nuluaren deserreferentziak gerta daitezke. SearchableTableEmitter.cpp 614
Uste dut dena argi dagoela eta ez duela azalpenik behar.
N10 zatia: akatsa
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-Studioko abisua:
Ez du balio aldagai bat bere buruari esleitzeak. Seguruenik idatzi nahi zuten:
Identifier->Type = Question->Type;
N11 zatia: haustura susmagarria
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-Studioko abisua:
Hasieran oso eragile susmagarria dago apurtu. Ahaztu al zaizu hemen beste zerbait idaztea?
N12 zatia: erakusle bat egiaztatzea deserreferentziatu ondoren
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-Studioko abisua:
Indizea Callee hasieran deserreferentzia egiten da funtzioa deitzen den unean lortuTTI.
Eta gero, erakusle hau berdintasuna egiaztatu beharko litzateke nullptr:
if (!Callee || Callee->isDeclaration())
Baina berandu da...
N13 - N... zatia: Erakusle bat egiaztatzea deserreferentziatu ondoren
Aurreko kode zatian aztertutako egoera ez da bakarra. Hemen agertzen da:
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 abisua: V595 [CWE-476] 'CalleeFn' erakuslea erabili zen nullptr-en aurka egiaztatu aurretik. Egiaztatu lerroak: 1079, 1081. SimplifyLibCalls.cpp 1079
Eta hemen:
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 abisua: V595 [CWE-476] 'ND' erakuslea erabili zen nullptr-en aurka egiaztatu aurretik. Egiaztatu lerroak: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Eta hemen:
- V595 [CWE-476] "U" erakuslea erabili zen nullptr-en aurka egiaztatu aurretik. Egiaztatu lerroak: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND' erakuslea erabili zen nullptr-en aurka egiaztatu aurretik. Egiaztatu lerroak: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Eta orduan V595 zenbakiarekin abisuak aztertzeko interesik gabe geratu nintzen. Beraz, ez dakit hemen zerrendatutakoez gain antzeko akats gehiago dauden. Seguruenik badago.
N17, N18 zatia: Aldaketa susmagarria
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studioko abisua:
Baliteke akats bat ez izatea eta kodeak nahi bezala funtzionatzen du. Baina argi eta garbi hau oso leku susmagarria da eta egiaztatu egin behar da.
Demagun aldagaia Tamaina 16ren berdina da, eta, ondoren, kodearen egileak aldagai batean jasotzea aurreikusi zuen NImms esanahia:
1111111111111111111111111111111111111111111111111111111111100000
Hala ere, errealitatean emaitza hau izango da:
0000000000000000000000000000000011111111111111111111111111100000
Kontua da kalkulu guztiak 32 biteko sinatu gabeko mota erabiliz egiten direla. Eta orduan bakarrik, 32 biteko sinatu gabeko mota hau inplizituki zabalduko da uint64_t. Kasu honetan, bit esanguratsuenak zero izango dira.
Honela konpondu dezakezu egoera:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Antzeko egoera: V629 [CWE-190] Demagun 'Immr << 6' adierazpena ikuskatzea. 32 biteko balioaren bit-desplazamendua, ondoren 64 biteko motara hedatuz. AArch64AddressingModes.h 269
N19 zatia: gako-hitza falta da bestela?
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-Studioko abisua:
Hemen ez dago akatsik. Lehenengoaren orduko blokeaz geroztik if honekin bukatzen da jarraitu, orduan ez du axola, gako-hitz bat dago bestela edo ez. Edozein modutan kodeak berdin funtzionatuko du. Oraindik galduta bestela kodea argiago eta arriskutsuagoa bihurtzen du. Etorkizunean bada jarraitu desagertzen da, kodea guztiz desberdin hasiko da lanean. Nire ustez hobe da gehitzea bestela.
N20 zatia: mota bereko lau akats
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-Studioko abisuak:
- V655 [CWE-480] Kateak kateatu egin ziren baina ez dira erabiltzen. Demagun 'Emaitza + Izena.str()' adierazpena ikuskatzea. Sinboloa.cpp 32
- V655 [CWE-480] Kateak kateatu egin ziren baina ez dira erabiltzen. Demagun 'Emaitza + "(ObjC Klasea)" + Name.str()' adierazpena ikuskatzea. Sinboloa.cpp 35
- V655 [CWE-480] Kateak kateatu egin ziren baina ez dira erabiltzen. Demagun 'Emaitza + "(ObjC Class EH) " + Name.str()' adierazpena ikuskatzea. Sinboloa.cpp 38
- V655 [CWE-480] Kateak kateatu egin ziren baina ez dira erabiltzen. Demagun 'Emaitza + "(ObjC IVar)" + Name.str()' adierazpena ikuskatzea. Sinboloa.cpp 41
Istripuz, + operadorea erabiltzen da += eragilearen ordez. Emaitza zentzurik gabeko diseinuak dira.
N21 zatia: Definitu gabeko portaera
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();
}
}
}
Saiatu kode arriskutsua zeure burua aurkitzen. Eta hau arreta distraitzeko argazkia da, erantzuna berehala ez begiratzeko:
PVS-Studioko abisua:
Arazoaren lerroa:
FeaturesMap[Op] = FeaturesMap.size();
Elementua bada Op ez da aurkitzen, orduan elementu berri bat sortzen da mapan eta mapa honetako elementu kopurua idazten da bertan. Funtzioa deituko den ezezaguna da tamaina elementu berri bat gehitu aurretik edo ondoren.
N22-N24 zatia: errepikatutako lanak
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-Studioko abisua:
Ez dut uste hemen benetako akatsik dagoenik. Alferrikako zeregin errepikatua besterik ez. Baina oraindik hutsala.
Era berean:
- V519 [CWE-563] 'B.NDesc' aldagaiari balioak esleitzen zaizkio segidan. Agian hau akats bat da. Egiaztatu lerroak: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Aldagaiari bi aldiz jarraian balioak esleitzen zaizkio. Agian hau akats bat da. Kontrol-lerroak: 59, 61. coff2yaml.cpp 61
N25-N27 zatia: biresleipen gehiago
Ikus dezagun berriro esleipenaren bertsio apur bat desberdina.
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 abisua: V519 [CWE-563] 'Lerrokatzea' aldagaiari balioak esleitzen zaizkio segidan. Agian hau akats bat da. Egiaztatu lerroak: 1158, 1160. LoadStoreVectorizer.cpp 1160
Kode oso bitxia da, itxuraz errore logiko bat daukana. Hasieran, aldakorra Lerrokatzea baldintzaren arabera balio bat esleitzen da. Eta gero esleipena berriro gertatzen da, baina orain inolako egiaztapenik gabe.
Antzeko egoerak hemen ikus daitezke:
- V519 [CWE-563] 'Ondorioak' aldagaiari balioak esleitzen zaizkio segidan. Agian hau akats bat da. Egiaztatu lerroak: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] 'ExpectNoDerefChunk' aldagaiari balioak esleitzen zaizkio bi aldiz jarraian. Agian hau akats bat da. Egiaztatu lerroak: 4970, 4973. SemaType.cpp 4973
N28 zatia: beti benetako baldintza
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-Studioko abisua:
Egiaztapenak ez du zentzurik. Aldakorra hurrengo Byte beti ez balioaren berdina 0x90, aurreko egiaztapenetik datorrena. Hau errore logiko bat da.
N29 zatia - N...: beti egia/gezurra baldintza
Analizatzaileak abisu asko ematen ditu egoera osoa (
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-Studioko abisua:
0xE konstantea hamartarren 14 balioa da. Azterketa RegNo == 0xe ez du zentzurik bada RegNo > 13, orduan funtzioak bere exekuzioa osatuko du.
V547 eta V560 IDekin beste abisu asko zeuden, baina bezala
Adibide bat emango dizut abiarazle hauek aztertzea zergatik den aspergarria. Analizatzaileak arrazoi osoa du hurrengo kodearen abisua ematean. Baina hau ez da akats bat.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio abisua: V547 [CWE-570] '!HasError' adierazpena beti da faltsua. UnwrappedLineParser.cpp 1635
N30 zatia: itzulera susmagarria
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-Studioko abisua:
Hau errore bat edo teknika zehatz bat da, kodea irakurtzen duten programatzaileei zerbait azaltzeko xedea duena. Diseinu honek ez dit ezer azaltzen eta oso susmagarria dirudi. Hobe da horrela ez idaztea :).
Nekatuta? Ondoren, tea edo kafea prestatzeko garaia iritsi da.
Diagnostiko berriek identifikatutako akatsak
Uste dut diagnostiko zaharren 30 aktibazio nahikoa dela. Ikus dezagun orain zer gauza interesgarri aurki daitezkeen ondoren analizatzailean agertu ziren diagnostiko berriekin
N31 zatia: eskuraezina den kodea
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-Studioko abisua:
Ikus dezakezunez, operadorearen bi adarrak if operadoreari dei batekin amaitzen da bueltan. Horren arabera, edukiontzia CtorDtorsByPriority ez da inoiz garbituko.
N32 zatia: eskuraezina den kodea
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-Studioko abisua: V779 [CWE-561] Kode eskuraezina detektatu da. Baliteke erroreren bat egotea. LLParser.cpp 835
Egoera interesgarria. Ikus dezagun lehenik leku hau:
return ParseTypeIdEntry(SummaryID);
break;
Lehen begiratuan, badirudi hemen ez dagoela errorerik. Operadorearen itxura du apurtu hemen gehigarri bat dago, eta besterik gabe ezabatu dezakezu. Hala ere, dena ez da hain sinplea.
Analizatzaileak abisua ematen du lerroetan:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Eta, hain zuzen ere, kode hau ailegatu ezin da. Kasu guztiak barne aldatzeko operadorearen dei batekin amaitzen da bueltan. Eta orain zentzugabea bakarrik apurtu ez dirudi hain kaltegabea! Beharbada adarren batek amaitu beharko luke apurtu, ez piztuta bueltan?
N33 zatia: bit altuen ausazko berrezarri
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-Studioko abisua:
Kontuan izan funtzioa getStubAlignment mota itzultzen du sinatu gabe. Kalkula dezagun adierazpenaren balioa, funtzioak 8 balioa itzultzen duela suposatuz:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Orain ohartu aldagaia dela DatuTamaina 64 biteko sinatu gabeko mota du. Ematen du DataSize & 0xFFFFFFF8u eragiketa egitean, maila handiko hogeita hamabi bit guztiak zerora berrezarriko direla. Seguruenik, hau ez da programatzaileak nahi zuena. Sumoa dut kalkulatu nahi zuela: DataSize & 0xFFFFFFFFFFFFFFF8u.
Errorea konpontzeko, hau idatzi beharko zenuke:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Edo, beraz:
DataSize &= ~(getStubAlignment() - 1ULL);
N34 zatia: huts egin du mota esplizitua igortzea
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-Studioko abisua:
Mota-igorpen esplizitua erabiltzen da gainezkatzea ekiditeko mota-aldagaiak biderkatzean int. Hala ere, mota esplizituak hemen igortzeak ez du gainezkatzetik babesten. Lehenik eta behin, aldagaiak biderkatuko dira, eta orduan bakarrik biderketaren 32 biteko emaitza motara zabalduko da.
N35 zatia: Kopiatu-Itsatsi huts egin zuen
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;
}
....
}
Diagnostiko interesgarri berri honek kode zati bat kopiatu eta bertan izen batzuk aldatzen hasi diren egoerak identifikatzen ditu, baina leku batean ez dute zuzendu.
Kontuan izan bigarren blokean aldatu zirela Op0 on Op1. Baina leku batean ez zuten konpondu. Seguruenik honela idatzi behar zen:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
N36 zatia: Nahasketa aldagaia
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studioko abisua:
Oso arriskutsua da funtzio-argumentuei klaseko kideen izen berdinak ematea. Oso erraza da nahastea. Horrelako kasu bat besterik ez dugu aurrean. Adierazpen honek ez du zentzurik:
Mode &= Mask;
Funtzioaren argumentua aldatzen da. Hori da dena. Argumentu hau jada ez da erabiltzen. Ziurrenik honela idatzi beharko zenuke:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
N37 zatia: Nahasketa aldagaia
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;
}
Abisua PVS-Studio: V1001 [CWE-563] 'Tamaina' aldagaia esleituta dago baina ez da funtzioaren amaieran erabiltzen. Object.cpp 424
Egoera aurrekoaren antzekoa da. Idatzi behar da:
this->Size += this->EntrySize;
N38-N47 zatia: aurkibidea egiaztatzea ahaztu zaie
Aurretik, diagnostiko-abiaraztearen adibideak aztertu genituen
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-ko abisua: V1004 [CWE-476] 'Ptr' erakuslea modu seguruan erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 729, 738. TargetTransformInfoImpl.h 738
Aldakorra Ptr berdinak izan daitezke nullptrtxekeak frogatzen duenez:
if (Ptr != nullptr)
Hala ere, erakusle honen azpian erreferentzia kendu egiten da aldez aurretik egiaztatu gabe:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Azter dezagun antzeko beste kasu bat.
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 abisua: V1004 [CWE-476] 'FD' erakuslea segurtasunez erabili zen nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 3228, 3231. CGDebugInfo.cpp 3231
Erreparatu seinaleari FD. Ziur nago arazoa argi ikusten dela eta ez dela azalpen berezirik behar.
Eta aurrerago:
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-ko abisua: V1004 [CWE-476] 'PtrTy' erakuslea seguru erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 960, 965. InterleavedLoadCombinePass.cpp 965
Nola babestu horrelako akatsetatik? Adi egon Code-Review-n eta erabili PVS-Studio analizatzaile estatikoa zure kodea aldizka egiaztatzeko.
Ez du balio mota honetako akatsak dituzten beste kode zati batzuk aipatzea. Artikuluan abisu-zerrenda bat bakarrik utziko dut:
- V1004 [CWE-476] 'Expr' erakuslea modu seguruan erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] 'PI' erakuslea modu seguruan erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] 'StatepointCall' erakuslea modu seguruan erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatzeko lerroak: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] 'RV' erakuslea modu seguruan erabili zen nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn' erakuslea modu seguruan erabili da nullptr-en aurka egiaztatu ondoren. Egiaztatu lerroak: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] 'TC' erakuslea modu seguruan erabili zen nullptr-en aurka egiaztatu ondoren. Kontrol-lerroak: 1819, 1824. Driver.cpp 1824
N48-N60 zatia: ez da kritikoa, baina akats bat (memoriaren ihesa posiblea)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studioko abisua:
Edukiontzi baten amaieran elementu bat gehitzeko std::bektorea > ezin duzu besterik idatzi xxx.push_back(X berria), ez baitago konbertsio inpliziturik X* Π² std::unique_ptr.
Irtenbide arrunta idaztea da xxx.emplace_back(X berria)biltzen duenez: metodo emplace_back elementu bat zuzenean bere argumentuetatik eraikitzen du eta, beraz, eraikitzaile esplizituak erabil ditzake.
Ez da segurua. Bektorea beteta badago, memoria berriro esleituko da. Memoria berregokitzeko eragiketak huts egin dezake eta, ondorioz, salbuespen bat botako da std::bad_alloc. Kasu honetan, erakuslea galduko da eta sortutako objektua ez da inoiz ezabatuko.
Irtenbide segurua sortzea da bakarra_ptrzeina erakuslearen jabe izango da bektoreak memoria birlokatzen saiatu aurretik:
xxx.push_back(std::unique_ptr<X>(new X))
C++14az geroztik, 'std::make_unique' erabil dezakezu:
xxx.push_back(std::make_unique<X>())
Akats mota hau ez da kritikoa LLVMrentzat. Memoria ezin bada esleitu, konpilatzailea geldituko da. Hala ere, luzea duten aplikazioetarako
Beraz, kode honek LLVMrentzat mehatxu praktikorik suposatzen ez duen arren, erabilgarria iruditu zait errore-eredu honi buruz hitz egitea eta PVS-Studio analizatzaileak identifikatzen ikasi duela.
Mota honetako beste ohar batzuk:
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Passes' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. PassManager.h 546
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'AAs' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. AliasAnalysis.h 324
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Entries' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'AllEdges' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. CFGMST.h 268
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'VMaps' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Erregistroak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. FDRLogBuilder.h 30
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'PendingSubmodules' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. ModuleMap.cpp 810
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Objektuak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. DebugMap.cpp 88
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Estrategiak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 685
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 686
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 688
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 689
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 690
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 691
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 692
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 693
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Aldatzaileak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. llvm-stress.cpp 694
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Erakenduak' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Stash' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Jaberik gabeko erakuslea gehitzen da 'Matchers' edukiontzian 'emplace_back' metodoaren bidez. Memoria-ihes bat gertatuko da salbuespen bat izanez gero. GlobalISelEmitter.cpp 2702
Ondorioa
Guztira 60 abisu eman nituen eta gero gelditu nintzen. Ba al dago PVS-Studio analizatzaileak LLVMn detektatzen dituen beste akatsik? Bai, badut. Hala ere, artikuluaren kode zatiak idazten ari nintzelarik, arratsaldea zen, edo hobeto esanda, gaua zen, eta eguna deitzeko ordua zela erabaki nuen.
Espero dut interesgarria iruditzea eta PVS-Studio analizatzailea probatu nahi izatea.
Analizatzailea deskargatu eta meatze-gakoa lor dezakezu hemen
Garrantzitsuena, erabili azterketa estatikoa aldizka. Behin-behineko egiaztapenak, guk egindako analisi estatikoen metodologia ezagutarazteko eta PVS-Studio ez dira agertoki normalak.
Zorte on zure kodearen kalitatea eta fidagarritasuna hobetzeko!
Artikulu hau ingelesez hitz egiten duen publiko batekin partekatu nahi baduzu, mesedez, erabili itzulpen esteka: Andrey Karpov.
Iturria: www.habr.com