Yli kaksi vuotta on kulunut LLVM-projektin viimeisestä kooditarkastuksesta PVS-Studio-analysaattorillamme. Varmistetaan, että PVS-Studio-analysaattori on edelleen johtava työkalu virheiden ja mahdollisten haavoittuvuuksien tunnistamiseen. Tätä varten tarkistamme ja löydämme uusia virheitä LLVM 8.0.0 -julkaisusta.
Artikkeli kirjoitettavana
Rehellisesti sanottuna en halunnut kirjoittaa tätä artikkelia. Ei ole mielenkiintoista kirjoittaa projektista, jonka olemme jo tarkistaneet useita kertoja (
Joka kerta kun uusi LLVM-versio julkaistaan tai päivitetään
Katso, Clang Static Analyzerin uusi versio on oppinut löytämään uusia virheitä! Minusta näyttää siltä, että PVS-Studion käytön merkitys on vähenemässä. Clang löytää enemmän virheitä kuin ennen ja ottaa kiinni PVS-Studion ominaisuuksista. Mitä ajattelet tästä?
Tähän haluan aina vastata jotain tällaista:
Emme myöskään istu toimettomana! Olemme parantaneet merkittävästi PVS-Studio-analysaattorin ominaisuuksia. Joten älä huoli, jatkamme johtamista kuten ennenkin.
Valitettavasti tämä on huono vastaus. Siinä ei ole todisteita. Ja siksi kirjoitan tämän artikkelin nyt. Joten LLVM-projekti on jälleen kerran tarkastettu ja siinä on löydetty useita virheitä. Esitän nyt ne, jotka tuntuivat kiinnostavilta. Clang Static Analyzer ei löydä näitä virheitä (tai se on erittäin hankalaa tehdä sen avulla). Mutta voimme. Lisäksi löysin ja kirjoitin kaikki nämä virheet yhden illan aikana.
Mutta artikkelin kirjoittaminen kesti useita viikkoja. En vain jaksanut laittaa tätä kaikkea tekstiksi :).
Muuten, jos olet kiinnostunut siitä, mitä tekniikoita PVS-Studio-analysaattorissa käytetään virheiden ja mahdollisten haavoittuvuuksien tunnistamiseen, suosittelen tutustumista tähän
Uusi ja vanha diagnostiikka
Kuten jo todettiin, noin kaksi vuotta sitten LLVM-projekti tarkastettiin uudelleen ja löydetyt virheet korjattiin. Nyt tämä artikkeli esittelee uuden erän virheitä. Miksi uusia bugeja löydettiin? Tähän on 3 syytä:
- LLVM-projekti kehittyy, muuttaa vanhaa koodia ja lisää uutta koodia. Luonnollisesti muokattuun ja kirjoitettuun koodiin tulee uusia virheitä. Tämä osoittaa selvästi, että staattista analyysiä tulisi käyttää säännöllisesti, ei satunnaisesti. Artikkelimme osoittavat hyvin PVS-Studio-analysaattorin ominaisuudet, mutta tällä ei ole mitään tekemistä koodin laadun parantamisen ja virheiden korjauskustannusten vähentämisen kanssa. Käytä staattista koodianalysaattoria säännöllisesti!
- Viimeistelemme ja parannamme olemassa olevaa diagnostiikkaa. Siksi analysaattori voi tunnistaa virheet, joita se ei huomannut aikaisempien skannausten aikana.
- PVS-Studioon on ilmestynyt uutta diagnostiikkaa, jota ei ollut olemassa 2 vuotta sitten. Päätin korostaa niitä erillisessä osiossa näyttääkseni selkeästi PVS-Studion kehityksen.
Diagnostiikan avulla havaitut viat, jotka olivat olemassa 2 vuotta sitten
Fragmentti N1: Kopioi-Liitä
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-Studion varoitus:
On tarkistettu, että nimi alkaa alimerkkijonolla "avx512.mask.permvar.". Toisessa tarkistuksessa he ilmeisesti halusivat kirjoittaa jotain muuta, mutta unohtivat korjata kopioitua tekstiä.
Fragmentti N2: Kirjoitusvirhe
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;
....
}
Varoitus PVS-Studio: V501 '|':n vasemmalla ja oikealla puolella on identtiset alilausekkeet 'CXNameRange_WantQalifier'. operaattori. CIndex.cpp 7245
Kirjoitusvirheen vuoksi samaa nimettyä vakiota käytetään kahdesti CXNameRange_WantQalifier.
Fragmentti N3: Sekaannus operaattorin etusijalle
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studion varoitus:
Minusta tämä on erittäin kaunis virhe. Kyllä, tiedän, että minulla on outoja ajatuksia kauneudesta :).
Nyt sen mukaan
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Käytännön näkökulmasta tällaisella ehdolla ei ole järkeä, koska se voidaan pelkistää:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Tämä on selvä virhe. Todennäköisesti he halusivat verrata 0/1 muuttujaan indeksi. Korjataksesi koodin sinun on lisättävä sulut kolmiosaisen operaattorin ympärille:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Muuten, kolmiosainen operaattori on erittäin vaarallinen ja aiheuttaa loogisia virheitä. Ole erittäin varovainen sen kanssa äläkä ole ahne sulkeiden kanssa. Katsoin tätä aihetta tarkemmin
Fragmentti N4, N5: Nollaosoitin
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-Studion varoitus:
Jos osoitin LHS on tyhjä, tulee antaa varoitus. Sen sijaan tähän samaan nollaosoittimeen viitataan: LHS->getAsString().
Tämä on hyvin tyypillinen tilanne, kun virhe on piilotettu virhekäsittelijään, koska kukaan ei testaa niitä. Staattiset analysaattorit tarkistavat kaiken tavoitettavissa olevan koodin riippumatta siitä, kuinka usein sitä käytetään. Tämä on erittäin hyvä esimerkki siitä, kuinka staattinen analyysi täydentää muita testaus- ja virhesuojaustekniikoita.
Samanlainen osoittimen käsittelyvirhe RHS Sallittu alla olevassa koodissa: V522 [CWE-476] Nollaosoittimen 'RHS' viittauksen purkaminen saattaa tapahtua. TGParser.cpp 2186
Fragmentti N6: Osoittimen käyttö siirron jälkeen
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 Varoitus: V522 [CWE-476] Nollaosoittimen 'ProgClone' viittauksen purkaminen saattaa tapahtua. Miscompilation.cpp 601
Alussa älykäs osoitin ProgClone lakkaa omistamasta kohdetta:
BD.setNewProgram(std::move(ProgClone));
Itse asiassa nyt ProgClone on nollaosoitin. Siksi nollaosoittimen viittauksen pitäisi tapahtua juuri alla:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Mutta todellisuudessa näin ei tapahdu! Huomaa, että silmukkaa ei todellisuudessa suoriteta.
Säiliön alussa Väärin käännetty funktio tyhjennetty:
MiscompiledFunctions.clear();
Seuraavaksi tämän säilön kokoa käytetään silmukkatilassa:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
On helppo nähdä, että silmukka ei käynnisty. Mielestäni tämä on myös virhe ja koodi pitäisi kirjoittaa eri tavalla.
Näyttää siltä, että olemme kohdanneet tuon kuuluisan virhepariteetin! Yksi virhe peittää toisen :).
Fragmentti N7: Osoittimen käyttö siirron jälkeen
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-varoitus: V522 [CWE-476] Nollaosoittimen 'Test'-viittauksen purkaminen saattaa tapahtua. Miscompilation.cpp 709
Taas sama tilanne. Aluksi kohteen sisältöä siirretään, ja sitten sitä käytetään ikään kuin mitään ei olisi tapahtunut. Näen tämän tilanteen yhä useammin ohjelmakoodissa sen jälkeen kun liikesemantiikka ilmestyi C++:ssa. Tästä syystä rakastan C++-kieltä! On olemassa yhä enemmän uusia tapoja ampua oma jalkasi irti. PVS-Studio-analysaattorilla on aina työtä :).
Fragmentti N8: Nollaosoitin
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-varoitus: V522 [CWE-476] Nollaosoittimen 'Type'-viittauksen purkaminen saattaa tapahtua. PrettyFunctionDumper.cpp 233
Virhekäsittelijöiden lisäksi virheenkorjaustulostustoimintoja ei yleensä testata. Meillä on juuri tällainen tapaus edessämme. Toiminto odottaa käyttäjää, joka ongelmiensa ratkaisemisen sijaan pakotetaan korjaamaan se.
korjaa:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragmentti N9: Nollaosoitin
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-varoitus: V522 [CWE-476] Nollaosoittimen 'Ty' viittauksen purkaminen saattaa tapahtua. SearchableTableEmitter.cpp 614
Mielestäni kaikki on selvää eikä vaadi selityksiä.
Fragmentti N10: Kirjoitusvirhe
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-Studion varoitus:
Ei ole mitään järkeä määrittää muuttujaa itselleen. Todennäköisesti he halusivat kirjoittaa:
Identifier->Type = Question->Type;
Fragmentti N11: Epäilyttävä rikkoutuminen
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-Studion varoitus:
Alussa on erittäin epäilyttävä operaattori rikkoa. Unohditko kirjoittaa tänne jotain muuta?
Fragmentti N12: Tarkistetaan osoitinta viittauksen poistamisen jälkeen
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-Studion varoitus:
Indeksi Callee alussa on viittaus pois funktiota kutsuttaessa hanki TTI.
Ja sitten käy ilmi, että tämän osoittimen tasa-arvo on tarkistettava nullptr:
if (!Callee || Callee->isDeclaration())
Mutta on liian myöhäistä…
Fragmentti N13 - N...: Tarkistetaan osoitinta viittauksen poistamisen jälkeen
Edellisessä koodikatkelmassa käsitelty tilanne ei ole ainutlaatuinen. Se näkyy täällä:
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-varoitus: V595 [CWE-476] CalleeFn-osoitinta käytettiin ennen kuin se varmistettiin nullptr:n suhteen. Tarkistusrivit: 1079, 1081. SimplifyLibCalls.cpp 1079
Ja täällä:
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-varoitus: V595 [CWE-476] 'ND'-osoitinta käytettiin ennen kuin se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Ja täällä:
- V595 [CWE-476] 'U'-osoitinta käytettiin ennen kuin se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] 'ND'-osoitinta käytettiin ennen kuin se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Ja sitten minua ei kiinnostanut V595-varoitusten tutkiminen. Joten en tiedä onko muita vastaavia virheitä tässä lueteltujen lisäksi. Todennäköisimmin on.
Fragmentti N17, N18: Epäilyttävä siirtymä
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studion varoitus:
Se ei välttämättä ole bugi ja koodi toimii täsmälleen tarkoitetulla tavalla. Mutta tämä on selvästi erittäin epäilyttävä paikka, ja se on tarkistettava.
Sanotaanko muuttuja Koko on yhtä suuri kuin 16, ja sitten koodin kirjoittaja suunnitteli saavansa sen muuttujaksi NImms arvo:
1111111111111111111111111111111111111111111111111111111111100000
Todellisuudessa tulos on kuitenkin:
0000000000000000000000000000000011111111111111111111111111100000
Tosiasia on, että kaikki laskelmat suoritetaan käyttämällä 32-bittistä allekirjoittamatonta tyyppiä. Ja vasta sitten tämä 32-bittinen allekirjoittamaton tyyppi laajennetaan implisiittisesti uint64_t. Tässä tapauksessa merkittävimmät bitit ovat nolla.
Voit korjata tilanteen näin:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Samanlainen tilanne: V629 [CWE-190] Harkitse 'Immr << 6' -lausekkeen tarkistamista. 32-bittisen arvon bittisiirto, jonka jälkeen laajennus 64-bittiseen tyyppiin. AArch64AddressingModes.h 269
Fragmentti N19: Avainsana puuttuu muu?
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-Studion varoitus:
Tässä ei ole virhettä. Ensimmäisen silloisesta lohkosta lähtien if loppuu jatkaa, ei sillä ole väliä, sillä on avainsana muu tai ei. Joka tapauksessa koodi toimii samalla tavalla. Edelleen ikävä muu tekee koodista epäselvämmän ja vaarallisemman. Jos tulevaisuudessa jatkaa katoaa, koodi alkaa toimia täysin eri tavalla. Mielestäni on parempi lisätä muu.
Fragmentti N20: Neljä samantyyppistä kirjoitusvirhettä
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-Studion varoitukset:
- V655 [CWE-480] Merkkijonot ketjutettiin, mutta niitä ei käytetä. Harkitse "Result + Name.str()" -lausekkeen tarkistamista. Symboli.cpp 32
- V655 [CWE-480] Merkkijonot ketjutettiin, mutta niitä ei käytetä. Harkitse lausekkeen 'Result + "(ObjC Class)" + Name.str()' tarkistamista. Symboli.cpp 35
- V655 [CWE-480] Merkkijonot ketjutettiin, mutta niitä ei käytetä. Harkitse 'Result + "(ObjC Class EH)" + Name.str()' -lausekkeen tarkistamista. Symboli.cpp 38
- V655 [CWE-480] Merkkijonot ketjutettiin, mutta niitä ei käytetä. Harkitse lausekkeen 'Result + "(ObjC Ivar)" + Name.str() tarkistamista. Symboli.cpp 41
Vahingossa +-operaattoria käytetään +=-operaattorin sijaan. Tuloksena on suunnitelmia, joilla ei ole merkitystä.
Fragmentti N21: Määrittelemätön toiminta
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();
}
}
}
Yritä löytää vaarallinen koodi itse. Ja tämä on kuva, joka häiritsee huomion, jotta ei heti katsoisi vastausta:
PVS-Studion varoitus:
Ongelma rivi:
FeaturesMap[Op] = FeaturesMap.size();
Jos elementti Op ei löydy, niin karttaan luodaan uusi elementti ja siihen kirjoitetaan tämän kartan elementtien määrä. Ei vain tiedetä, kutsutaanko funktiota koko ennen tai jälkeen uuden elementin lisäämisen.
Fragmentti N22-N24: Toistetut tehtävät
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-Studion varoitus:
En usko, että tässä on todellista virhettä. Vain tarpeeton toistuva tehtävä. Mutta silti virhe.
samankaltaisella tavalla:
- V519 [CWE-563] 'B.NDesc'-muuttujalle annetaan arvot kahdesti peräkkäin. Ehkä tämä on virhe. Tarkistusrivit: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Muuttujalle annetaan arvot kahdesti peräkkäin. Ehkä tämä on virhe. Tarkistusrivit: 59, 61. coff2yaml.cpp 61
Fragmentti N25-N27: Lisää uudelleenmäärityksiä
Katsotaanpa nyt hieman erilaista versiota uudelleenmäärityksestä.
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-varoitus: V519 [CWE-563] 'Alignment'-muuttujalle annetaan arvot kahdesti peräkkäin. Ehkä tämä on virhe. Tarkistusrivit: 1158, 1160. LoadStoreVectorizer.cpp 1160
Tämä on hyvin outo koodi, joka ilmeisesti sisältää loogisen virheen. Alussa muuttuva suuntaus arvo määritetään tilanteen mukaan. Ja sitten tehtävä toistuu, mutta nyt ilman tarkistusta.
Samanlaisia tilanteita voi nähdä täältä:
- V519 [CWE-563] 'Effects'-muuttujalle annetaan arvot kahdesti peräkkäin. Ehkä tämä on virhe. Tarkistusrivit: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] 'ExpectNoDerefChunk'-muuttujalle annetaan arvot kahdesti peräkkäin. Ehkä tämä on virhe. Tarkistusrivit: 4970, 4973. SemaType.cpp 4973
Fragmentti N28: Aina oikea kunto
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-Studion varoitus:
Tarkistamisessa ei ole järkeä. Muuttuva nextByte ei aina ole sama kuin arvo 0x90, mikä seuraa edellisestä tarkistuksesta. Tämä on jonkinlainen looginen virhe.
Fragmentti N29 - N...: Aina tosi/epätosi ehdot
Analysaattori antaa monia varoituksia siitä, että koko tila (
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-Studion varoitus:
Vakio 0xE on arvo 14 desimaaleina. Tutkimus RegNo == 0xe ei ole järkeä, koska jos RegNo > 13, toiminto suorittaa suorituksensa loppuun.
Tunnuksilla V547 ja V560 oli monia muita varoituksia, mutta kuten myös
Annan sinulle esimerkin siitä, miksi näiden laukaisimien tutkiminen on tylsää. Analysaattori on täysin oikeassa antamassaan varoituksen seuraavasta koodista. Mutta tämä ei ole virhe.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio Varoitus: V547 [CWE-570] Lauseke '!HasError' on aina epätosi. UnwrappedLineParser.cpp 1635
Fragmentti N30: Epäilyttävä paluu
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-Studion varoitus:
Tämä on joko virhe tai erityinen tekniikka, jonka tarkoituksena on selittää jotain koodia lukeville ohjelmoijille. Tämä malli ei selitä minulle mitään ja näyttää erittäin epäilyttävältä. Parempi olla kirjoittamatta noin :).
Väsynyt? Sitten on aika keittää teetä tai kahvia.
Uuden diagnoosin perusteella havaitut viat
Mielestäni 30 aktivointia vanhasta diagnostiikasta riittää. Katsotaan nyt mitä mielenkiintoista löytyy analysaattoriin sen jälkeen ilmestyneestä uudesta diagnostiikasta
Fragmentti N31: Koodi, jota ei tavoiteta
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-Studion varoitus:
Kuten näette, operaattorin molemmat haarat if päättyy puheluun operaattorille palata. Vastaavasti säiliö CctorDtorsByPriority ei koskaan tyhjennetä.
Fragmentti N32: Koodi, jota ei tavoiteta
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-varoitus: V779 [CWE-561] Ei tavoitettavissa oleva koodi havaittu. On mahdollista, että kyseessä on virhe. LLParser.cpp 835
Mielenkiintoinen tilanne. Katsotaanpa ensin tätä paikkaa:
return ParseTypeIdEntry(SummaryID);
break;
Ensi silmäyksellä näyttää siltä, että tässä ei ole virhettä. Se näyttää operaattorilta rikkoa täällä on ylimääräinen, ja voit yksinkertaisesti poistaa sen. Kaikki eivät kuitenkaan ole niin yksinkertaisia.
Analysaattori antaa varoituksen riveillä:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Ja todellakin, tämä koodi on saavuttamaton. Kaikki tapaukset mukana kytkin päättyy operaattorin puheluun palata. Ja nyt järjetön yksin rikkoa ei näytä niin vaarattomalta! Ehkä yhden oksien pitäisi päättyä rikkoamutta ei palata?
Fragmentti N33: Korkeiden bittien satunnainen nollaus
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-Studion varoitus:
Huomaa, että toiminto getStubAlignment palauttaa tyypin allekirjoittamaton. Lasketaan lausekkeen arvo olettaen, että funktio palauttaa arvon 8:
~(getStubAlignment() - 1)
~ (8u-1)
0xFFFFFFFF8u
Huomaa nyt, että muuttuja DataSize on 64-bittinen allekirjoittamaton tyyppi. Osoittautuu, että suoritettaessa DataSize & 0xFFFFFFF8u -toimintoa kaikki kolmekymmentäkaksi korkean kertaluokan bittiä nollataan. Todennäköisesti tämä ei ole sitä, mitä ohjelmoija halusi. Epäilen, että hän halusi laskea: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Voit korjata virheen kirjoittamalla tämän:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Tai niin:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragmentti N34: Epäonnistunut eksplisiittisen tyypin 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-Studion varoitus:
Eksplisiittistä tyyppivalua käytetään ylivuodon välttämiseksi tyyppimuuttujia kerrottaessa int. Selkeä tyyppivalu ei kuitenkaan suojaa ylivuodolta. Ensin muuttujat kerrotaan ja vasta sitten kertolasku 32-bittinen tulos laajennetaan tyyppiin
Fragmentti N35: Kopioi ja liitä epäonnistui
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;
}
....
}
Tämä uusi mielenkiintoinen diagnoosi tunnistaa tilanteet, joissa koodinpätkä on kopioitu ja joitain nimiä siinä on alettu muuttaa, mutta yhdestä paikasta ei ole korjattu.
Huomaa, että toisessa lohkossa ne muuttuivat Op0 päälle Op1. Mutta yhdessä paikassa he eivät korjanneet sitä. Todennäköisesti se olisi pitänyt kirjoittaa näin:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragmentti N36: Muuttuva hämmennys
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studion varoitus:
On erittäin vaarallista antaa funktion argumenteille samat nimet kuin luokan jäsenille. Se on erittäin helppoa hämmentyä. Meillä on juuri tällainen tapaus edessämme. Tässä ilmaisussa ei ole järkeä:
Mode &= Mask;
Funktion argumentti muuttuu. Siinä kaikki. Tätä argumenttia ei enää käytetä. Todennäköisesti sinun olisi pitänyt kirjoittaa se näin:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragmentti N37: Muuttuva hämmennys
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;
}
Varoitus PVS-Studio: V1001 [CWE-563] Koko-muuttuja on määritetty, mutta sitä ei käytetä funktion loppuun mennessä. Object.cpp 424
Tilanne on samanlainen kuin edellisessä. Se pitäisi kirjoittaa:
this->Size += this->EntrySize;
Fragmentti N38-N47: He unohtivat tarkistaa indeksin
Aiemmin tarkastelimme esimerkkejä diagnostisista laukaisuista
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-varoitus: V1004 [CWE-476] 'Ptr'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:n suhteen. Tarkistusrivit: 729, 738. TargetTransformInfoImpl.h 738
muuttuja PTR voi olla tasa-arvoinen nullptr, kuten sekki osoittaa:
if (Ptr != nullptr)
Tämän osoittimen alla on kuitenkin poistettu viittaus ilman ennakkotarkastuksia:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Tarkastellaanpa toista vastaavaa tapausta.
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-varoitus: V1004 [CWE-476] 'FD'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:n suhteen. Tarkistusrivit: 3228, 3231. CGDebugInfo.cpp 3231
Kiinnitä huomiota merkkiin FD. Olen varma, että ongelma on selvästi nähtävissä, eikä erityisiä selityksiä tarvita.
Ja kauemmas:
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-varoitus: V1004 [CWE-476] 'PtrTy'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:n suhteen. Tarkistusrivit: 960, 965. InterleavedLoadCombinePass.cpp 965
Kuinka suojautua tällaisilta virheiltä? Ole tarkkaavaisempi Code-Reviewissa ja käytä PVS-Studion staattista analysaattoria tarkistaaksesi koodisi säännöllisesti.
Ei ole mitään järkeä lainata muita tämäntyyppisiä virheitä sisältäviä koodinpätkiä. Jätän artikkeliin vain luettelon varoituksista:
- V1004 [CWE-476] 'Laus'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] PI-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se oli varmistettu nullptr:tä vastaan. Tarkistusrivit: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] 'StatepointCall'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] RV-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:n suhteen. Tarkistusrivit: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] 'CalleeFn'-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:tä vastaan. Tarkistusrivit: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] TC-osoitinta käytettiin epäturvallisesti sen jälkeen, kun se varmistettiin nullptr:tä vastaan. Tarkistuslinjat: 1819, 1824. Driver.cpp 1824
Fragmentti N48-N60: Ei kriittinen, mutta vika (mahdollinen muistivuoto)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studion varoitus:
Voit lisätä elementin säilön loppuun kuten std::vektori > et voi vain kirjoittaa xxx.push_back(uusi X), koska ei ole implisiittistä muuntamista X* в std::unique_ptr.
Yleinen ratkaisu on kirjoittaa xxx.emplace_back(uusi X)koska se kääntää: metodi emplace_back rakentaa elementin suoraan argumenteistaan ja voi siksi käyttää eksplisiittisiä konstruktoreita.
Se ei ole turvallista. Jos vektori on täynnä, muisti varataan uudelleen. Muistin uudelleenallokointitoiminto saattaa epäonnistua, mikä johtaa poikkeuksen tekemiseen std::bad_alloc. Tässä tapauksessa osoitin katoaa eikä luotua objektia koskaan poisteta.
Turvallinen ratkaisu on luoda ainutlaatuinen_ptrjoka omistaa osoittimen ennen kuin vektori yrittää jakaa muistia uudelleen:
xxx.push_back(std::unique_ptr<X>(new X))
C++14:stä lähtien voit käyttää 'std::make_unique':
xxx.push_back(std::make_unique<X>())
Tämäntyyppinen vika ei ole kriittinen LLVM:lle. Jos muistia ei voida varata, kääntäjä yksinkertaisesti pysähtyy. Kuitenkin sovelluksiin, joissa on pitkä
Joten vaikka tämä koodi ei aiheuta käytännön uhkaa LLVM:lle, minusta oli hyödyllistä puhua tästä virhekuviosta ja siitä, että PVS-Studio-analysaattori on oppinut tunnistamaan sen.
Muut tämän tyyppiset varoitukset:
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Passes' -säilöön 'emplace_back'-metodilla. Poikkeustapauksessa tapahtuu muistivuoto. PassManager.h 546
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään "AAs"-säilöyn "emplace_back"-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. AliasAnalysis.h 324
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Entries'-säilöön 'emplace_back'-metodilla. Poikkeustapauksessa tapahtuu muistivuoto. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'AllEdges' -säilöön 'emplace_back'-metodilla. Poikkeustapauksessa tapahtuu muistivuoto. CFGMST.h 268
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään VMaps-säilöyn emplace_back-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään "Records"-säilöön "emplace_back"-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. FDRLogBuilder.h 30
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään "PendingSubmodules" -säilöön "emplace_back"-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. ModuleMap.cpp 810
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Objects'-säilöön 'emplace_back'-metodilla. Poikkeustapauksessa tapahtuu muistivuoto. DebugMap.cpp 88
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään Strategiat-säilöön menetelmällä 'emplace_back'. Poikkeustapauksessa tapahtuu muistivuoto. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 685
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 686
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 688
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 689
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 690
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 691
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 692
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 693
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään 'Modifiers'-säilöön 'emplace_back'-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. llvm-stress.cpp 694
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään Operandit-säilöön menetelmällä 'emplace_back'. Poikkeustapauksessa tapahtuu muistivuoto. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään "Stash"-säilöön "emplace_back"-menetelmällä. Poikkeustapauksessa tapahtuu muistivuoto. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Osoitin ilman omistajaa lisätään Matchers-säilöön menetelmällä 'emplace_back'. Poikkeustapauksessa tapahtuu muistivuoto. GlobalISelEmitter.cpp 2702
Johtopäätös
Annoin yhteensä 60 varoitusta ja lopetin sitten. Onko muita vikoja, joita PVS-Studio-analysaattori havaitsee LLVM:ssä? Kyllä minulla on. Kuitenkin, kun kirjoitin koodinpätkiä artikkeliin, oli myöhäinen ilta tai pikemminkin yö, ja päätin, että on aika kutsua sitä päiväksi.
Toivottavasti pidit siitä mielenkiintoisena ja haluat kokeilla PVS-Studio-analysaattoria.
Voit ladata analysaattorin ja saada miinanraivaajan avaimen osoitteessa
Mikä tärkeintä, käytä staattista analyysiä säännöllisesti. Kertatarkastukset, jonka olemme toteuttaneet staattisen analyysin metodologian popularisoimiseksi ja PVS-Studio eivät ole normaali skenaario.
Onnea koodisi laadun ja luotettavuuden parantamiseen!
Jos haluat jakaa tämän artikkelin englanninkielisen yleisön kanssa, käytä käännöslinkkiä: Andrey Karpov.
Lähde: will.com