Praėjo daugiau nei dveji metai nuo paskutinio LLVM projekto kodo patikrinimo naudojant mūsų PVS-Studio analizatorių. Įsitikinkite, kad „PVS-Studio“ analizatorius vis dar yra pagrindinis klaidų ir galimų pažeidžiamumų nustatymo įrankis. Norėdami tai padaryti, mes patikrinsime ir rasime naujų klaidų LLVM 8.0.0 leidime.
Straipsnis rašomas
Jei atvirai, aš nenorėjau rašyti šio straipsnio. Neįdomu rašyti apie projektą, kurį jau kelis kartus tikrinome (
Kiekvieną kartą, kai išleidžiama arba atnaujinama nauja LLVM versija
Žiūrėkite, naujoji Clang Static Analyzer versija išmoko rasti naujų klaidų! Man atrodo, kad PVS-Studio naudojimo aktualumas mažėja. Clang randa daugiau klaidų nei anksčiau ir pasivijo PVS-Studio galimybes. Ką tu manai apie tai?
Į tai aš visada noriu atsakyti maždaug taip:
Mes taip pat nesėdime be darbo! Ženkliai patobulinome PVS-Studio analizatoriaus galimybes. Taigi nesijaudinkite, mes ir toliau pirmaujame kaip anksčiau.
Deja, tai blogas atsakymas. Jokių įrodymų jame nėra. Ir todėl dabar rašau šį straipsnį. Taigi, LLVM projektas dar kartą buvo patikrintas ir jame rasta įvairių klaidų. Dabar pademonstruosiu tuos, kurie man pasirodė įdomūs. Clang Static Analyzer šių klaidų neranda (arba su jo pagalba tai padaryti labai nepatogu). Bet mes galime. Be to, visas šias klaidas radau ir užsirašiau per vieną vakarą.
Tačiau straipsnio rašymas užtruko kelias savaites. Tiesiog negalėjau prisiversti viso šito į tekstą :).
Beje, jei domitės, kokios technologijos naudojamos PVS-Studio analizatoriuje identifikuojant klaidas ir galimus pažeidžiamumus, tuomet siūlau susipažinti su šiuo
Nauja ir sena diagnostika
Kaip jau minėta, prieš maždaug dvejus metus LLVM projektas buvo dar kartą patikrintas ir rastos klaidos ištaisytos. Dabar šiame straipsnyje bus pateikta nauja klaidų grupė. Kodėl buvo rasta naujų klaidų? Tam yra 3 priežastys:
- LLVM projektas vystosi, keičiamas senas kodas ir pridedamas naujas kodas. Natūralu, kad pakeistame ir parašytame kode yra naujų klaidų. Tai aiškiai parodo, kad statinė analizė turėtų būti naudojama reguliariai, o ne retkarčiais. Mūsų straipsniai puikiai parodo PVS-Studio analizatoriaus galimybes, tačiau tai neturi nieko bendra su kodo kokybės gerinimu ir klaidų taisymo išlaidų mažinimu. Reguliariai naudokite statinio kodo analizatorių!
- Baigiame ir tobuliname esamą diagnostiką. Todėl analizatorius gali nustatyti klaidas, kurių nepastebėjo ankstesnių nuskaitymų metu.
- PVS-Studijoje atsirado nauja diagnostika, kurios prieš 2 metus nebuvo. Nusprendžiau juos pabrėžti atskirame skyriuje, kad aiškiai parodyčiau PVS-Studio raidą.
Diagnostika nustatyti defektai, buvę prieš 2 metus
Fragmentas N1: nukopijuokite ir įklijuokite
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 įspėjimas:
Dar kartą patikrinama, ar pavadinimas prasideda poeilute „avx512.mask.permvar.“. Per antrąjį patikrinimą jie akivaizdžiai norėjo parašyti dar ką nors, bet pamiršo pataisyti nukopijuotą tekstą.
Fragmentas N2: rašybos klaida
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;
....
}
Įspėjimas PVS-Studio: V501 Kairėje ir dešinėje nuo '| operatorius. CIndex.cpp 7245
Dėl rašybos klaidos ta pati konstanta naudojama du kartus CXNameRange_WantQualifier.
Fragmentas N3: painiavos su operatoriaus pirmenybe
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio įspėjimas:
Mano nuomone, tai labai graži klaida. Taip, aš žinau, kad turiu keistų idėjų apie grožį :).
Dabar, pasak
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Praktiniu požiūriu tokia sąlyga nėra prasminga, nes ją galima sumažinti iki:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Tai aiški klaida. Greičiausiai jie norėjo palyginti 0/1 su kintamuoju rodyklė. Norėdami pataisyti kodą, turite pridėti skliaustus aplink trijų dalių operatorių:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Beje, trijų dalių operatorius yra labai pavojingas ir provokuoja logines klaidas. Būkite labai atsargūs su juo ir nebūkite godūs su skliaustais. Pažiūrėjau šią temą plačiau
Fragmentas N4, N5: Nulinė rodyklė
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 įspėjimas:
Jei rodyklė LHS yra niekinis, turėtų būti paskelbtas įspėjimas. Tačiau vietoj to ta pati nulinė rodyklė bus pašalinta: LHS->getAsString().
Tai labai tipiška situacija, kai klaida yra paslėpta klaidų tvarkyklėje, nes niekas jų netikrina. Statiniai analizatoriai tikrina visą pasiekiamą kodą, nesvarbu, kaip dažnai jis naudojamas. Tai labai geras pavyzdys, kaip statinė analizė papildo kitus testavimo ir apsaugos nuo klaidų metodus.
Panaši rodyklės tvarkymo klaida RHS leistina kode žemiau: V522 [CWE-476] Nulinės rodyklės „RHS“ nuoroda gali būti panaikinta. TGParser.cpp 2186
Fragmentas N6: žymeklio naudojimas po perkėlimo
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 įspėjimas: V522 [CWE-476] Nulinės rodyklės „ProgClone“ nuoroda gali būti panaikinta. Klaidinga kompiliacija.cpp 601
Pradžioje protinga rodyklė „ProgClone“. nustoja priklausyti objektui:
BD.setNewProgram(std::move(ProgClone));
Tiesą sakant, dabar „ProgClone“. yra nulinė rodyklė. Todėl nulinės rodyklės nuoroda turėtų įvykti žemiau:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Tačiau iš tikrųjų tai neįvyks! Atminkite, kad ciklas iš tikrųjų nėra vykdomas.
Talpyklos pradžioje Neteisingai sukompiliuotos funkcijos išvalyta:
MiscompiledFunctions.clear();
Toliau šio konteinerio dydis naudojamas kilpos sąlygomis:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Nesunku pastebėti, kad kilpa neprasideda. Manau, kad tai taip pat yra klaida ir kodas turėtų būti parašytas kitaip.
Atrodo, kad susidūrėme su tuo garsiuoju klaidų lygiu! Viena klaida maskuoja kitą :).
Fragmentas N7: žymeklio naudojimas po perkėlimo
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 įspėjimas: V522 [CWE-476] Nulinės rodyklės „Test“ nuoroda gali būti panaikinta. Klaidinga kompiliacija.cpp 709
Vėl ta pati situacija. Iš pradžių objekto turinys perkeliamas, o vėliau naudojamas taip, lyg nieko nebūtų nutikę. Šią situaciją vis dažniau matau programos kode po judesių semantikos atsiradimo C++. Štai kodėl man patinka C++ kalba! Atsiranda vis daugiau naujų būdų, kaip nusiauti koją. PVS-Studio analizatorius visada turės darbo :).
Fragmentas N8: Nulinė rodyklė
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 įspėjimas: V522 [CWE-476] Nulinės rodyklės „Tipas“ nuoroda gali būti panaikinta. PrettyFunctionDumper.cpp 233
Be klaidų tvarkyklių, derinimo spausdinimo funkcijos paprastai nėra testuojamos. Mums kaip tik toks atvejis prieš akis. Funkcija laukia vartotojo, kuris, užuot spręsdamas savo problemas, bus priverstas ją taisyti.
Teisingai:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragmentas N9: Nulinė rodyklė
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 įspėjimas: V522 [CWE-476] Nulinės rodyklės „Ty“ nuoroda gali būti panaikinta. SearchableTableEmitter.cpp 614
Manau, kad viskas aišku ir nereikalauja paaiškinimo.
Fragmentas N10: rašybos klaida
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 įspėjimas:
Nėra prasmės priskirti sau kintamąjį. Greičiausiai jie norėjo parašyti:
Identifier->Type = Question->Type;
Fragmentas N11: įtartinas lūžis
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 įspėjimas:
Pradžioje yra labai įtartinas operatorius pertrauka. Ar pamiršai čia dar ką nors parašyti?
Fragmentas N12: žymeklio tikrinimas po nuorodos panaikinimo
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 įspėjimas:
Rodyklė Callee pradžioje yra atšaukiamas funkcijos iškvietimo metu gautiTTI.
Ir tada paaiškėja, kad ši rodyklė turėtų būti patikrinta dėl lygybės nullptr:
if (!Callee || Callee->isDeclaration())
Bet jau per vėlu…
Fragmentas N13 - N...: Rodyklės tikrinimas po nuorodos panaikinimo
Ankstesniame kodo fragmente aptarta situacija nėra unikali. Jis pasirodo čia:
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“ įspėjimas: V595 [CWE-476] Rodyklė „CalleeFn“ buvo panaudota prieš patikrinant, ar ji atitinka nullptr. Patikrinimo eilutės: 1079, 1081. SimplifyLibCalls.cpp 1079
Ir čia:
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“ įspėjimas: V595 [CWE-476] „ND“ rodyklė buvo naudojama prieš patikrinant, ar ji atitinka nullptr. Patikrinimo eilutės: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Ir čia:
- V595 [CWE-476] „U“ rodyklė buvo panaudota prieš patikrinant, ar ji atitinka nullptr. Patikrinimo eilutės: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] „ND“ rodyklė buvo panaudota prieš patikrinant, ar ji atitinka nullptr. Patikrinimo eilutės: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Ir tada man nebeįdomu studijuoti įspėjimus su numeriu V595. Taigi nežinau, ar yra daugiau panašių klaidų, be čia išvardytų. Greičiausiai yra.
Fragmentas N17, N18: įtartinas poslinkis
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio įspėjimas:
Tai gali būti ne klaida, o kodas veikia tiksliai taip, kaip numatyta. Tačiau tai akivaizdžiai labai įtartina vieta ir ją reikia patikrinti.
Tarkime, kintamasis Dydis yra lygus 16, o tada kodo autorius planavo jį gauti kintamuoju NImms vertė:
1111111111111111111111111111111111111111111111111111111111100000
Tačiau iš tikrųjų rezultatas bus toks:
0000000000000000000000000000000011111111111111111111111111100000
Faktas yra tas, kad visi skaičiavimai atliekami naudojant 32 bitų nepasirašytą tipą. Ir tik tada šis 32 bitų nepasirašytas tipas bus netiesiogiai išplėstas iki uint64_t. Šiuo atveju reikšmingiausi bitai bus lygūs nuliui.
Pataisyti situaciją galite taip:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Panaši situacija: V629 [CWE-190] Apsvarstykite galimybę patikrinti išraišką „Immr << 6“. 32 bitų vertės bitų poslinkis su vėlesniu išplėtimu į 64 bitų tipą. AArch64AddressingModes.h 269
Fragmentas N19: trūksta raktinio žodžio kitas?
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 įspėjimas:
Čia nėra klaidos. Nuo tuometinio pirmojo bloko if baigiasi su tęsti, tada nesvarbu, yra raktinis žodis kitas arba ne. Bet kuriuo atveju kodas veiks taip pat. Vis tiek pasigedo kitas daro kodą neaiškesnį ir pavojingesnį. Jei ateityje tęsti dingsta, kodas pradės veikti visiškai kitaip. Mano nuomone, geriau pridėti kitas.
Fragmentas N20: keturios to paties tipo rašybos klaidos
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 įspėjimai:
- V655 [CWE-480] Stygos buvo sujungtos, bet nenaudojamos. Apsvarstykite galimybę patikrinti išraišką „Rezultatas + pavadinimas.str()“. Simbolis.cpp 32
- V655 [CWE-480] Stygos buvo sujungtos, bet nenaudojamos. Apsvarstykite galimybę patikrinti reiškinį „Rezultatas + „(ObjC klasė)“ + Name.str()“. Simbolis.cpp 35
- V655 [CWE-480] Stygos buvo sujungtos, bet nenaudojamos. Apsvarstykite galimybę patikrinti reiškinį „Rezultatas + „(ObjC klasė EH)“ + Name.str()“. Simbolis.cpp 38
- V655 [CWE-480] Stygos buvo sujungtos, bet nenaudojamos. Apsvarstykite galimybę patikrinti reiškinį „Result + „(ObjC Ivar)“ + Name.str()“. Simbolis.cpp 41
Atsitiktinai vietoj += yra naudojamas operatorius +. Rezultatas yra dizainas, kuris neturi prasmės.
Fragmentas N21: Neapibrėžta elgsena
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();
}
}
}
Pabandykite patys surasti pavojingą kodą. Ir tai yra paveikslėlis, skirtas atitraukti dėmesį, kad iš karto nežiūrėtumėte į atsakymą:
PVS-Studio įspėjimas:
Problemos eilutė:
FeaturesMap[Op] = FeaturesMap.size();
Jei elementas Op nerastas, tada žemėlapyje sukuriamas naujas elementas ir ten įrašomas elementų skaičius šiame žemėlapyje. Tik nežinoma, ar funkcija bus iškviesta Dydis prieš arba po naujo elemento pridėjimo.
Fragmentas N22-N24: kartojamos užduotys
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 įspėjimas:
Nemanau, kad čia yra tikra klaida. Tiesiog nereikalinga pakartotinė užduotis. Bet vis tiek klaida.
Taip pat:
- V519 [CWE-563] Kintamajam „B.NDesc“ priskiriamos reikšmės du kartus iš eilės. Galbūt tai klaida. Patikrinimo eilutės: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Kintamajam priskiriamos reikšmės du kartus iš eilės. Galbūt tai klaida. Patikrinimo eilutės: 59, 61. coff2yaml.cpp 61
Fragmentas N25-N27: daugiau pakeitimų
Dabar pažvelkime į šiek tiek kitokią perskyrimo versiją.
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 įspėjimas: V519 [CWE-563] Kintamajam „Išlygiavimas“ priskiriamos reikšmės du kartus iš eilės. Galbūt tai klaida. Patikrinimo eilutės: 1158, 1160. LoadStoreVectorizer.cpp 1160
Tai labai keistas kodas, kuriame, matyt, yra loginė klaida. Pradžioje kintamasis Išlyginimas priklausomai nuo sąlygos priskiriama reikšmė. Ir tada užduotis vėl atsiranda, bet dabar be jokio patikrinimo.
Panašias situacijas galite pamatyti čia:
- V519 [CWE-563] Kintamajam „Efektai“ priskiriamos reikšmės du kartus iš eilės. Galbūt tai klaida. Patikrinimo eilutės: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Kintamajam „ExpectNoDerefChunk“ priskiriamos reikšmės du kartus iš eilės. Galbūt tai klaida. Patikrinimo eilutės: 4970, 4973. SemaType.cpp 4973
Fragmentas N28: Visada tikros būklės
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 įspėjimas:
Tikrinti nėra prasmės. Kintamasis nextByte visada nelygu vertei 0x90, kuris išplaukia iš ankstesnio patikrinimo. Tai kažkokia loginė klaida.
Fragmentas N29 - N...: Visada teisingos / klaidingos sąlygos
Analizatorius pateikia daug įspėjimų, kad visa būklė (
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 įspėjimas:
Konstanta 0xE yra 14 dešimtainė reikšmė. Apžiūra RegNo == 0xe nėra prasmės, nes jei RegNo > 13, tada funkcija baigs vykdyti.
Buvo daug kitų įspėjimų su ID V547 ir V560, bet kaip ir su
Pateiksiu pavyzdį, kodėl šių trigerių tyrimas yra nuobodus. Analizatorius yra visiškai teisus, duodamas įspėjimą dėl šio kodo. Bet tai nėra klaida.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio įspėjimas: V547 [CWE-570] Išraiška „!HasError“ visada yra klaidinga. UnwrappedLineParser.cpp 1635
Fragmentas N30: įtartinas grįžimas
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 įspėjimas:
Tai yra klaida arba specifinė technika, skirta kažką paaiškinti programuotojams, skaitantiems kodą. Šis dizainas man nieko nepaaiškina ir atrodo labai įtartinai. Geriau taip nerašyk :).
Pavargęs? Tada laikas išsivirti arbatos ar kavos.
Nauja diagnostika nustatyti defektai
Manau, užtenka 30 senos diagnostikos aktyvavimų. Dabar pažiūrėkime, ką įdomaus galima rasti su nauja diagnostika, kuri pasirodė analizatoriuje po to
Fragmentas N31: nepasiekiamas kodas
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 įspėjimas:
Kaip matote, abi operatoriaus šakos if baigiasi skambučiu operatoriui grįžti. Atitinkamai, konteineris „CtorDtorsByPriority“. niekada nebus išvalytas.
Fragmentas N32: nepasiekiamas kodas
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 įspėjimas: V779 [CWE-561] Aptiktas nepasiekiamas kodas. Gali būti, kad yra klaida. LLParser.cpp 835
Įdomi situacija. Pirmiausia pažiūrėkime į šią vietą:
return ParseTypeIdEntry(SummaryID);
break;
Iš pirmo žvilgsnio atrodo, kad čia nėra jokios klaidos. Atrodo kaip operatorius pertrauka čia yra papildomas, kurį galite tiesiog ištrinti. Tačiau ne viskas taip paprasta.
Analizatorius pateikia įspėjimą eilutėse:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Ir iš tiesų, šis kodas nepasiekiamas. Visi atvejai pereiti baigiasi operatoriaus skambučiu grįžti. O dabar beprasmiška viena pertrauka neatrodo toks nekenksmingas! Galbūt viena iš šakų turėtų baigtis pertrauka, Neprijungtas grįžti?
Fragmentas N33: atsitiktinis didelių bitų nustatymas iš naujo
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 įspėjimas:
Atkreipkite dėmesį, kad funkcija getStubAlignment grąžinimo tipas nepasirašytos. Apskaičiuokime išraiškos reikšmę, darydami prielaidą, kad funkcija grąžina reikšmę 8:
~(getStubAlignment() - 1)
~ (8u-1)
0xFFFFFFFF8u
Dabar atkreipkite dėmesį, kad kintamasis Duomenų dydis turi 64 bitų nepasirašytą tipą. Pasirodo, kad atliekant DataSize & 0xFFFFFFF8u operaciją, visi trisdešimt du aukščiausios eilės bitai bus atstatyti į nulį. Greičiausiai programuotojas to nenorėjo. Įtariu, kad jis norėjo apskaičiuoti: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Norėdami ištaisyti klaidą, turėtumėte parašyti taip:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Arba taip:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragmentas N34: nepavyko perduoti aiškaus tipo
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 įspėjimas:
Aiškus tipo liejimas naudojamas siekiant išvengti perpildymo dauginant tipo kintamuosius int. Tačiau aiškus tipo liejimas neapsaugo nuo perpildymo. Pirmiausia kintamieji bus padauginti, o tik tada 32 bitų daugybos rezultatas bus išplėstas iki tipo
Fragmentas N35: nepavyko nukopijuoti ir įklijuoti
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;
}
....
}
Ši nauja įdomi diagnostika nustato situacijas, kai kodo dalis buvo nukopijuota ir kai kurie pavadinimai jame pradėti keisti, tačiau vienoje vietoje jo nepataisė.
Atkreipkite dėmesį, kad antrajame bloke jie pasikeitė Op0 apie Op1. Tačiau vienoje vietoje jie to nepataisė. Greičiausiai tai turėjo būti parašyta taip:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragmentas N36: kintamoji painiava
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio įspėjimas:
Labai pavojinga funkcijų argumentams suteikti tuos pačius pavadinimus kaip ir klasės nariams. Labai lengva susipainioti. Mums kaip tik toks atvejis prieš akis. Ši išraiška neturi prasmės:
Mode &= Mask;
Pasikeičia funkcijos argumentas. Tai viskas. Šis argumentas nebenaudojamas. Greičiausiai turėjote parašyti taip:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragmentas N37: kintamoji painiava
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;
}
Įspėjimas PVS-Studio: V1001 [CWE-563] Kintamasis „Dydis“ yra priskirtas, bet nenaudojamas iki funkcijos pabaigos. Object.cpp 424
Situacija panaši į ankstesnę. Turėtų būti parašyta:
this->Size += this->EntrySize;
Fragmentas N38-N47: jie pamiršo patikrinti indeksą
Anksčiau mes žiūrėjome į diagnostikos paleidimo pavyzdžius
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 įspėjimas: V1004 [CWE-476] „Ptr“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar ji neatitinka nullptr. Patikrinimo eilutės: 729, 738. TargetTransformInfoImpl.h 738
Kintamas Ptr gali būti lygus nullptr, kaip rodo čekis:
if (Ptr != nullptr)
Tačiau žemiau šis žymeklis yra atšauktas be išankstinio patikrinimo:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Panagrinėkime kitą panašų atvejį.
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 įspėjimas: V1004 [CWE-476] „FD“ rodyklė buvo naudojama nesaugiai, kai buvo patikrinta, ar ji neatitinka nullptr. Patikrinimo eilutės: 3228, 3231. CGDebugInfo.cpp 3231
Atkreipkite dėmesį į ženklą FD. Esu tikras, kad problema aiškiai matoma ir nereikia jokio specialaus paaiškinimo.
Ir toliau:
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“ įspėjimas: V1004 [CWE-476] „PtrTy“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar ji neatitinka nullptr. Patikrinimo eilutės: 960, 965. InterleavedLoadCombinePass.cpp 965
Kaip apsisaugoti nuo tokių klaidų? Būkite atidesni kodo peržiūrai ir naudokite PVS-Studio statinį analizatorių, kad reguliariai patikrintumėte savo kodą.
Cituoti kitus kodo fragmentus su tokio tipo klaidomis nėra prasmės. Straipsnyje paliksiu tik įspėjimų sąrašą:
- V1004 [CWE-476] „Expr“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar ji atitinka nullptr. Patikrinimo eilutės: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] „PI“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar nulptr. Patikrinimo eilutės: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] „StatepointCall“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar nulptr. Patikrinimo eilutės: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] „RV“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar ji neatitinka nullptr. Patikrinimo eilutės: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Rodyklė „CalleeFn“ buvo naudojama nesaugiai po to, kai buvo patikrinta prieš nullptr. Patikrinimo eilutės: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] „TC“ rodyklė buvo naudojama nesaugiai po to, kai buvo patikrinta, ar ji neatitinka nullptr. Patikrinimo eilutės: 1819, 1824. Driver.cpp 1824
Fragmentas N48-N60: nekritiškas, bet defektas (galimas atminties nutekėjimas)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio įspėjimas:
Norėdami pridėti elementą prie konteinerio galo, pvz std::vektorius > tu negali tiesiog rašyti xxx.push_back(naujas X), nes nėra netiesioginio konvertavimo iš X* в std::unikalus_ptr.
Dažnas sprendimas yra rašyti xxx.emplace_back(naujas X)nes jis kompiliuoja: metodas emplace_back konstruoja elementą tiesiai iš savo argumentų ir todėl gali naudoti aiškius konstruktorius.
Tai nesaugu. Jei vektorius pilnas, atmintis paskirstoma iš naujo. Atminties perskirstymo operacija gali nepavykti, todėl gali atsirasti išimtis std::bad_alloc. Tokiu atveju žymeklis bus prarastas ir sukurtas objektas niekada nebus ištrintas.
Saugus sprendimas – kurti unikalus_ptrkuriai priklausys žymeklis prieš vektoriui bandant perskirstyti atmintį:
xxx.push_back(std::unique_ptr<X>(new X))
Nuo C++14 galite naudoti „std::make_unique“:
xxx.push_back(std::make_unique<X>())
Šio tipo defektai nėra svarbūs LLVM. Jei atminties paskirstyti nepavyks, kompiliatorius tiesiog sustos. Tačiau programoms su ilgomis
Taigi, nors šis kodas nekelia praktinės grėsmės LLVM, man pasirodė naudinga pakalbėti apie šį klaidų modelį ir tai, kad PVS-Studio analizatorius išmoko jį identifikuoti.
Kiti šio tipo įspėjimai:
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Leidimai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. PassManager.h 546
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „AAs“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. AliasAnalysis.h 324
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Įrašai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „AllEdges“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. CFGMST.h 268
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „VMaps“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. SimpleLoopUnswitch.cpp 2012 m
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Įrašai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. FDRLogBuilder.h 30
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „PendingSubmodules“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. ModuleMap.cpp 810
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Objektai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. DebugMap.cpp 88
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „Strategies“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 685
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 686
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 688
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 689
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 690
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 691
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 692
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 693
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į konteinerį „Modifikatoriai“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. llvm-stress.cpp 694
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į „Operands“ konteinerį naudojant „emplace_back“ metodą. Išimties atveju atsiras atminties nutekėjimas. GlobalISelEmitter.cpp 1911 m
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į talpyklą „Stash“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Rodyklė be savininko įtraukiama į sudėtinį rodinį „Atitikmenys“ metodu „emplace_back“. Išimties atveju atsiras atminties nutekėjimas. GlobalISelEmitter.cpp 2702
išvada
Iš viso paskelbiau 60 įspėjimų ir tada sustojau. Ar yra kitų defektų, kuriuos PVS-Studio analizatorius aptinka LLVM? Taip aš turiu. Tačiau, kai rašiau straipsnio kodo fragmentus, buvo vėlus vakaras, tiksliau net naktis, ir nusprendžiau, kad laikas tai pavadinti diena.
Tikiuosi, kad jums tai buvo įdomu ir norėsite išbandyti PVS-Studio analizatorių.
Galite atsisiųsti analizatorių ir gauti minų paieškos raktą adresu
Svarbiausia, reguliariai naudokite statinę analizę. Vienkartiniai patikrinimai, kurį atliekame siekdami populiarinti statinės analizės metodiką ir PVS-Studio nėra įprastas scenarijus.
Sėkmės gerinant kodo kokybę ir patikimumą!
Jei norite pasidalinti šiuo straipsniu su angliškai kalbančia auditorija, naudokite vertimo nuorodą: Andrejus Karpovas.
Šaltinis: www.habr.com