Mear dan twa jier binne ferrûn sûnt de lêste koadekontrôle fan it LLVM-projekt mei ús PVS-Studio-analyzer. Litte wy derfoar soargje dat de PVS-Studio-analyzer noch altyd in liedend ark is foar it identifisearjen fan flaters en potensjele kwetsberens. Om dit te dwaan, sille wy nije flaters kontrolearje en fine yn 'e LLVM 8.0.0-release.
Artikel te skriuwen
Om earlik te wêzen, woe ik dit artikel net skriuwe. It is net nijsgjirrich om te skriuwen oer in projekt dat wy al ferskate kearen kontrolearre hawwe (
Elke kear wurdt in nije ferzje fan LLVM útbrocht of bywurke
Sjoch, de nije ferzje fan Clang Static Analyzer hat leard nije flaters te finen! It liket my ta dat de relevânsje fan it brûken fan PVS-Studio ôfnimt. Clang fynt mear flaters dan earder en komt yn op mei de mooglikheden fan PVS-Studio. Wat fine jo hjirfan?
Dêr wol ik altyd soksawat op antwurdzje as:
Wy sitte ek net stil! Wy hawwe de mooglikheden fan 'e PVS-Studio analysator signifikant ferbettere. Dus meitsje jo gjin soargen, wy bliuwe liede lykas earder.
Spitigernôch, dit is in min antwurd. Der sitte gjin bewiis yn. En dêrom skriuw ik dit artikel no. Sa is it LLVM-projekt nochris kontrolearre en binne der in ferskaat oan flaters yn fûn. Ik sil no dejingen demonstrearje dy't my ynteressant liken. Clang Static Analyzer kin dizze flaters net fine (of it is ekstreem ûngemaklik om dit te dwaan mei syn help). Mar wy kinne. Boppedat haw ik al dy flaters yn ien jûn fûn en opskreaun.
Mar it skriuwen fan it artikel duorre ferskate wiken. Ik koe it my gewoan net bringe om dit alles yn tekst te setten :).
Trouwens, as jo ynteressearre binne yn hokker technologyen wurde brûkt yn 'e PVS-Studio-analyzer om flaters en potinsjele kwetsberens te identifisearjen, dan stel ik foar om mei dit yn 'e kunde te kommen.
Nije en âlde diagnostyk
Lykas al opmurken, waard sawat twa jier lyn it LLVM-projekt nochris kontrolearre, en de fûnen fûnen waarden korrizjearre. No sil dit artikel in nije partij flaters presintearje. Wêrom waarden nije bugs fûn? D'r binne 3 redenen foar dit:
- It LLVM-projekt evoluearret, feroaret âlde koade en foegje nije koade ta. Fansels binne d'r nije flaters yn 'e feroare en skreaune koade. Dit toant dúdlik oan dat statyske analyze regelmjittich brûkt wurde moat, en net sa no en dan. Us artikels litte de mooglikheden fan 'e PVS-Studio-analyzer goed sjen, mar dit hat neat te meitsjen mei it ferbetterjen fan koadekwaliteit en it ferminderjen fan de kosten foar it reparearjen fan flaters. Brûk regelmjittich in statyske koadeanalysator!
- Wy finalisearje en ferbetterje besteande diagnostyk. Dêrom kin de analysator flaters identifisearje dy't it net opmurken hat by eardere scans.
- Nije diagnoaze binne ferskynd yn PVS-Studio dy't 2 jier lyn net bestie. Ik besleat se yn in aparte seksje te markearjen om de ûntwikkeling fan PVS-Studio dúdlik te sjen.
Defekten identifisearre troch diagnostyk dy't bestiene 2 jier lyn
Fragmint N1: Kopiearje-plakke
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}
PVS-Studio warskôging:
Der wurdt dûbel kontrolearre dat de namme begjint mei de substring "avx512.mask.permvar.". By de twadde kontrôle woene se fansels noch wat skriuwe, mar fergeaten de kopiearre tekst te ferbetterjen.
Fragmint N2: Typo
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;
....
}
Warskôging PVS-Studio: V501 D'r binne identike sub-ekspresjes 'CXNameRange_WantQualifier' links en rjochts fan '|' operator. CIndex.cpp 7245
Troch in typflater wurdt deselde neamde konstante twa kear brûkt CXNameRange_WantQualifier.
Fragmint N3: Betizing mei operator foarrang
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio warskôging:
Neffens my is dit in hiel moaie flater. Ja, ik wit dat ik nuvere ideeën haw oer skientme :).
No, neffens
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Ut in praktysk eachpunt is sa'n betingst gjin sin, om't it kin wurde fermindere nei:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Dit is in dúdlike flater. Meast wierskynlik woene se 0/1 fergelykje mei in fariabele Yndeks. Om de koade te reparearjen moatte jo heakjes tafoegje om de ternêre operator:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Troch de wei, de ternêre operator is tige gefaarlik en provosearret logyske flaters. Wês der tige foarsichtich mei en wês net gierig mei heakjes. Ik seach nei dit ûnderwerp yn mear detail
Fragmint N4, N5: Null oanwizer
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast<TypedInit>(Result);
....
LHS = dyn_cast<TypedInit>(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() +
"' to string");
return nullptr;
}
....
}
PVS-Studio warskôging:
As de oanwizer LHS is nul, in warskôging moat wurde útjûn. Ynstee dêrfan sil dizze selde nul-oanwizer lykwols derferfereard wurde: LHS->getAsString().
Dit is in hiel typyske situaasje as in flater is ferburgen yn in flater handler, sûnt gjinien test se. Statyske analysatoren kontrolearje alle berikbere koade, nettsjinsteande hoe faak it wurdt brûkt. Dit is in heul goed foarbyld fan hoe't statyske analyse oare test- en flaterbeskermingstechniken oanfolje.
Similar pointer handling flater RHS tastien yn de koade krekt hjirûnder: V522 [CWE-476] Dereferencing fan de nul pointer 'RHS' kin plakfine. TGParser.cpp 2186
Fragmint N6: Mei help fan de oanwizer nei it ferpleatsen
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 Warskôging: V522 [CWE-476] Deferearing fan 'e nul-oanwizer 'ProgClone' kin plakfine. Miscompilation.cpp 601
Oan it begjin in tûke oanwizer ProgClone hâldt op mei it besit fan it objekt:
BD.setNewProgram(std::move(ProgClone));
Yn feite, no ProgClone is in nul pointer. Dêrom moat in nul-oanwizer derferferinsje krekt hjirûnder foarkomme:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Mar yn werklikheid sil dit net barre! Tink derom dat de loop net eins wurdt útfierd.
Oan it begjin fan 'e kontener Miscompiled Functions wiske:
MiscompiledFunctions.clear();
Dêrnei wurdt de grutte fan dizze kontener brûkt yn 'e loopbetingst:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
It is maklik om te sjen dat de loop net begjint. Ik tink dat dit ek in brek is en de koade moat oars skreaun wurde.
It liket derop dat wy dy ferneamde pariteit fan flaters tsjinkamen! Ien flater maskearret in oare :).
Fragmint N7: Mei help fan de oanwizer nei it ferpleatsen
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 warskôging: V522 [CWE-476] Derferferinsje fan 'e nuloanwizer 'Test' kin plakfine. Miscompilation.cpp 709
Deselde situaasje wer. Earst wurdt de ynhâld fan it objekt ferpleatst, en dan wurdt it brûkt as wie der neat bard. Ik sjoch dizze situaasje hieltyd faker yn programma koade neidat beweging semantyk ferskynde yn C ++. Dit is wêrom ik de C++-taal hâld! D'r binne hieltyd mear nije manieren om jo eigen skonk ôf te sjitten. De PVS-Studio analysator sil altyd wurk hawwe :).
Fragmint N8: Null oanwizer
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 warskôging: V522 [CWE-476] Deferearing fan 'e nuloanwizer 'Type' kin plakfine. PrettyFunctionDumper.cpp 233
Neist flaterhannelers, debuggen printout funksjes wurde meastal net hifke. Wy hawwe krekt sa'n gefal foar ús. De funksje wachtet op de brûker, dy't, ynstee fan syn problemen op te lossen, twongen wurde om it te reparearjen.
Korrekt:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragmint N9: Null oanwizer
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 warskôging: V522 [CWE-476] Deferearing fan 'e nuloanwizer 'Ty' kin plakfine. SearchableTableEmitter.cpp 614
Ik tink dat alles dúdlik is en gjin útlis nedich is.
Fragmint N10: Typo
bool FormatTokenLexer::tryMergeCSharpNullConditionals() {
....
auto &Identifier = *(Tokens.end() - 2);
auto &Question = *(Tokens.end() - 1);
....
Identifier->ColumnWidth += Question->ColumnWidth;
Identifier->Type = Identifier->Type; // <=
Tokens.erase(Tokens.end() - 1);
return true;
}
PVS-Studio warskôging:
It hat gjin punt in tawize in fariabele oan himsels. Meast wierskynlik woene se skriuwe:
Identifier->Type = Question->Type;
Fragmint N11: Fertochte brek
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}
PVS-Studio warskôging:
Der is in tige fertochte operator oan it begjin brekke. Binne jo fergetten om hjir wat oars te skriuwen?
Fragmint N12: Kontrolearje in oanwizer nei dereferencing
InlineCost AMDGPUInliner::getInlineCost(CallSite CS) {
Function *Callee = CS.getCalledFunction();
Function *Caller = CS.getCaller();
TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
if (!Callee || Callee->isDeclaration())
return llvm::InlineCost::getNever("undefined callee");
....
}
PVS-Studio warskôging:
Pointer Callee oan it begjin wurdt derferferearde op it momint dat de funksje oanroppen wurdt getTTI.
En dan docht bliken dat dizze oanwizer kontrolearre wurde moat op gelikensens nullptr:
if (!Callee || Callee->isDeclaration())
Mar it is te let...
Fragmint N13 - N...: Kontrolearje in oanwizer nei dereferencing
De situaasje besprutsen yn it foarige koadefragmint is net unyk. It ferskynt hjir:
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 warskôging: V595 [CWE-476] De 'CalleeFn' oanwizer waard brûkt foardat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 1079, 1081. SimplifyLibCalls.cpp 1079
En hjir:
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 warskôging: V595 [CWE-476] De 'ND'-oanwizer waard brûkt foardat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 532, 534. SemaTemplateInstantiateDecl.cpp 532
En hjir:
- V595 [CWE-476] De 'U'-oanwizer waard brûkt foardat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] De 'ND'-oanwizer waard brûkt foardat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 2149, 2151. SemaTemplateInstantiate.cpp 2149
En doe waard ik net ynteressearre yn it bestudearjen fan de warskôgings mei nûmer V595. Dat ik wit net oft d'r mear ferlykbere flaters binne neist de hjir neamde. Meast wierskynlik is der.
Fragmint N17, N18: Fertochte ferskowing
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio warskôging:
It kin gjin brek wêze en de koade wurket krekt lykas bedoeld. Mar dit is dúdlik in hiel fertocht plak en moat wurde kontrolearre.
Litte wy sizze de fariabele Grutte is gelyk oan 16, en dan is de skriuwer fan 'e koade pland om it yn in fariabele te krijen NImms wearde:
1111111111111111111111111111111111111111111111111111111111100000
Yn werklikheid sil it resultaat lykwols wêze:
0000000000000000000000000000000011111111111111111111111111100000
It feit is dat alle berekkeningen foarkomme mei it 32-bit net-ûndertekene type. En allinich dan sil dit 32-bit net-ûndertekene type ymplisyt wurde útwreide nei uint64_t. Yn dit gefal sille de meast wichtige bits nul wêze.
Jo kinne de situaasje sa reparearje:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Fergelykbere situaasje: V629 [CWE-190] Beskôgje it ynspektearjen fan de 'Immr << 6'-ekspresje. Bitferskowing fan 'e 32-bit wearde mei in folgjende útwreiding nei it 64-bit type. AArch64AddressingModes.h 269
Fragmint N19: Missing keyword oars?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token.
// Skip it.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Handle optional arguments
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Invalid operand type");
}
....
}
PVS-Studio warskôging:
Der is gjin flater hjir. Sûnt it doe-blok fan 'e earste if einiget mei trochgean, dan makket it neat út, der is in kaaiwurd oars of net. Hoe dan ek, de koade sil itselde wurkje. Noch mist oars makket de koade ûndúdliker en gefaarliker. As yn 'e takomst trochgean ferdwynt, sil de koade folslein oars begjinne te wurkjen. Neffens my is it better om te foegjen oars.
Fragmint N20: Fjouwer typflaters fan itselde type
LLVM_DUMP_METHOD void Symbol::dump(raw_ostream &OS) const {
std::string Result;
if (isUndefined())
Result += "(undef) ";
if (isWeakDefined())
Result += "(weak-def) ";
if (isWeakReferenced())
Result += "(weak-ref) ";
if (isThreadLocalValue())
Result += "(tlv) ";
switch (Kind) {
case SymbolKind::GlobalSymbol:
Result + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClass:
Result + "(ObjC Class) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClassEHType:
Result + "(ObjC Class EH) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCInstanceVariable:
Result + "(ObjC IVar) " + Name.str(); // <=
break;
}
OS << Result;
}
PVS-Studio warskôgings:
- V655 [CWE-480] De snaren waarden gearfoege, mar wurde net brûkt. Tink oan it ynspektearjen fan 'Result + Name.str()'-ekspresje. Symbol.cpp 32
- V655 [CWE-480] De snaren waarden gearfoege, mar wurde net brûkt. Tink oan it ynspektearjen fan de 'Resultaat + "(ObjC Class)" + Name.str()' ekspresje. Symbol.cpp 35
- V655 [CWE-480] De snaren waarden gearfoege, mar wurde net brûkt. Tink derom om de 'Resultaat + "(ObjC Class EH) " + Name.str()'-ekspresje te ynspektearjen. Symbol.cpp 38
- V655 [CWE-480] De snaren waarden gearfoege, mar wurde net brûkt. Tink oan it ynspektearjen fan de 'Resultaat + "(ObjC IVar)" + Name.str()' ekspresje. Symbol.cpp 41
By ûngelok wurdt de + operator brûkt ynstee fan de += operator. It resultaat is ûntwerpen dy't sûnder betsjutting binne.
Fragmint N21: Undefiniearre gedrach
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();
}
}
}
Besykje sels de gefaarlike koade te finen. En dit is in foto om de oandacht ôf te lieden om net fuortendaliks nei it antwurd te sjen:
PVS-Studio warskôging:
Probleem line:
FeaturesMap[Op] = FeaturesMap.size();
As elemint Op is net fûn, dan wurdt in nij elemint oanmakke yn 'e kaart en it oantal eleminten yn dizze kaart wurdt dêr skreaun. It is gewoan ûnbekend oft de funksje oanroppen wurdt grutte foar of nei it tafoegjen fan in nij elemint.
Fragmint N22-N24: Werhelle opdrachten
Error MachOObjectFile::checkSymbolTable() const {
....
} else {
MachO::nlist STE = getSymbolTableEntry(SymDRI);
NType = STE.n_type; // <=
NType = STE.n_type; // <=
NSect = STE.n_sect;
NDesc = STE.n_desc;
NStrx = STE.n_strx;
NValue = STE.n_value;
}
....
}
PVS-Studio warskôging:
Ik tink net dat hjir in echte flater is. Gewoan in ûnnedige werhelle opdracht. Mar dochs in blunder.
Likegoed:
- V519 [CWE-563] De 'B.NDesc' fariabele wurdt twa kear opienfolgjende wearden tawiisd. Miskien is dit in flater. Kontrolearje rigels: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] De fariabele wurdt twa kear opienfolgjende wearden tawiisd. Miskien is dit in flater. Kontrolearje rigels: 59, 61. coff2yaml.cpp 61
Fragmint N25-N27: Mear reassignments
Litte wy no sjen nei in wat oare ferzje fan weryndieling.
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 warskôging: V519 [CWE-563] De 'Alignment' fariabele wurdt twa kear opienfolgjende wearden tawiisd. Miskien is dit in flater. Kontrolearje rigels: 1158, 1160. LoadStoreVectorizer.cpp 1160
Dit is hiel frjemd koade dy't blykber befettet in logyske flater. Oan it begjin, fariabele alignment in wearde wurdt tawiisd ôfhinklik fan de betingst. En dan komt de opdracht wer foar, mar no sûnder kontrôle.
Fergelykbere situaasjes kinne hjir sjoen wurde:
- V519 [CWE-563] De fariabele 'Effekten' wurdt twa kear opienfolgjende wearden tawiisd. Miskien is dit in flater. Kontrolearje rigels: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] De fariabele 'ExpectNoDerefChunk' wurdt twa kear opienfolgjende wearden tawiisd. Miskien is dit in flater. Kontrolearje rigels: 4970, 4973. SemaType.cpp 4973
Fragmint N28: Altyd wiere betingst
static int readPrefixes(struct InternalInstruction* insn) {
....
uint8_t byte = 0;
uint8_t nextByte;
....
if (byte == 0xf3 && (nextByte == 0x88 || nextByte == 0x89 ||
nextByte == 0xc6 || nextByte == 0xc7)) {
insn->xAcquireRelease = true;
if (nextByte != 0x90) // PAUSE instruction support // <=
break;
}
....
}
PVS-Studio warskôging:
Kontrolearje hat gjin sin. Fariabel nextByte altyd net gelyk oan de wearde 0x90, dy't folget út 'e foarige kontrôle. Dit is in soarte fan logyske flater.
Fragmint N29 - N...: Altyd wiere/falske betingsten
De analysator jout in protte warskôgings út dat de heule betingst (
static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
uint64_t Address, const void *Decoder) {
DecodeStatus S = MCDisassembler::Success;
if (RegNo > 13)
return MCDisassembler::Fail;
if ((RegNo & 1) || RegNo == 0xe)
S = MCDisassembler::SoftFail;
....
}
PVS-Studio warskôging:
De konstante 0xE is de wearde 14 yn desimaal. Eksamen RegNo == 0xe makket gjin sin want as RegNo > 13, dan sil de funksje syn útfiering foltôgje.
Der wiene in protte oare warskôgings mei IDs V547 en V560, mar as mei
Ik sil jo in foarbyld jaan wêrom't it studearjen fan dizze triggers saai is. De analysator is perfoarst rjocht by it útjaan fan in warskôging foar de folgjende koade. Mar dit is gjin flater.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio Warskôging: V547 [CWE-570] Ekspresje '!HasError' is altyd falsk. UnwrappedLineParser.cpp 1635
Fragmint N30: Fertochte werom
static bool
isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) {
for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg),
E = MRI.def_instr_end(); It != E; ++It) {
return (*It).isImplicitDef();
}
....
}
PVS-Studio warskôging:
Dit is of in flater of in spesifike technyk dy't bedoeld is om wat te ferklearjen oan programmeurs dy't de koade lêze. Dit ûntwerp ferklearret my neat en sjocht heul fertocht. It is better om net sa te skriuwen :).
Wurch? Dan is it tiid om tee of kofje te meitsjen.
Defekten identifisearre troch nije diagnostyk
Ik tink dat 30 aktivearrings fan âlde diagnostyk genôch is. Litte wy no sjen wat nijsgjirrige dingen kinne wurde fûn mei de nije diagnostyk dy't ferskynde yn 'e analyzer nei
Fragmint N31: Unberikber koade
Error CtorDtorRunner::run() {
....
if (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}
PVS-Studio warskôging:
Sa't jo sjen kinne, beide tûken fan de operator if einiget mei in oprop oan de operator werom. As gefolch, de kontener CtorDtorsByPriority sil nea wurde wiske.
Fragmint N32: Unberikber koade
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 warskôging: V779 [CWE-561] Unbereikbere koade ûntdutsen. It is mooglik dat der in flater is. LLParser.cpp 835
Nijsgjirrige situaasje. Litte wy earst nei dit plak sjen:
return ParseTypeIdEntry(SummaryID);
break;
Op it earste each liket it derop dat hjir gjin flater is. It liket de operator brekke der is in ekstra ien hjir, en do kinst gewoan wiskje it. Lykwols, net allegear sa ienfâldich.
De analysator jout in warskôging op 'e rigels:
Lex.setIgnoreColonInIdentifiers(false);
return false;
En yndie, dizze koade is net te berikken. Alle gefallen yn omskeakelje einiget mei in oprop fan de operator werom. En no sinleas allinne brekke sjocht der net sa ûnskuldich út! Miskien moat ien fan 'e tûken einigje mei brekke, net oan werom?
Fragmint N33: Willekeurich weromsette fan hege bits
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}
PVS-Studio warskôging:
Tink derom dat de funksje getStubAlignment jout type net ûndertekene. Litte wy de wearde fan 'e útdrukking berekkenje, oannommen dat de funksje de wearde 8 werombringt:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
No merken dat de fariabele DataSize hat in 64-bit net-ûndertekene type. It docht bliken dat by it útfieren fan de DataSize & 0xFFFFFFF8u operaasje, alle twa en tritich hege-oarder bits sille wurde weromset nei nul. Meast wierskynlik, dit is net wat de programmeur woe. Ik fermoedzje dat hy woe berekkenje: DataSize & 0xFFFFFFFFFFFFFFFFF8u.
Om de flater te reparearjen, moatte jo dit skriuwe:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Of sa:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragmint N34: Mislearre eksplisite type cast
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Unexpected scaling factor");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}
PVS-Studio warskôging:
Eksplisite type casting wurdt brûkt om oerstreaming te foarkommen by it fermannichfâldigjen fan type fariabelen int. Lykwols, eksplisite type casting hjir net beskermje tsjin oerstreaming. Earst wurde de fariabelen fermannichfâldige, en pas dan sil it 32-bit resultaat fan 'e fermannichfâldigje útwreide wurde nei it type
Fragmint N35: mislearre kopiearje-plakke
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;
}
....
}
Dizze nije nijsgjirrige diagnostyk identifisearret situaasjes dêr't in stik koade is kopiearre en guon nammen binne begûn te feroarjen yn it, mar op ien plak se hawwe net korrizjearre.
Tink derom dat se yn it twadde blok feroare binne Op 0 op Op 1. Mar op ien plak ha se it net reparearre. Meast wierskynlik hie it sa skreaun wêze moatten:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragmint N36: Fariabele ferwarring
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio warskôging:
It is tige gefaarlik om funksje-arguminten deselde nammen te jaan as klasseleden. It is heul maklik om yn 'e war te kommen. Wy hawwe krekt sa'n gefal foar ús. Dizze útdrukking makket gjin sin:
Mode &= Mask;
De funksje argumint feroaret. Da's alles. Dit argumint wurdt net mear brûkt. Meast wierskynlik moatte jo it sa skreaun hawwe:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragmint N37: Fariabele ferwarring
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;
}
Warskôging PVS-Studio: V1001 [CWE-563] De fariabele 'Grutte' wurdt tawiisd, mar wurdt net brûkt troch de ein fan 'e funksje. Object.cpp 424
De situaasje is fergelykber mei de foarige. It moat skreaun wurde:
this->Size += this->EntrySize;
Fragmint N38-N47: Se fergeaten de yndeks te kontrolearjen
Earder seagen wy nei foarbylden fan diagnostyske triggering
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 warskôging: V1004 [CWE-476] De 'Ptr'-oanwizer waard ûnfeilich brûkt nei't it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 729, 738. TargetTransformInfoImpl.h 738
Variable Ptr kin gelyk wêze nullptr, sa't bliken docht út de kontrôle:
if (Ptr != nullptr)
Under dizze oanwizer wurdt lykwols sûnder foarôfgeand kontrolearjen derferfereard:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Litte wy in oare ferlykbere saak beskôgje.
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 warskôging: V1004 [CWE-476] De 'FD'-oanwizer waard ûnfeilich brûkt nei't it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 3228, 3231. CGDebugInfo.cpp 3231
Jou omtinken oan it teken FD. Ik bin der wis fan dat it probleem dúdlik sichtber is en gjin spesjale útlis is nedich.
En fierder:
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 warskôging: V1004 [CWE-476] De 'PtrTy'-oanwizer waard ûnfeilich brûkt nei't it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 960, 965. InterleavedLoadCombinePass.cpp 965
Hoe te beskermjen dysels út sokke flaters? Wês opmerkliker op Code-Review en brûk de statyske analysator PVS-Studio om jo koade regelmjittich te kontrolearjen.
D'r is gjin punt om oare koadefragminten te neamen mei flaters fan dit type. Ik sil allinich in list mei warskôgingen yn it artikel litte:
- V1004 [CWE-476] De 'Expr'-oanwizer waard ûnfeilich brûkt nei't it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] De 'PI'-oanwizer waard ûnfeilich brûkt neidat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] De 'StatepointCall'-oanwizer waard ûnfeilich brûkt nei't it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] De 'RV'-oanwizer waard ûnfeilich brûkt neidat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] De 'CalleeFn' oanwizer waard ûnfeilich brûkt neidat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] De 'TC'-oanwizer waard ûnfeilich brûkt neidat it waard ferifiearre tsjin nullptr. Kontrolearje rigels: 1819, 1824. Driver.cpp 1824
Fragmint N48-N60: Net kritysk, mar in defekt (mooglik ûnthâldlek)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio warskôging:
Om in elemint ta te foegjen oan 'e ein fan in kontener lykas std ::vektor > do kinst net samar skriuwe xxx.push_back(nije X), sûnt der is gjin ymplisite bekearing fan X* в std::unyk_ptr.
In mienskiplike oplossing is om te skriuwen xxx.emplace_back(nije X)sûnt it compiles: metoade emplace_back konstruearret in elemint direkt út syn arguminten en kin dêrom eksplisite constructors brûke.
It is net feilich. As de vector is fol, dan ûnthâld wurdt opnij tawiisd. De operaasje foar ûnthâldherallokaasje kin mislearje, wêrtroch in útsûndering wurdt smiten std::bad_alloc. Yn dit gefal sil de oanwizer ferlern gean en it oanmakke objekt sil nea wurde wiske.
In feilige oplossing is te meitsjen unyk_ptrdy't de oanwizer sil hawwe foardat de fektor besiket ûnthâld te realisearjen:
xxx.push_back(std::unique_ptr<X>(new X))
Sûnt C++14 kinne jo 'std::make_unique' brûke:
xxx.push_back(std::make_unique<X>())
Dit soarte fan defekt is net kritysk foar LLVM. As ûnthâld kin net wurde tawiisd, sil de kompilator gewoan stopje. Lykwols, foar applikaasjes mei lange
Dat, hoewol dizze koade gjin praktyske bedriging foar LLVM foarmet, fûn ik it nuttich om te praten oer dit flaterpatroan en dat de PVS-Studio-analyzer hat leard om it te identifisearjen.
Oare warskôgings fan dit type:
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Passes' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. PassManager.h 546
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'AAs' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. AliasAnalysis.h 324
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de kontener 'Yngongen' troch de metoade 'emplace_back'. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'AllEdges' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. CFGMST.h 268
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan 'e 'VMaps' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. SimpleLoopUswitch.cpp 2012
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Records' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. FDRLogBuilder.h 30
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'PendingSubmodules' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. ModuleMap.cpp 810
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Objekten' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. DebugMap.cpp 88
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan 'e kontener 'Strategyen' troch de metoade 'emplace_back'. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 685
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 686
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 688
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 689
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 690
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 691
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 692
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 693
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Modifiers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. llvm-stress.cpp 694
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Operands' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Stash' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] In oanwizer sûnder eigner wurdt tafoege oan de 'Matchers' kontener troch de 'emplace_back' metoade. In ûnthâld lek sil foarkomme yn gefal fan in útsûndering. GlobalISelEmitter.cpp 2702
konklúzje
Ik joech yn totaal 60 warskôgingen en stoppe doe. Binne d'r oare defekten dy't de PVS-Studio-analyzer detektearret yn LLVM? Ja ik haw. Doe't ik lykwols koadefragminten foar it artikel skreau, wie it jûns let, of leaver sels nacht, en ik besleat dat it tiid wie om it in dei te neamen.
Ik hoopje dat jo it ynteressant fûnen en de PVS-Studio-analysator wolle besykje.
Jo kinne de analyzer downloade en de minesweeper-kaai krije by
It wichtichste, brûk regelmjittich statyske analyse. Ienmalige kontrôles, útfierd troch ús om de metodyk fan statyske analyze te popularisearjen en PVS-Studio binne gjin normaal senario.
Good luck yn it ferbetterjen fan de kwaliteit en betrouberens fan jo koade!
As jo dit artikel wolle diele mei in Ingelsktalig publyk, brûk dan de oersettingskeppeling: Andrey Karpov.
Boarne: www.habr.com