Ir pagājuši vairāk nekā divi gadi kopš pēdējās LLVM projekta koda pārbaudes, izmantojot mūsu PVS-Studio analizatoru. Pārliecināsimies, ka PVS-Studio analizators joprojām ir vadošais rīks kļūdu un iespējamo ievainojamību identificēšanai. Lai to izdarītu, mēs pārbaudīsim un atradīsim jaunas kļūdas LLVM 8.0.0 laidienā.
Raksts jāraksta
Godīgi sakot, es negribēju rakstīt šo rakstu. Nav interesanti rakstīt par projektu, ko esam jau vairākas reizes pārbaudījuši (
Katru reizi, kad tiek izlaista vai atjaunināta jauna LLVM versija
Skatieties, jaunā Clang Static Analyzer versija ir iemācījusies atrast jaunas kļūdas! Man šķiet, ka PVS-Studio izmantošanas aktualitāte samazinās. Clang atrod vairāk kļūdu nekā iepriekš un panāk PVS-Studio iespējas. Ko Tu domā par šo?
Uz šo es vienmēr gribu atbildēt kaut ko līdzīgu:
Mēs arī nesēžam dīkā! Esam ievērojami uzlabojuši PVS-Studio analizatora iespējas. Tāpēc neuztraucieties, mēs turpinām vadīt tāpat kā iepriekš.
Diemžēl šī ir slikta atbilde. Tajā nav pierādījumu. Un tāpēc es tagad rakstu šo rakstu. Tātad LLVM projekts kārtējo reizi ir pārbaudīts un tajā konstatētas dažādas kļūdas. Tagad es demonstrēšu tos, kas man šķita interesanti. Clang Static Analyzer nevar atrast šīs kļūdas (vai arī ar tā palīdzību to izdarīt ir ārkārtīgi neērti). Bet mēs varam. Turklāt visas šīs kļūdas atradu un pierakstīju vienā vakarā.
Taču raksta tapšana aizņēma vairākas nedēļas. Es vienkārši nevarēju to visu ievietot tekstā :).
Starp citu, ja jūs interesē, kādas tehnoloģijas tiek izmantotas PVS-Studio analizatorā, lai identificētu kļūdas un iespējamās ievainojamības, tad iesaku iepazīties ar šo
Jauna un veca diagnostika
Kā jau minēts, pirms aptuveni diviem gadiem LLVM projekts tika vēlreiz pārbaudīts un konstatētās kļūdas labotas. Tagad šajā rakstā tiks parādīta jauna kļūdu grupa. Kāpēc tika atrastas jaunas kļūdas? Tam ir 3 iemesli:
- LLVM projekts attīstās, mainot veco kodu un pievienojot jaunu kodu. Protams, modificētajā un rakstītajā kodā ir jaunas kļūdas. Tas skaidri parāda, ka statiskā analīze ir jāizmanto regulāri, nevis reizēm. Mūsu raksti labi parāda PVS-Studio analizatora iespējas, taču tam nav nekā kopīga ar koda kvalitātes uzlabošanu un kļūdu labošanas izmaksu samazināšanu. Regulāri izmantojiet statiskā koda analizatoru!
- Mēs pabeidzam un uzlabojam esošo diagnostiku. Tāpēc analizators var identificēt kļūdas, kuras tas nepamanīja iepriekšējo skenēšanas laikā.
- PVS-Studio ir parādījusies jauna diagnostika, kas neeksistēja pirms 2 gadiem. Es nolēmu tos izcelt atsevišķā sadaļā, lai skaidri parādītu PVS-Studio attīstību.
Diagnostikas rezultātā konstatēti defekti, kas bijuši pirms 2 gadiem
Fragments N1: kopēt un ielīmēt
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 brīdinājums:
Ir divreiz pārbaudīts, vai nosaukums sākas ar apakšvirkni "avx512.mask.permvar.". Otrajā pārbaudē acīmredzot gribēja rakstīt ko citu, bet aizmirsa izlabot nokopēto tekstu.
Fragments N2: drukas kļūda
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;
....
}
Brīdinājums PVS-Studio: V501 Ir identiskas apakšizteiksmes 'CXNameRange_WantQalifier' pa kreisi un pa labi no '|' operators. CIndex.cpp 7245
Drukas kļūdas dēļ viena un tā pati nosauktā konstante tiek izmantota divas reizes CXNameRange_WantQalifier.
Fragments N3: neskaidrības ar operatora prioritāti
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio brīdinājums:
Manuprāt, tā ir ļoti skaista kļūda. Jā, es zinu, ka man ir dīvaini priekšstati par skaistumu :).
Tagad, saskaņā ar
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
No praktiskā viedokļa šādam nosacījumam nav jēgas, jo to var samazināt līdz:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Tā ir skaidra kļūda. Visticamāk, viņi gribēja salīdzināt 0/1 ar mainīgo Indekss. Lai labotu kodu, trīskāršajam operatoram ir jāpievieno iekavas:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Starp citu, trīskāršais operators ir ļoti bīstams un provocē loģiskas kļūdas. Esiet ļoti uzmanīgs ar to un neesiet mantkārīgs ar iekavām. Es apskatīju šo tēmu sīkāk
Fragments N4, N5: Nulles rādītājs
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 brīdinājums:
Ja rādītājs LHS ir nulles, jāizsaka brīdinājums. Tomēr tā vietā tiks noņemta atsauce uz šo pašu nulles rādītāju: LHS->getAsString().
Šī ir ļoti tipiska situācija, kad kļūdu apstrādātājā ir paslēpta kļūda, jo neviens tos nepārbauda. Statiskie analizatori pārbauda visu sasniedzamo kodu neatkarīgi no tā, cik bieži tas tiek izmantots. Šis ir ļoti labs piemērs tam, kā statiskā analīze papildina citas testēšanas un kļūdu aizsardzības metodes.
Līdzīga rādītāja apstrādes kļūda RHS atļauts tieši zemāk esošajā kodā: V522 [CWE-476] Var notikt nulles rādītāja “RHS” atsauces atcelšana. TGParser.cpp 2186
Fragments N6: kursora lietošana pēc pārvietošanas
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 brīdinājums: V522 [CWE-476] Var notikt nulles rādītāja 'ProgClone' atsauces atcelšana. Miscompilation.cpp 601
Sākumā gudrs rādītājs ProgClone pārstāj piederēt objektam:
BD.setNewProgram(std::move(ProgClone));
Patiesībā tagad ProgClone ir nulles rādītājs. Tāpēc nulles rādītāja norādei jānotiek tieši zemāk:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Bet patiesībā tas nenotiks! Ņemiet vērā, ka cilpa faktiski netiek izpildīta.
Konteinera sākumā Nepareizi kompilētas funkcijas notīrīts:
MiscompiledFunctions.clear();
Tālāk tiek izmantots šī konteinera izmērs cilpas stāvoklī:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Ir viegli redzēt, ka cilpa nesākas. Es domāju, ka tā ir arī kļūda, un kods ir jāraksta savādāk.
Šķiet, ka esam saskārušies ar šo slaveno kļūdu paritāti! Viena kļūda maskē otru :).
Fragments N7: kursora lietošana pēc pārvietošanas
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 brīdinājums: V522 [CWE-476] Var notikt nulles rādītāja 'Test' atsauces atcelšana. Miscompilation.cpp 709
Atkal tā pati situācija. Sākumā objekta saturs tiek pārvietots, un tad tas tiek izmantots tā, it kā nekas nebūtu noticis. Arvien biežāk šo situāciju redzu programmas kodā pēc kustību semantikas parādīšanās C++. Tāpēc es mīlu C++ valodu! Ir arvien vairāk jaunu veidu, kā nošaut sev kāju. PVS-Studio analizatoram vienmēr būs darbs :).
Fragments N8: nulles rādītājs
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 brīdinājums: V522 [CWE-476] Var notikt nulles rādītāja “Tips” atsauces atcelšana. PrettyFunctionDumper.cpp 233
Papildus kļūdu apstrādātājiem parasti netiek pārbaudītas atkļūdošanas izdrukas funkcijas. Mums ir tieši tāds gadījums. Funkcija gaida lietotāju, kurš tā vietā, lai atrisinātu savas problēmas, būs spiests to labot.
Pareizi:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragments N9: nulles rādītājs
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 brīdinājums: V522 [CWE-476] Var notikt nulles rādītāja 'Ty' atsauces atcelšana. SearchableTableEmitter.cpp 614
Es domāju, ka viss ir skaidrs un neprasa paskaidrojumus.
Fragments N10: drukas kļūda
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 brīdinājums:
Nav jēgas piešķirt sev mainīgo. Visticamāk, viņi gribēja rakstīt:
Identifier->Type = Question->Type;
Fragments N11: aizdomīgs pārtraukums
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 brīdinājums:
Sākumā ir ļoti aizdomīgs operators pārtraukums. Vai aizmirsāt šeit uzrakstīt vēl kaut ko?
Fragments N12: rādītāja pārbaude pēc atsauces noņemšanas
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 brīdinājums:
Rādītājs Callee sākumā tiek atcelta atsauce funkcijas izsaukšanas brīdī getTTI.
Un tad izrādās, ka šī rādītāja vienlīdzība ir jāpārbauda nullptr:
if (!Callee || Callee->isDeclaration())
Bet ir par vēlu…
Fragments N13 - N...: rādītāja pārbaude pēc atsauces noņemšanas
Iepriekšējā koda fragmentā aplūkotā situācija nav unikāla. Tas parādās šeit:
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 brīdinājums: V595 [CWE-476] 'CalleeFn' rādītājs tika izmantots, pirms tas tika pārbaudīts pret nullptr. Pārbaudes rindiņas: 1079, 1081. SimplifyLibCalls.cpp 1079
Un šeit:
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 brīdinājums: V595 [CWE-476] 'ND' rādītājs tika izmantots, pirms tika pārbaudīts, vai tas ir pret nullptr. Pārbaudes rindiņas: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Un šeit:
- V595 [CWE-476] “U” rādītājs tika izmantots, pirms tas tika pārbaudīts pret nullptr. Pārbaudes rindiņas: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND' rādītājs tika izmantots, pirms tika pārbaudīts, vai tas atbilst nullptr. Pārbaudes rindiņas: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Un tad man kļuva neinteresanti pētīt brīdinājumus ar numuru V595. Tāpēc es nezinu, vai bez šeit uzskaitītajām ir vēl līdzīgas kļūdas. Visticamāk, ka ir.
Fragments N17, N18: aizdomīga nobīde
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio brīdinājums:
Tā var nebūt kļūda, un kods darbojas tieši tā, kā paredzēts. Bet šī noteikti ir ļoti aizdomīga vieta, un tā ir jāpārbauda.
Teiksim mainīgais Izmēri ir vienāds ar 16, un tad koda autors plānoja to iegūt mainīgā NImms nozīme:
1111111111111111111111111111111111111111111111111111111111100000
Tomēr patiesībā rezultāts būs:
0000000000000000000000000000000011111111111111111111111111100000
Fakts ir tāds, ka visi aprēķini tiek veikti, izmantojot 32 bitu neparakstīto tipu. Un tikai tad šis 32 bitu neparakstītais tips tiks netieši paplašināts līdz uint64_t. Šajā gadījumā nozīmīgākie biti būs nulle.
Jūs varat labot situāciju šādi:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Līdzīga situācija: V629 [CWE-190] Apsveriet iespēju pārbaudīt izteiksmi “Immr << 6”. 32 bitu vērtības bitu nobīde ar sekojošu paplašināšanu uz 64 bitu tipu. AArch64AddressingModes.h 269
Fragments N19: trūkst atslēgvārda cits?
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 brīdinājums:
Šeit nav kļūdu. Kopš toreizējā bloka pirmā if beidzas ar turpināt, tad tas nav svarīgi, ir atslēgvārds cits vai nē. Jebkurā gadījumā kods darbosies vienādi. Joprojām garām cits padara kodu neskaidrāku un bīstamāku. Ja nākotnē turpināt pazūd, kods sāks darboties pavisam citādi. Manuprāt, labāk ir pievienot cits.
Fragments N20: četras viena veida drukas kļūdas
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 brīdinājumi:
- V655 [CWE-480] Virknes tika savienotas, bet netiek izmantotas. Apsveriet iespēju pārbaudīt izteiksmi "Result + Name.str()". Simbols.cpp 32
- V655 [CWE-480] Virknes tika savienotas, bet netiek izmantotas. Apsveriet iespēju pārbaudīt izteiksmi 'Result + "(ObjC Class)" + Name.str()'. Simbols.cpp 35
- V655 [CWE-480] Virknes tika savienotas, bet netiek izmantotas. Apsveriet iespēju pārbaudīt izteiksmi 'Result + "(ObjC Class EH)" + Name.str()'. Simbols.cpp 38
- V655 [CWE-480] Virknes tika savienotas, bet netiek izmantotas. Apsveriet iespēju pārbaudīt izteiksmi 'Result + "(ObjC Ivar)" + Name.str()'. Simbols.cpp 41
Nejauši operators + tiek izmantots operatora += vietā. Rezultāts ir dizaini, kuriem nav nozīmes.
Fragments N21: nenoteikta darbība
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();
}
}
}
Mēģiniet pats atrast bīstamo kodu. Un šis ir attēls, lai novērstu uzmanību, lai uzreiz neskatītos uz atbildi:
PVS-Studio brīdinājums:
Problēmas līnija:
FeaturesMap[Op] = FeaturesMap.size();
Ja elements Op nav atrasts, tad kartē tiek izveidots jauns elements un tur tiek ierakstīts elementu skaits šajā kartē. Tikai nav zināms, vai funkcija tiks izsaukta izmērs pirms vai pēc jauna elementa pievienošanas.
Fragments N22-N24: Atkārtoti uzdevumi
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 brīdinājums:
Es domāju, ka šeit nav īstas kļūdas. Vienkārši nevajadzīgs atkārtots uzdevums. Bet vienalga kļūda.
Tāpat:
- V519 [CWE-563] Mainīgajam “B.NDesc” tiek piešķirtas vērtības divas reizes pēc kārtas. Varbūt tā ir kļūda. Pārbaudes rindas: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Mainīgajam tiek piešķirtas vērtības divas reizes pēc kārtas. Varbūt tā ir kļūda. Pārbaudes rindiņas: 59, 61. coff2yaml.cpp 61
Fragments N25-N27: vairāk pārdalīšanas
Tagad apskatīsim nedaudz atšķirīgu pārcelšanas versiju.
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 brīdinājums: V519 [CWE-563] Mainīgajam “Izlīdzināšana” tiek piešķirtas vērtības divas reizes pēc kārtas. Varbūt tā ir kļūda. Pārbaudes rindas: 1158, 1160. LoadStoreVectorizer.cpp 1160
Šis ir ļoti dīvains kods, kas acīmredzot satur loģisku kļūdu. Sākumā mainīgs Noregulēšana vērtība tiek piešķirta atkarībā no stāvokļa. Un tad uzdevums notiek vēlreiz, bet tagad bez jebkādas pārbaudes.
Līdzīgas situācijas var redzēt šeit:
- V519 [CWE-563] Mainīgajam “Efekti” tiek piešķirtas vērtības divas reizes pēc kārtas. Varbūt tā ir kļūda. Pārbaudes rindiņas: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Mainīgajam “ExpectNoDerefChunk” tiek piešķirtas vērtības divas reizes pēc kārtas. Varbūt tā ir kļūda. Pārbaudes rindas: 4970, 4973. SemaType.cpp 4973
Fragments N28: Vienmēr patiess stāvoklis
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 brīdinājums:
Pārbaudīt nav jēgas. Mainīgs nextByte vienmēr nav vienāds ar vērtību 0x90, kas izriet no iepriekšējās pārbaudes. Tā ir sava veida loģiska kļūda.
Fragments N29 - N...: Vienmēr patiesi/nepatiesi nosacījumi
Analizators izdod daudzus brīdinājumus, ka viss stāvoklis (
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 brīdinājums:
Konstante 0xE ir vērtība 14 decimāldaļās. Pārbaude RegNr == 0xe nav jēgas, jo ja Reģ.nr. > 13, tad funkcija pabeigs tās izpildi.
Bija daudz citu brdinjumu ar ID V547 un V560, bet k ar
Es sniegšu jums piemēru, kāpēc šo trigeru izpēte ir garlaicīga. Analizatoram ir pilnīga taisnība, izdodot brīdinājumu par šādu kodu. Bet tā nav kļūda.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio brīdinājums: V547 [CWE-570] Izteiksme “!HasError” vienmēr ir nepatiesa. UnwrappedLineParser.cpp 1635
Fragments N30: aizdomīga atgriešanās
static bool
isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) {
for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg),
E = MRI.def_instr_end(); It != E; ++It) {
return (*It).isImplicitDef();
}
....
}
PVS-Studio brīdinājums:
Tā ir kļūda vai īpaša tehnika, kas paredzēta, lai kaut ko izskaidrotu programmētājiem, kas lasa kodu. Šis dizains man neko neizskaidro un izskatās ļoti aizdomīgs. Labāk tā nerakstīt :).
Noguris? Tad ir pienācis laiks pagatavot tēju vai kafiju.
Defekti, kas konstatēti pēc jaunas diagnostikas
Domāju, ka pietiek ar 30 vecās diagnostikas aktivizējumiem. Tagad paskatīsimies, ko interesantu var atrast ar jauno diagnostiku, kas analizatorā parādījās pēc tam
Fragments N31: nesasniedzams kods
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 brīdinājums:
Kā redzat, abas operatora filiāles if beidzas ar zvanu operatoram atgriešanās. Attiecīgi konteiners CtorDtorsByPriority nekad netiks iztīrīts.
Fragments N32: nesasniedzams kods
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 brīdinājums: V779 [CWE-561] Konstatēts nesasniedzams kods. Iespējams, ka ir radusies kļūda. LLParser.cpp 835
Interesanta situācija. Vispirms apskatīsim šo vietu:
return ParseTypeIdEntry(SummaryID);
break;
No pirmā acu uzmetiena šķiet, ka šeit nav nekādas kļūdas. Izskatās pēc operatora pārtraukums šeit ir papildu, un jūs varat to vienkārši izdzēst. Tomēr ne viss ir tik vienkārši.
Analizators izdod brīdinājumu uz līnijām:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Un tiešām, šis kods nav sasniedzams. Visi gadījumi iekšā slēdzis beidzas ar operatora zvanu atgriešanās. Un tagad bezjēdzīgi vienatnē pārtraukums neizskatās tik nekaitīgs! Varbūt vienam no zariem vajadzētu beigties ar pārtraukumsbet ne atgriešanās?
Fragments N33: lielu bitu nejauša atiestatīšana
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 brīdinājums:
Lūdzu, ņemiet vērā, ka funkcija getStubAlignment atgriešanas veids neparakstīts. Aprēķināsim izteiksmes vērtību, pieņemot, ka funkcija atgriež vērtību 8:
~(getStubAlignment() - 1)
~ (8u-1)
0xFFFFFFFF8u
Tagad ievērojiet, ka mainīgais DataSize ir 64 bitu neparakstīts tips. Izrādās, ka, veicot DataSize & 0xFFFFFFF8u darbību, visi trīsdesmit divi augstākās kārtas biti tiks atiestatīti uz nulli. Visticamāk, tas nav tas, ko programmētājs vēlējās. Man ir aizdomas, ka viņš gribēja aprēķināt: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Lai labotu kļūdu, jums vajadzētu rakstīt šo:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Vai arī:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragments N34: neizdevās skaidra tipa apraide
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 brīdinājums:
Skaidra tipa izliešana tiek izmantota, lai izvairītos no pārplūdes, reizinot tipa mainīgos int. Tomēr skaidra tipa liešana šeit nepasargā no pārplūdes. Pirmkārt, mainīgie tiks reizināti, un tikai pēc tam reizināšanas 32 bitu rezultāts tiks paplašināts līdz veidam
Fragments N35: neizdevās kopēt un ielīmēt
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;
}
....
}
Šī jaunā interesantā diagnostika identificē situācijas, kad koda gabals ir nokopēts un daži nosaukumi tajā ir sākti mainīt, bet vienuviet tie nav izlaboti.
Lūdzu, ņemiet vērā, ka otrajā blokā tie mainījās Op0 par Op1. Bet vienā vietā viņi to nelaboja. Visticamāk, to vajadzēja rakstīt šādi:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragments N36: mainīgs apjukums
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio brīdinājums:
Ir ļoti bīstami funkciju argumentiem piešķirt tādus pašus nosaukumus kā klases dalībniekiem. Ir ļoti viegli apjukt. Mums ir tieši tāds gadījums. Šim izteicienam nav jēgas:
Mode &= Mask;
Funkcijas arguments mainās. Tas ir viss. Šis arguments vairs netiek izmantots. Visticamāk, jums to vajadzēja rakstīt šādi:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragments N37: mainīgs apjukums
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;
}
Brīdinājums PVS-Studio: V1001 [CWE-563] Mainīgais “Izmērs” ir piešķirts, bet netiek izmantots līdz funkcijas beigām. Object.cpp 424
Situācija ir līdzīga iepriekšējai. Būtu jāraksta:
this->Size += this->EntrySize;
Fragments N38-N47: viņi aizmirsa pārbaudīt indeksu
Iepriekš mēs apskatījām diagnostikas aktivizēšanas piemērus
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 brīdinājums: V1004 [CWE-476] “Ptr” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts, vai tas ir pret nullptr. Pārbaudes rindas: 729, 738. TargetTransformInfoImpl.h 738
Mainīgs Ptr var būt vienādi nullptr, par ko liecina čeks:
if (Ptr != nullptr)
Tomēr zem šī rādītāja tiek noņemta atsauce bez iepriekšējas pārbaudes:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Apskatīsim citu līdzīgu gadījumu.
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 brīdinājums: V1004 [CWE-476] “FD” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts, vai tas ir pret nullptr. Pārbaudes rindas: 3228, 3231. CGDebugInfo.cpp 3231
Pievērsiet uzmanību zīmei FD. Esmu pārliecināts, ka problēma ir skaidri redzama un nav nepieciešams īpašs skaidrojums.
Un tālāk:
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 brīdinājums: V1004 [CWE-476] “PtrTy” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts pret nullptr. Pārbaudes rindiņas: 960, 965. InterleavedLoadCombinePass.cpp 965
Kā pasargāt sevi no šādām kļūdām? Esiet uzmanīgāks Code-Review un izmantojiet PVS-Studio statisko analizatoru, lai regulāri pārbaudītu savu kodu.
Nav jēgas citēt citus koda fragmentus ar šāda veida kļūdām. Rakstā atstāšu tikai brīdinājumu sarakstu:
- V1004 [CWE-476] Pēc tam, kad tā tika pārbaudīta pret nullptr, rādītājs Expr tika izmantots nedroši. Pārbaudes rindiņas: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] “PI” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts, vai tas ir pret nullptr. Pārbaudes rindiņas: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] StatepointCall rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts pret nullptr. Pārbaudes rindas: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] “RV” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts pret nullptr. Pārbaudes rindas: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] “CalleeFn” rādītājs tika nedroši izmantots pēc tam, kad tika pārbaudīts pret nullptr. Pārbaudes rindiņas: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] “TC” rādītājs tika izmantots nedroši pēc tam, kad tika pārbaudīts pret nullptr. Pārbaudes līnijas: 1819, 1824. Driver.cpp 1824
Fragments N48-N60: nav kritisks, bet defekts (iespējama atmiņas noplūde)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio brīdinājums:
Lai pievienotu elementu konteinera beigām, piemēram std::vektors > tu nevari vienkārši rakstīt xxx.push_back(jauns X), jo nav netiešas konvertēšanas no X* в std::unikāls_ptr.
Izplatīts risinājums ir rakstīt xxx.emplace_back(jauns X)jo tas apkopo: metode emplace_back konstruē elementu tieši no saviem argumentiem un tāpēc var izmantot skaidrus konstruktorus.
Tas nav droši. Ja vektors ir pilns, atmiņa tiek atkārtoti piešķirta. Atmiņas pārdalīšanas darbība var neizdoties, kā rezultātā var tikt parādīts izņēmums std::bad_alloc. Šajā gadījumā rādītājs tiks pazaudēts un izveidotais objekts nekad netiks izdzēsts.
Drošs risinājums ir izveidot unikāls_ptrkam pieder rādītājs, pirms vektors mēģinās pārdalīt atmiņu:
xxx.push_back(std::unique_ptr<X>(new X))
Kopš C++14 varat izmantot 'std::make_unique':
xxx.push_back(std::make_unique<X>())
Šāda veida defekts nav kritisks LLVM. Ja atmiņu nevar piešķirt, kompilators vienkārši apstāsies. Tomr lietojumiem ar garo
Tāpēc, lai gan šis kods praktiski neapdraud LLVM, man bija noderīgi runāt par šo kļūdu modeli un to, ka PVS-Studio analizators ir iemācījies to identificēt.
Citi šāda veida brīdinājumi:
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Apliecības”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. PassManager.h 546
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “AAs”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. AliasAnalysis.h 324
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Entries”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “AllEdges”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. CFGMST.h 268
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram "VMaps", izmantojot metodi "emplace_back". Izņēmuma gadījumā notiks atmiņas noplūde. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Records”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. FDRLogBuilder.h 30
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram "PendingSubmodules", izmantojot metodi "emplace_back". Izņēmuma gadījumā notiks atmiņas noplūde. ModuleMap.cpp 810
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram "Objekti", izmantojot metodi "emplace_back". Izņēmuma gadījumā notiks atmiņas noplūde. DebugMap.cpp 88
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram "Stratēģijas", izmantojot metodi "emplace_back". Izņēmuma gadījumā notiks atmiņas noplūde. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 685
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 686
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 688
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 689
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 690
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 691
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 692
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 693
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram 'Modifiers', izmantojot metodi 'emplace_back'. Izņēmuma gadījumā notiks atmiņas noplūde. llvm-stress.cpp 694
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Operands”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. GlobalISelEmitter.cpp 1911. gads
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Stash”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Rādītājs bez īpašnieka tiek pievienots konteineram “Atbilstības meklētāji”, izmantojot metodi “emplace_back”. Izņēmuma gadījumā notiks atmiņas noplūde. GlobalISelEmitter.cpp 2702
Secinājums
Kopumā es izteicu 60 brīdinājumus un pēc tam pārtraucu. Vai ir citi defekti, ko PVS-Studio analizators atklāj LLVM? Jā, man ir. Taču, kad rakstīju koda fragmentus, bija vēls vakars, pareizāk sakot, pat nakts, un es nolēmu, ka laiks to saukt par dienu.
Ceru, ka jums tas šķita interesanti un vēlēsities izmēģināt PVS-Studio analizatoru.
Jūs varat lejupielādēt analizatoru un iegūt mīnu meklētāja atslēgu vietnē
Vissvarīgākais ir regulāri izmantot statisko analīzi. Vienreizējas pārbaudes, ko veicam, lai popularizētu statiskās analīzes metodoloģiju un PVS-Studio nav normāls scenārijs.
Veiksmi koda kvalitātes un uzticamības uzlabošanā!
Ja vēlaties dalīties ar šo rakstu ar angliski runājošu auditoriju, lūdzu, izmantojiet tulkošanas saiti: Andrejs Karpovs.
Avots: www.habr.com