Han passat més de dos anys des de la darrera comprovació de codi del projecte LLVM mitjançant el nostre analitzador PVS-Studio. Assegurem-nos que l'analitzador PVS-Studio segueix sent una eina líder per identificar errors i vulnerabilitats potencials. Per fer-ho, comprovarem i trobarem nous errors a la versió LLVM 8.0.0.
Article a escriure
Per ser sincer, no volia escriure aquest article. No és interessant escriure sobre un projecte que ja hem revisat diverses vegades (, , ). És millor escriure sobre alguna cosa nova, però no tinc més remei.
Cada vegada que es publica o s'actualitza una nova versió de LLVM , rebem preguntes del tipus següent al nostre correu:
Mira, la nova versió de Clang Static Analyzer ha après a trobar nous errors! Em sembla que la rellevància d'utilitzar PVS-Studio està disminuint. Clang troba més errors que abans i es posa al dia amb les capacitats de PVS-Studio. Què en penses d'això?
A això sempre vull respondre alguna cosa com:
Tampoc ens quedem ociosos! Hem millorat significativament les capacitats de l'analitzador PVS-Studio. Així que no us preocupeu, seguim liderant com abans.
Malauradament, aquesta és una mala resposta. No hi ha proves en ella. I per això ara escric aquest article. Així doncs, el projecte LLVM s'ha tornat a comprovar i s'han trobat diversos errors. Ara faré una demostració dels que em van semblar interessants. Clang Static Analyzer no pot trobar aquests errors (o és extremadament inconvenient fer-ho amb la seva ajuda). Però podem. A més, vaig trobar i escriure tots aquests errors en una nit.
Però escriure l'article va trigar diverses setmanes. Simplement no vaig poder posar tot això en un text :).
Per cert, si esteu interessats en quines tecnologies s'utilitzen a l'analitzador PVS-Studio per identificar errors i vulnerabilitats potencials, us suggereixo que us familiaritzeu amb això. .
Diagnòstics nous i antics
Com ja s'ha dit, fa uns dos anys es va tornar a comprovar el projecte LLVM i es van corregir els errors trobats. Ara aquest article presentarà un nou lot d'errors. Per què es van trobar errors nous? Hi ha 3 raons per a això:
- El projecte LLVM està evolucionant, canviant el codi antic i afegint codi nou. Naturalment, hi ha nous errors en el codi modificat i escrit. Això demostra clarament que l'anàlisi estàtica s'ha d'utilitzar amb regularitat, i no ocasionalment. Els nostres articles mostren bé les capacitats de l'analitzador PVS-Studio, però això no té res a veure amb la millora de la qualitat del codi i la reducció del cost de la correcció d'errors. Utilitzeu un analitzador de codi estàtic amb regularitat!
- Estem ultimant i millorant els diagnòstics existents. Per tant, l'analitzador pot identificar errors que no va notar durant les exploracions anteriors.
- A PVS-Studio han aparegut nous diagnòstics que no existien fa 2 anys. Vaig decidir destacar-los en una secció a part per mostrar clarament el desenvolupament de PVS-Studio.
Defectes identificats per diagnòstics que existien fa 2 anys
Fragment N1: Copiar-Enganxar
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
....
}Advertència de PVS-Studio: [CWE-570] Hi ha subexpressions idèntiques 'Name.startswith("avx512.mask.permvar.")' a l'esquerra i a la dreta del '||' operador. AutoUpgrade.cpp 73
Es comprova que el nom comenci amb la subcadena "avx512.mask.permvar.". En la segona comprovació, òbviament, volien escriure una altra cosa, però es van oblidar de corregir el text copiat.
Fragment N2: error ortogràfic
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;
....
}Advertència PVS-Studio: V501 Hi ha subexpressions idèntiques 'CXNameRange_WantQualifier' a l'esquerra i a la dreta del '|' operador. CIndex.cpp 7245
A causa d'una errada d'ortografia, la mateixa constant anomenada s'utilitza dues vegades CXNameRange_WantQualifier.
Fragment N3: confusió amb la precedència de l'operador
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Advertència de PVS-Studio: [CWE-783] Potser l'operador '?:' funciona d'una manera diferent de la que s'esperava. L'operador '?:' té una prioritat més baixa que l'operador '=='. PPCTargetTransformInfo.cpp 404
Al meu entendre, aquest és un error molt bonic. Sí, sé que tinc idees estranyes sobre la bellesa :).
Ara, segons , l'expressió s'avalua de la següent manera:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Des d'un punt de vista pràctic, aquesta condició no té sentit, ja que es pot reduir a:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())Això és un error clar. El més probable és que volien comparar 0/1 amb una variable Index. Per corregir el codi, cal afegir parèntesis al voltant de l'operador ternari:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))Per cert, l'operador ternari és molt perillós i provoca errors lògics. Tingueu molta cura amb ell i no siguis cobdiciós amb els parèntesis. Vaig mirar aquest tema amb més detall , al capítol "Aneu amb compte amb l'operador i tanqueu-lo entre parèntesis".
Fragment N4, N5: punter nul
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;
}
....
}Advertència de PVS-Studio: [CWE-476] Es pot produir una desreferenciació del punter nul 'LHS'. TGParser.cpp 2152
Si el punter LHS és nul·la, s'hauria d'emetre un avís. Tanmateix, en canvi, aquest mateix punter nul es desreferenciarà: LHS->getAsString().
Aquesta és una situació molt típica quan un error s'amaga en un gestor d'errors, ja que ningú els prova. Els analitzadors estàtics comproven tot el codi accessible, sense importar la freqüència amb què s'utilitzi. Aquest és un molt bon exemple de com l'anàlisi estàtica complementa altres tècniques de prova i protecció d'errors.
Error de manipulació del punter similar RHS permès al codi a continuació: V522 [CWE-476] Es pot produir una desreferenciació del punter nul 'RHS'. TGParser.cpp 2186
Fragment N6: Ús del punter després de moure's
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);
}
....
}Advertència de PVS-Studio: V522 [CWE-476] Es pot produir una desreferenciació del punter nul 'ProgClone'. Miscompilation.cpp 601
Al principi un punter intel·ligent ProgClone deixa de ser propietari de l'objecte:
BD.setNewProgram(std::move(ProgClone));De fet, ara ProgClone és un punter nul. Per tant, una desreferència de punter nul hauria de produir-se just a sota:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Però, en realitat, això no passarà! Tingueu en compte que el bucle no s'executa realment.
A l'inici del contenidor Funcions compilades incorrectament esborrat:
MiscompiledFunctions.clear();A continuació, la mida d'aquest contenidor s'utilitza en la condició de bucle:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {És fàcil veure que el bucle no comença. Crec que això també és un error i el codi s'hauria d'escriure de manera diferent.
Sembla que ens hem trobat amb aquesta famosa paritat d'errors! Un error emmascara un altre :).
Fragment N7: Ús del punter després de moure's
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;
}
....
}Advertència de PVS-Studio: V522 [CWE-476] Es pot produir una desreferenciació del punter nul 'Prova'. Miscompilation.cpp 709
De nou la mateixa situació. Al principi, es mou el contingut de l'objecte, i després s'utilitza com si no hagués passat res. Veig aquesta situació cada cop més sovint al codi del programa després que la semàntica del moviment aparegués en C++. Per això m'encanta el llenguatge C++! Cada cop hi ha més maneres noves de disparar la teva pròpia cama. L'analitzador PVS-Studio sempre tindrà feina :).
Fragment N8: punter nul
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);
}Advertència de PVS-Studio: V522 [CWE-476] Es pot produir una desreferenciació del punter nul 'Tipus'. PrettyFunctionDumper.cpp 233
A més dels controladors d'errors, les funcions d'impressió de depuració no es solen provar. Només tenim un cas així davant nostre. La funció està esperant l'usuari, que en comptes de resoldre els seus problemes, es veurà obligat a solucionar-ho.
Dret:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";Fragment N9: punter nul
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());
....
}Advertència de PVS-Studio: V522 [CWE-476] Es pot produir una desreferenciació del punter nul 'Ty'. SearchableTableEmitter.cpp 614
Crec que tot està clar i no requereix cap explicació.
Fragment N10: error ortogràfic
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;
}Advertència de PVS-Studio: La variable 'Identificador->Tipus' s'assigna a ella mateixa. FormatTokenLexer.cpp 249
No té sentit assignar-se una variable a si mateixa. El més probable és que volien escriure:
Identifier->Type = Question->Type;Fragment N11: Trencament sospitós
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Advertència de PVS-Studio: [CWE-478] Considereu la possibilitat d'inspeccionar la declaració 'switch'. És possible que falti el primer operador "cas". SystemZAsmParser.cpp 652
Hi ha un operador molt sospitós al principi trencar. T'has oblidat d'escriure alguna cosa més aquí?
Fragment N12: Comprovació d'un punter després de desreferenciar
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");
....
}Advertència de PVS-Studio: [CWE-476] El punter 'Callee' es va utilitzar abans de verificar-lo contra nullptr. Consulteu les línies: 172, 174. AMDGPUInline.cpp 172
Punter Callee al principi és desreferenciat en el moment en què es crida la funció getTTI.
I després resulta que aquest punter s'ha de comprovar per a la igualtat nullptr:
if (!Callee || Callee->isDeclaration())Però és massa tard...
Fragment N13 - N...: Comprovació d'un punter després de desreferenciar
La situació comentada al fragment de codi anterior no és única. Apareix aquí:
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()) { // <=
....
}Advertiment de PVS-Studio: V595 [CWE-476] El punter 'CalleeFn' es va utilitzar abans de verificar-lo amb nullptr. Comproveu les línies: 1079, 1081. SimplifyLibCalls.cpp 1079
I aquí:
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()); // <=
....
}Advertiment de PVS-Studio: V595 [CWE-476] El punter 'ND' es va utilitzar abans de verificar-lo amb nullptr. Comproveu les línies: 532, 534. SemaTemplateInstantiateDecl.cpp 532
I aquí:
- V595 [CWE-476] El punter "U" es va utilitzar abans de verificar-se amb nullptr. Línies de comprovació: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] El punter 'ND' es va utilitzar abans de verificar-se amb nullptr. Comproveu les línies: 2149, 2151. SemaTemplateInstantiate.cpp 2149
I aleshores no em va interessar estudiar els avisos amb el número V595. Així que no sé si hi ha més errors similars a més dels que s'enumeren aquí. El més probable és que n'hi hagi.
Fragment N17, N18: Canvi sospitós
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Advertència de PVS-Studio: [CWE-190] Penseu en inspeccionar l'expressió '~(Mida - 1) << 1'. Desplaçament de bits del valor de 32 bits amb una expansió posterior al tipus de 64 bits. AArch64AddressingModes.h 260
Pot ser que no sigui un error i el codi funcioni exactament com es pretén. Però aquest és clarament un lloc molt sospitós i s'ha de comprovar.
Diguem la variable mida és igual a 16, i llavors l'autor del codi va planejar obtenir-lo en una variable NImms valor:
1111111111111111111111111111111111111111111111111111111111100000
Tanmateix, en realitat el resultat serà:
0000000000000000000000000000000011111111111111111111111111100000
El fet és que tots els càlculs es fan amb el tipus sense signar de 32 bits. I només llavors, aquest tipus sense signar de 32 bits s'ampliarà implícitament uint64_t. En aquest cas, els bits més significatius seran zero.
Podeu solucionar la situació així:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;Situació semblant: V629 [CWE-190] Considereu la possibilitat d'inspeccionar l'expressió 'Immr << 6'. Desplaçament de bits del valor de 32 bits amb una expansió posterior al tipus de 64 bits. AArch64AddressingModes.h 269
Fragment N19: falta la paraula clau else?
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");
}
....
}Advertència de PVS-Studio: [CWE-670] Considereu la possibilitat d'inspeccionar la lògica de l'aplicació. És possible que falti la paraula clau "else". AMDGPUAsmParser.cpp 5655
Aquí no hi ha cap error. Des del llavors bloc de la primera if acaba amb continuar, llavors no importa, hi ha una paraula clau else o no. De qualsevol manera, el codi funcionarà igual. Encara es troba a faltar else fa que el codi sigui més poc clar i perillós. Si en el futur continuar desapareix, el codi començarà a funcionar d'una manera completament diferent. Al meu entendre, és millor afegir else.
Fragment N20: Quatre errors tipogràfics del mateix tipus
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;
}Advertències de PVS-Studio:
- V655 [CWE-480] Les cadenes es van concatenar però no s'utilitzen. Penseu en inspeccionar l'expressió "Result + Name.str()". Símbol.cpp 32
- V655 [CWE-480] Les cadenes es van concatenar però no s'utilitzen. Penseu en inspeccionar l'expressió "Result + "(Classe ObjC)" + Name.str()". Símbol.cpp 35
- V655 [CWE-480] Les cadenes es van concatenar però no s'utilitzen. Penseu en inspeccionar l'expressió "Result + "(ObjC Class EH)" + Name.str()". Símbol.cpp 38
- V655 [CWE-480] Les cadenes es van concatenar però no s'utilitzen. Penseu en inspeccionar l'expressió "Resultat + "(ObjC IVar)" + Nom.str()". Símbol.cpp 41
Per accident, s'utilitza l'operador + en comptes de l'operador +=. El resultat són dissenys sense sentit.
Fragment N21: Comportament no definit
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();
}
}
}Intenta trobar tu mateix el codi perillós. I aquesta és una imatge per distreure l'atenció per no mirar immediatament la resposta:
Advertència de PVS-Studio: [CWE-758] S'utilitza una construcció perillosa: 'FeaturesMap[Op] = FeaturesMap.size()', on 'FeaturesMap' és de la classe 'map'. Això pot provocar un comportament indefinit. RISCVCompressInstEmitter.cpp 490
Línia del problema:
FeaturesMap[Op] = FeaturesMap.size();Si element Op no es troba, llavors es crea un nou element al mapa i s'escriu el nombre d'elements d'aquest mapa. Només es desconeix si es cridarà la funció mida abans o després d'afegir un element nou.
Fragment N22-N24: Tasques repetides
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;
}
....
}Advertència de PVS-Studio: [CWE-563] A la variable 'NType' se li assignen valors dues vegades consecutives. Potser això és un error. Comproveu les línies: 1663, 1664. MachOObjectFile.cpp 1664
No crec que hi hagi cap error real aquí. Només una tasca repetida innecessària. Però encara és un error.
Igualment:
- V519 [CWE-563] A la variable 'B.NDesc' s'assignen valors dues vegades consecutives. Potser això és un error. Comproveu les línies: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] A la variable se li assignen valors dues vegades consecutives. Potser això és un error. Línies de control: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Més reasignacions
Ara mirem una versió lleugerament diferent de la reassignació.
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;
....
}Advertència de PVS-Studio: V519 [CWE-563] La variable 'Alineació' s'assignen valors dues vegades consecutives. Potser això és un error. Comproveu les línies: 1158, 1160. LoadStoreVectorizer.cpp 1160
Aquest és un codi molt estrany que aparentment conté un error lògic. Al principi, variable Alineació s'assigna un valor en funció de la condició. I aleshores l'assignació es torna a produir, però ara sense cap control.
Situacions semblants es poden veure aquí:
- V519 [CWE-563] A la variable 'Efectes' s'assignen valors dues vegades consecutives. Potser això és un error. Comproveu les línies: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] La variable 'ExpectNoDerefChunk' s'assignen valors dues vegades consecutives. Potser això és un error. Comproveu les línies: 4970, 4973. SemaType.cpp 4973
Fragment N28: condició sempre veritable
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;
}
....
}Advertència de PVS-Studio: [CWE-571] L'expressió 'nextByte != 0x90' sempre és certa. X86DisassemblerDecoder.cpp 379
Comprovar no té sentit. Variable nextByte sempre no és igual al valor 0x90, que es desprèn de la comprovació anterior. Això és una mena d'error lògic.
Fragment N29 - N...: Condicions sempre cert/fals
L'analitzador emet molts avisos que tota la condició () o part d'aquest () sempre és cert o fals. Sovint, aquests no són errors reals, sinó simplement codi descuidat, resultat de l'expansió de macros, i similars. Tanmateix, té sentit mirar tots aquests avisos, ja que de tant en tant es produeixen errors lògics genuïns. Per exemple, aquesta secció del codi és sospitosa:
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;
....
}Advertència de PVS-Studio: [CWE-570] Una part de l'expressió condicional és sempre falsa: RegNo == 0xe. ARMDisassembler.cpp 939
La constant 0xE és el valor 14 en decimal. Examen RegNo == 0xe no té sentit perquè si RegNo > 13, aleshores la funció completarà la seva execució.
Hi havia molts altres avisos amb els ID V547 i V560, però igual , no m'interessava estudiar aquestes advertències. Ja estava clar que tenia prou material per escriure un article :). Per tant, es desconeix quants errors d'aquest tipus es poden identificar a LLVM mitjançant PVS-Studio.
Et donaré un exemple de per què estudiar aquests desencadenants és avorrit. L'analitzador té tota la raó a l'hora d'emetre una advertència per al codi següent. Però això no és un error.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Advertiment de PVS-Studio: V547 [CWE-570] L'expressió '!HasError' sempre és falsa. UnwrappedLineParser.cpp 1635
Fragment N30: Retorn sospitó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();
}
....
}Advertència de PVS-Studio: [CWE-670] Un "retorn" incondicional dins d'un bucle. R600OptimizeVectorRegisters.cpp 63
Es tracta d'un error o d'una tècnica específica que pretén explicar alguna cosa als programadors que llegeixen el codi. Aquest disseny no m'explica res i sembla molt sospitós. Val més no escriure així :).
Cansat? Aleshores és el moment de fer te o cafè.
Defectes identificats per nous diagnòstics
Crec que amb 30 activacions de diagnòstics antics n'hi ha prou. Vegem ara quines coses interessants es poden trobar amb els nous diagnòstics que van aparèixer després a l'analitzador xecs. En total, es van afegir 66 diagnòstics de propòsit general a l'analitzador C++ durant aquest temps.
Fragment N31: codi inabastable
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();
}Advertència de PVS-Studio: [CWE-561] S'ha detectat un codi inabastable. És possible que hi hagi un error. ExecutionUtils.cpp 146
Com podeu veure, ambdues branques de l'operador if finalitza amb una trucada a l'operador return. En conseqüència, el contenidor CtorDtorsByPriority mai s'esborrarà.
Fragment N32: codi inabastable
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;
}Advertiment de PVS-Studio: V779 [CWE-561] S'ha detectat un codi inabastable. És possible que hi hagi un error. LLParser.cpp 835
Situació interessant. Mirem primer aquest lloc:
return ParseTypeIdEntry(SummaryID);
break;A primera vista, sembla que aquí no hi ha cap error. Sembla l'operador trencar n'hi ha un més aquí, i simplement podeu eliminar-lo. Tanmateix, no tot és tan senzill.
L'analitzador emet un avís a les línies:
Lex.setIgnoreColonInIdentifiers(false);
return false;I, de fet, aquest codi és inabastable. Tots els casos a interruptor acaba amb una trucada de l'operador return. I ara sol sense sentit trencar no sembla tan inofensiu! Potser una de les branques hauria d'acabar amb trencar, no encès return?
Fragment N33: restabliment aleatori de bits alts
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);
....
}Advertència de PVS-Studio: La mida de la màscara de bits és menor que la mida del primer operand. Això provocarà la pèrdua de bits més alts. RuntimeDyld.cpp 815
Tingueu en compte que la funció getStubAlignment retorna tipus sense signar. Calculem el valor de l'expressió, suposant que la funció retorna el valor 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Ara observeu que la variable DataSize té un tipus sense signar de 64 bits. Resulta que quan es realitza l'operació DataSize i 0xFFFFFFF8u, els trenta-dos bits d'ordre superior es restabliran a zero. Molt probablement, això no és el que volia el programador. Sospito que volia calcular: DataSize & 0xFFFFFFFFFFFFFFF8u.
Per corregir l'error, hauríeu d'escriure això:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);O bé:
DataSize &= ~(getStubAlignment() - 1ULL);Fragment N34: emissió de tipus explícit fallida
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);
....
}Advertència de PVS-Studio: [CWE-190] Possible desbordament. Penseu en convertir operands de l'operador 'NumElts * Scale' al tipus 'size_t', no al resultat. X86ISelLowering.h 1577
El càsting de tipus explícit s'utilitza per evitar el desbordament quan es multipliquen variables de tipus int. No obstant això, el tipus de fosa explícit aquí no protegeix contra el desbordament. Primer, es multiplicaran les variables i només aleshores el resultat de la multiplicació de 32 bits s'ampliarà al tipus .
Fragment N35: no s'ha pogut copiar i enganxar
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;
}
....
}[CWE-682] Es van trobar dos fragments de codi similars. Potser es tracta d'una errada d'ortografia i s'hauria d'utilitzar la variable "Op1" en comptes de "Op0". InstCombineCompares.cpp 5507
Aquest nou diagnòstic interessant identifica situacions en què s'ha copiat un fragment de codi i s'han començat a canviar alguns noms, però en un lloc no l'han corregit.
Tingueu en compte que al segon bloc van canviar L'0 en L'1. Però en un lloc no ho van arreglar. El més probable és que s'hauria d'haver escrit així:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Fragment N36: Confusió variable
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Advertència de PVS-Studio: [CWE-563] La variable 'Mode' s'assigna però no s'utilitza al final de la funció. SIModeRegister.cpp 48
És molt perillós donar als arguments de funció els mateixos noms que els membres de la classe. És molt fàcil confondre's. Només tenim un cas així davant nostre. Aquesta expressió no té sentit:
Mode &= Mask;L'argument de la funció canvia. Això és tot. Aquest argument ja no s'utilitza. El més probable és que l'hauries d'haver escrit així:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Fragment N37: Confusió variable
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;
}Advertència PVS-Studio: V1001 [CWE-563] La variable 'Size' s'assigna però no s'utilitza al final de la funció. Object.cpp 424
La situació és semblant a l'anterior. S'ha d'escriure:
this->Size += this->EntrySize;Fragment N38-N47: S'han oblidat de comprovar l'índex
Anteriorment, vam mirar exemples de desencadenament diagnòstic . La seva essència és que el punter es desreferencia al principi i només després es verifica. Diagnòstic jove té el sentit contrari, però també revela molts errors. Identifica situacions en què el punter es va comprovar al principi i després s'ha oblidat de fer-ho. Vegem aquests casos dins de LLVM.
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()); // <=
....
}Advertiment de PVS-Studio: V1004 [CWE-476] El punter "Ptr" s'ha utilitzat de manera insegura després de verificar-lo contra nullptr. Comproveu les línies: 729, 738. TargetTransformInfoImpl.h 738
Variable ptr pot ser igual nullptr, com demostra el xec:
if (Ptr != nullptr)No obstant això, a sota d'aquest punter es desfereix sense comprovació prèvia:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Considerem un altre cas semblant.
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(); // <=
....
}Advertiment de PVS-Studio: V1004 [CWE-476] El punter "FD" es va utilitzar de manera insegura després de verificar-lo contra nullptr. Comproveu les línies: 3228, 3231. CGDebugInfo.cpp 3231
Fixeu-vos en el senyal FD. Estic segur que el problema és clarament visible i no cal una explicació especial.
I més enllà:
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()); // <=
....
}Advertiment de PVS-Studio: V1004 [CWE-476] El punter "PtrTy" s'ha utilitzat de manera insegura després de verificar-lo contra nullptr. Comproveu les línies: 960, 965. InterleavedLoadCombinePass.cpp 965
Com protegir-se d'aquests errors? Estigueu més atents a Code-Review i utilitzeu l'analitzador estàtic PVS-Studio per comprovar regularment el vostre codi.
No té sentit citar altres fragments de codi amb errors d'aquest tipus. Deixaré només una llista d'advertiments a l'article:
- V1004 [CWE-476] El punter "Expr" es va utilitzar de manera insegura després de verificar-lo amb nullptr. Comproveu les línies: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] El punter "PI" es va utilitzar de manera insegura després de verificar-lo contra nullptr. Comproveu les línies: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] El punter 'StatepointCall' es va utilitzar de manera insegura després de verificar-lo amb nullptr. Consulteu les línies: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] El punter "RV" es va utilitzar de manera insegura després de verificar-lo contra nullptr. Consulteu les línies: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] El punter 'CalleeFn' es va utilitzar de manera insegura després de verificar-lo amb nullptr. Comproveu les línies: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] El punter "TC" es va utilitzar de manera insegura després de verificar-lo contra nullptr. Línies de control: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: no és crític, però és un defecte (possible fuga de memòria)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Advertència de PVS-Studio: [CWE-460] S'afegeix un punter sense propietari al contenidor "Estratègies" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-isel-fuzzer.cpp 58
Per afegir un element al final d'un contenidor com std::vector > no pots escriure només xxx.push_back (X nova), ja que no hi ha cap conversió implícita de X* в std::unique_ptr.
Una solució habitual és escriure xxx.emplace_back(nou X)ja que compila: mètode emplace_back construeix un element directament a partir dels seus arguments i, per tant, pot utilitzar constructors explícits.
No és segur. Si el vector està ple, la memòria es reassigna. L'operació de reassignació de memòria pot fallar, provocant una excepció std::bad_alloc. En aquest cas, el punter es perdrà i l'objecte creat mai s'eliminarà.
Una solució segura és crear unique_ptrque serà el propietari del punter abans que el vector intenti reassignar la memòria:
xxx.push_back(std::unique_ptr<X>(new X))Des de C++14, podeu utilitzar 'std::make_unique':
xxx.push_back(std::make_unique<X>())Aquest tipus de defecte no és crític per a LLVM. Si no es pot assignar memòria, el compilador simplement s'aturarà. No obstant això, per a aplicacions amb llarg , que no només pot acabar si l'assignació de memòria falla, això pot ser un veritable error desagradable.
Així, tot i que aquest codi no suposa una amenaça pràctica per a LLVM, em va semblar útil parlar d'aquest patró d'error i que l'analitzador PVS-Studio ha après a identificar-lo.
Altres advertències d'aquest tipus:
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Passes" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. PassManager.h 546
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "AAs" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. AliasAnalysis.h 324
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Entrades" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "AllEdges" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. CFGMST.h 268
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "VMaps" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Registres" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. FDRLogBuilder.h 30
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor 'PendingSubmodules' pel mètode 'emplace_back'. Es produirà una fuga de memòria en cas d'excepció. ModuleMap.cpp 810
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Objectes" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. DebugMap.cpp 88
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Estratègies" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 685
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 686
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 688
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 689
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 690
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 691
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 692
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 693
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Modificadors" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. llvm-stress.cpp 694
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor "Operands" pel mètode "emplace_back". Es produirà una fuga de memòria en cas d'excepció. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor 'Stash' pel mètode 'emplace_back'. Es produirà una fuga de memòria en cas d'excepció. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] S'afegeix un punter sense propietari al contenidor 'Matchers' pel mètode 'emplace_back'. Es produirà una fuga de memòria en cas d'excepció. GlobalISelEmitter.cpp 2702
Conclusió
Vaig emetre 60 avisos en total i després vaig parar. Hi ha altres defectes que l'analitzador PVS-Studio detecta a LLVM? Sí, ho tinc. No obstant això, quan estava escrivint fragments de codi per a l'article, era tard al vespre, o més aviat fins i tot de nit, i vaig decidir que era hora de dir-ho com a dia.
Espero que us hagi semblat interessant i voldreu provar l'analitzador PVS-Studio.
Podeu descarregar l'analitzador i obtenir la clau del buscamines a .
El més important, utilitzeu l'anàlisi estàtica amb regularitat. Comprovacions puntuals, realitzat per nosaltres per tal de popularitzar la metodologia d'anàlisi estàtica i PVS-Studio no són un escenari normal.
Molta sort per millorar la qualitat i la fiabilitat del vostre codi!
Si voleu compartir aquest article amb un públic de parla anglesa, utilitzeu l'enllaç de traducció: Andrey Karpov. .
Font: www.habr.com