
Sono passati più di due anni dall'ultima verifica del codice del progetto LLVM effettuata con il nostro analizzatore PVS-Studio. Assicuriamoci che l'analizzatore PVS-Studio rimanga lo strumento leader per scoprire errori e vulnerabilità potenziali. A questo scopo, verifichiamo e troviamo nuovi errori nella release LLVM 8.0.0.
Articolo che deve essere scritto
Onestamente, non avevo voglia di scrivere quest'articolo. Non è interessante discutere di un progetto che abbiamo già controllato più volte (, , ). Sarebbe meglio parlare di qualcosa di nuovo, ma non ho scelta.
Ogni volta che esce una nuova versione di LLVM o viene aggiornato , riceviamo domande del tipo:
Guarda, la nuova versione di Clang Static Analyzer è riuscita a trovare nuovi errori! Penso che la rilevanza nell'utilizzare PVS-Studio stia diminuendo. Clang trova più errori rispetto a prima e si sta avvicinando alle capacità di PVS-Studio. Cosa ne pensi?
A questo, mi viene sempre voglia di rispondere qualcosa del tipo:
Anche noi non stiamo fermi! Abbiamo notevolmente migliorato le capacità dell'analizzatore PVS-Studio. Quindi non preoccupatevi, continuiamo a mantenere la nostra leadership come prima.
Sfortunatamente, questa è una risposta poco soddisfacente. Non ci sono prove. È proprio per questo che sto scrivendo questo articolo. Quindi, il progetto LLVM è stato controllato ancora una volta e sono stati trovati vari errori. Quelli che mi sono sembrati interessanti, ora li mostrerò. Questi errori non possono essere trovati da Clang Static Analyzer (o è estremamente scomodo farlo). E noi possiamo farlo. Infatti, ho trovato e annotato tutti questi errori in una sola serata.
Scrivere l'articolo, tuttavia, si è protratto per diverse settimane. Non riuscivo a convincermi a mettere tutto questo in forma di testo :).
A proposito, se siete interessati a quali tecnologie sono utilizzate nell'analizzatore PVS-Studio per rilevare errori e vulnerabilità potenziali, vi propongo di dare un'occhiata a questa .
Nuove e vecchie diagnosi
Come già accennato, circa due anni fa il progetto LLVM è stato nuovamente controllato e gli errori trovati sono stati corretti. Ora, in questo articolo, verrà presentato un nuovo lotto di errori. Perché sono stati trovati nuovi errori? Ci sono 3 motivi:
- Il progetto LLVM è in sviluppo, il codice esistente viene modificato e ne viene scritto di nuovo. È evidente che nel codice modificato e nuovo ci sono nuovi errori. Questo dimostra bene che l'analisi statica dovrebbe essere applicata regolarmente, e non sporadicamente. I nostri articoli mostrano bene le capacità dell'analizzatore PVS-Studio, ma ciò non ha alcuna relazione con il miglioramento della qualità del codice e la riduzione dei costi di correzione degli errori. Utilizzate l'analizzatore statico di codice regolarmente!
- Stiamo affinando e migliorando le diagnosi esistenti. Pertanto, l'analizzatore può rilevare errori che non erano stati notati nei controlli precedenti.
- In PVS-Studio sono state aggiunte nuove diagnosi che non c'erano due anni fa. Ho deciso di evidenziarle in una sezione separata per mostrare chiaramente lo sviluppo di PVS-Studio.
Difetti rivelati dalle diagnosi esistenti due anni fa
Frammento N1: Copy-Paste
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Aggiunto in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Aggiunto in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Aggiunto in 7.0
Name == "avx512.cvtusi2sd" || // Aggiunto in 7.0
Name.startswith("avx512.mask.permvar.") || // Aggiunto in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Aggiunto in 7.0 // <=
Name == "sse2.pmulu.dq" || // Aggiunto in 7.0
Name == "sse41.pmuldq" || // Aggiunto in 7.0
Name == "avx2.pmulu.dq" || // Aggiunto in 7.0
....
}Avviso PVS-Studio: [CWE-570] Ci sono sub-espressioni identiche ‘Name.startswith(«avx512.mask.permvar.»)’ a sinistra e a destra dell'operatore ‘||’. AutoUpgrade.cpp 73
Viene controllato due volte se il nome inizia con la sottostringa «avx512.mask.permvar.». Nella seconda verifica si voleva chiaramente scrivere qualcos'altro, ma si è dimenticati di correggere il testo copiato.
Frammento N2: Errore di battitura
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;
....
}Avviso PVS-Studio: V501 Ci sono sub-espressioni identiche ‘CXNameRange_WantQualifier’ a sinistra e a destra dell'operatore ‘|’. CIndex.cpp 7245
A causa di un errore di battitura viene utilizzata due volte la stessa costante nominata CXNameRange_WantQualifier.
Frammento N3: Confusione con le priorità degli operatori
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}Avviso PVS-Studio: [CWE-783] Forse l'operatore ‘?:’ funziona in modo diverso da quel che ci si aspettava. L'operatore ‘?:’ ha una priorità inferiore rispetto all'operatore ‘==’. PPCTargetTransformInfo.cpp 404
A mio avviso, questo è un errore molto elegante. Sì, so di avere una concezione strana della bellezza :).
Attualmente, secondo , l'espressione viene calcolata nel seguente modo:
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0Da un punto di vista pratico, tale condizione non ha senso, poiché può essere semplificata in:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())È un chiaro errore. Probabilmente, 0/1 si voleva confrontare con la variabile Index. Per correggere il codice, è necessario aggiungere parentesi attorno all'operatore ternario:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))A proposito, l'operatore ternario è molto pericoloso e provoca errori logici. Fai attenzione quando lo usi e non essere avaro nell'aggiungere parentesi. Ho trattato questo argomento , nel capitolo "Attenzione all'operatore ?: e racchiudetelo tra parentesi".
Fragmento N4, N5: Puntatore nullo
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast(Result);
....
LHS = dyn_cast(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("non posso convertire '") + LHS->getAsString() +
"' in stringa");
return nullptr;
}
....
}Avviso PVS-Studio: [CWE-476] Potrebbe avvenire dereferenziazione del puntatore nullo ‘LHS’. TGParser.cpp 2152
Se il puntatore LHS risulta nullo, deve essere emesso un avviso. Tuttavia, ciò che accadrà è la dereferenziazione di quel puntatore nullo: LHS->getAsString().
Questa è una situazione comune, in cui l'errore si nasconde nel gestore degli errori, poiché nessuno li testa. Gli analizzatori statici verificano tutto il codice raggiungibile, indipendentemente da quanto spesso venga utilizzato. Questo è un ottimo esempio di come l'analisi statica integri altre tecniche di test e protezione dagli errori.
Un errore simile nella gestione del puntatore RHS è stato commesso nel codice poco più in basso: V522 [CWE-476] Potrebbe avvenire dereferenziazione del puntatore nullo ‘RHS’. TGParser.cpp 2186
Fragmento N6: Uso di un puntatore dopo il trasferimento
static Expected
ExtractBlocks(....)
{
....
std::unique_ptr ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Funzione non trovata??");
MiscompiledFunctions.push_back(NewF);
}
....
}Avviso PVS-Studio: V522 [CWE-476] Potrebbe avvenire dereferenziazione del puntatore nullo ‘ProgClone’. Miscompilation.cpp 601
All'inizio, il puntatore intelligente ProgClone cessa di possedere l'oggetto:
BD.setNewProgram(std::move(ProgClone));In realtà, ora ProgClone è un puntatore nullo. Pertanto, poco più in basso deve avvenire la dereferenziazione di un puntatore nullo:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);Ma, in realtà, ciò non accadrà! Nota che il ciclo non si esegue affatto.
All'inizio, il contenitore MiscompiledFunctions viene svuotato:
MiscompiledFunctions.clear();Successivamente, la dimensione di questo contenitore viene utilizzata nella condizione del ciclo:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {È facile vedere che il ciclo non parte. Penso che anche questo sia un errore, e il codice dovrebbe essere scritto in modo diverso.
Sembra che ci siamo imbattuti in quella famosa parità di errori! Un errore maschera l'altro :).
Fragmento N7: Uso di un puntatore dopo il trasferimento
static Expected TestOptimizer(BugDriver &BD, std::unique_ptr Test,
std::unique_ptr Safe) {
outs() << " Ottimizzando le funzioni in fase di test: ";
std::unique_ptr Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Errore nell'esecuzione di questa sequenza di passaggi"
<< " sul programma di input!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}Avviso PVS-Studio: V522 [CWE-476] Potrebbe avvenire dereferenziazione del puntatore nullo ‘Test’. Miscompilation.cpp 709
Ancora una volta la stessa situazione. Inizialmente, il contenuto dell'oggetto viene spostato, e poi viene utilizzato come se nulla fosse. Incontro sempre più spesso questa situazione nel codice dei programmi, da quando in C++ è stata introdotta la semantica del trasferimento. È per questo che amo il linguaggio C++! Emergono sempre nuovi modi per farsi del male. L'analizzatore PVS-Studio avrà sempre lavoro :).
Fragmento N8: Puntatore nullo
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "";
else
Type->dump(*this);
}Avviso PVS-Studio: V522 [CWE-476] Potrebbe avvenire dereferenziazione del puntatore nullo ‘Type’. PrettyFunctionDumper.cpp 233
Oltre ai gestori di errori, le funzioni per la stampa di dati di debug di solito non vengono testate. Proprio questo è il caso. La funzione aspetta l'utente, che invece di risolvere i suoi problemi, dovrà occuparsi della sua correzione.
Corretto:
if (Type)
Type->dump(*this);
else
Printer << "";Fragmento N9: Puntatore nullo
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // getAsString() + " vs. " + // getType()->getAsString());
....
}Avviso PVS-Studio: V522 [CWE-476] Potrebbe verificarsi dereferenziazione del puntatore nullo 'Ty'. SearchableTableEmitter.cpp 614
Credo che sia tutto chiaro e non richieda ulteriori spiegazioni.
Frammento N10: Errore di battitura
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;
}Avviso PVS-Studio: La variabile 'Identifier->Type' viene assegnata a se stessa. FormatTokenLexer.cpp 249
Non ha senso assegnare una variabile a se stessa. Probabilmente si voleva scrivere:
Identifier->Type = Question->Type;Frammento N11: Break sospetto
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}Avviso PVS-Studio: [CWE-478] Considera di ispezionare l'istruzione 'switch'. È possibile che il primo operatore 'case' sia mancante. SystemZAsmParser.cpp 652
All'inizio è presente un operatore molto sospetto break. Non si è dimenticati di scrivere qualcosa qui?
Frammento N12: Controllo del puntatore dopo dereferenziazione
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");
....
}Avviso PVS-Studio: [CWE-476] Il puntatore 'Callee' è stato utilizzato prima di essere verificato contro nullptr. Controllare le righe: 172, 174. AMDGPUInline.cpp 172
Puntatore Callee viene dereferenziato all'inizio durante la chiamata della funzione getTTI.
E poi si scopre che questo puntatore deve essere verificato per uguaglianza nullptr:
if (!Callee || Callee->isDeclaration())Ma è già troppo tardi…
Frammento N13 — N…: Controllo del puntatore dopo dereferenziazione
La situazione considerata nel frammento di codice precedente non è unica. Si presenta qui:
static Value *optimizeDoubleFP(CallInst *CI, IRBuilder &B,
bool isBinary, bool isPrecise = false) {
....
Function *CalleeFn = CI->getCalledFunction();
StringRef CalleeNm = CalleeFn->getName(); // getAttributes();
if (CalleeFn && !CalleeFn->isIntrinsic()) { // <=
....
}Avviso PVS-Studio: V595 [CWE-476] Il puntatore 'CalleeFn' è stato utilizzato prima di essere verificato contro nullptr. Controllare le righe: 1079, 1081. SimplifyLibCalls.cpp 1079
E qui:
void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
const Decl *Tmpl, Decl *New,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *OuterMostScope) {
....
NamedDecl *ND = dyn_cast(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null(ND->getDeclContext()); // isCXXInstanceMember()); // <=
....
}Avviso PVS-Studio: V595 [CWE-476] Il puntatore 'ND' è stato utilizzato prima di essere verificato contro nullptr. Controllare le righe: 532, 534. SemaTemplateInstantiateDecl.cpp 532
E qui:
- V595 [CWE-476] Il puntatore 'U' è stato utilizzato prima di essere verificato contro nullptr. Controllare le righe: 404, 407. DWARFFormValue.cpp 404
- V595 [CWE-476] Il puntatore 'ND' è stato utilizzato prima di essere verificato contro nullptr. Controllare le righe: 2149, 2151. SemaTemplateInstantiate.cpp 2149
E poi ho perso interesse nello studio degli avvisi con numero V595. Quindi non so se ci siano altri errori simili, oltre a quelli elencati qui. Probabilmente ci sono.
Frammento N17, N18: Spostamento sospetto
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}Avviso PVS-Studio: [CWE-190] Considera di ispezionare l'espressione ' ~(Size - 1) << 1'. Spostamento dei bit del valore a 32 bit con un successivo espansione al tipo a 64 bit. AArch64AddressingModes.h 260
Forse non è un errore, e il codice funziona proprio come previsto. Ma è sicuramente un posto molto sospetto e deve essere verificato.
Supponiamo che la variabile Dimensione sia uguale a 16, e quindi l'autore del codice avesse pianificato di ottenere nella variabile NImms il valore:
1111111111111111111111111111111111111111111111111111111111100000
Tuttavia, in realtà, otterremo il valore:
0000000000000000000000000000000011111111111111111111111111100000
Il fatto è che tutti i calcoli avvengono utilizzando un tipo unsigned a 32 bit. E solo allora, questo tipo unsigned a 32 bit verrà espanso implicitamente a uint64_t. In questo modo, i bit più significativi saranno zero.
Si può correggere la situazione in questo modo:
uint64_t NImms = ~static_cast(Size-1) << 1;Situazione analoga: V629 [CWE-190] Considera di ispezionare l'espressione 'Immr << 6'. Spostamento dei bit del valore a 32 bit con un successivo espansione al tipo a 64 bit. AArch64AddressingModes.h 269
Frammento N19: Parola chiave mancante 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 usa il token "vcc".
// Salta.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Gestisci gli argomenti opzionali
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Tipo di operando non valido");
}
....
}Avviso PVS-Studio: [CWE-670] Considera di ispezionare la logica dell'applicazione. È possibile che la parola chiave 'else' sia mancante. AMDGPUAsmParser.cpp 5655
Non c'è errore qui. Poiché il blocco then del primo in etcdhelper, che non modificherà il servizio kube-dns. termina con continue, non importa se c'è la parola chiave else o meno. In ogni caso, il codice funzionerà allo stesso modo. Tuttavia, il mancato uso di else rende il codice meno comprensibile e più pericoloso. Se in seguito continue scomparisse, il codice inizierebbe a funzionare in modo completamente diverso. A mio avviso, sarebbe meglio aggiungere else.
Frammento N20: Quattro errori di battitura simili
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;
}Avvisi PVS-Studio:
- V655 [CWE-480] Le stringhe sono state concatenate ma non sono utilizzate. Considera di controllare l'espressione ‘Result + Name.str()’. Symbol.cpp 32
- V655 [CWE-480] Le stringhe sono state concatenate ma non sono utilizzate. Considera di controllare l'espressione ‘Result + «(ObjC Class) » + Name.str()’. Symbol.cpp 35
- V655 [CWE-480] Le stringhe sono state concatenate ma non sono utilizzate. Considera di controllare l'espressione ‘Result + «(ObjC Class EH) » + Name.str()’. Symbol.cpp 38
- V655 [CWE-480] Le stringhe sono state concatenate ma non sono utilizzate. Considera di controllare l'espressione ‘Result + «(ObjC IVar) » + Name.str()’. Symbol.cpp 41
È stato utilizzato l'operatore + invece di +=. Di conseguenza, si ottengono costrutti privi di significato.
Frammento N21: Comportamento indefinito
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 non può essere vuoto");
for (auto &Op : Ops) {
assert(!Op.empty() && "Operatore vuoto");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}Prova a trovare il codice pericoloso da solo. Ecco un'immagine per distrarti, così non guardi subito la risposta:

Avviso PVS-Studio: [CWE-758] Viene utilizzata una costruzione pericolosa: ‘FeaturesMap[Op] = FeaturesMap.size()’, dove ‘FeaturesMap’ è di classe ‘map’. Questo potrebbe portare a un comportamento indefinito. RISCVCompressInstEmitter.cpp 490
Riga problematica:
FeaturesMap[Op] = FeaturesMap.size();Se l'elemento Op non è trovato, viene creato un nuovo elemento nella mappa e ci viene scritto il numero di elementi in questa mappa. Ma non è chiaro se la funzione sarà chiamata dimensione prima o dopo l'aggiunta del nuovo elemento.
Frammento N22-N24: Assegnazioni ripetute
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;
}
....
}Avviso PVS-Studio: [CWE-563] La variabile ‘NType’ è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 1663, 1664. MachOObjectFile.cpp 1664
Non credo che ci sia un vero errore qui. È solo un'assegnazione ripetuta superflua. Ma è comunque un errore.
Analogamente:
- V519 [CWE-563] La variabile ‘B.NDesc’ è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] La variabile è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 59, 61. coff2yaml.cpp 61
Frammento N25-N27: Altre assegnazioni ripetute
Ora consideriamo una variante leggermente diversa dell'assegnazione ripetuta.
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;
....
}Avviso PVS-Studio: V519 [CWE-563] La variabile ‘Alignment’ è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 1158, 1160. LoadStoreVectorizer.cpp 1160
Questo è un codice molto strano che apparentemente contiene un errore logico. All'inizio, alla variabile Alignment viene assegnato un valore a seconda della condizione. E poi avviene di nuovo l'assegnazione, ma ora senza alcun controllo.
Situazioni simili possono essere viste qui:
- V519 [CWE-563] La variabile ‘Effects’ è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] La variabile ‘ExpectNoDerefChunk’ è assegnata due volte consecutivamente. Forse è un errore. Controlla le righe: 4970, 4973. SemaType.cpp 4973
Frammento N28: Condizione sempre vera
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) // supporto per l'istruzione PAUSE
break;
}
....
}Avviso PVS-Studio: [CWE-571] L'espressione ‘nextByte != 0x90’ è sempre vera. X86DisassemblerDecoder.cpp 379
Il controllo non ha senso. La variabile nextByte è sempre diversa dal valore 0x90, che deriva dal controllo precedente. Si tratta di un qualche errore logico.
Frammento N29 — N…: Condizioni sempre vere/falsi
L'analizzatore restituisce molti avvisi che l'intera condizione () o parte di essa () è sempre vera o falsa. Spesso non si tratta di veri errori, ma solo di codice disordinato, il risultato di macro espanse e simili. Tuttavia, è utile esaminare tutti questi avvisi, poiché ogni tanto ci sono veri errori logici. Ad esempio, questo pezzo di codice è sospetto:
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;
....
}Avviso PVS-Studio: [CWE-570] Una parte dell'espressione condizionale è sempre false: RegNo == 0xe. ARMDisassembler.cpp 939
La costante 0xE è il valore 14 nel sistema decimale. Verifica RegNo == 0xe non ha senso, poiché se RegNo > 13, la funzione terminerà la sua esecuzione.
Ci sono stati numerosi altri avvisi con identificatori V547 e V560, ma, come nel caso di , non mi è interessato esaminare questi avvisi. Era già chiaro che avevo abbastanza materiale per scrivere un articolo :). Quindi non si sa quante di queste errori di questo tipo possano essere rilevati in LLVM utilizzando PVS-Studio.
Ecco un esempio del perché studiare questi allarmi sia noioso. L'analizzatore ha completamente ragione a generare un avviso sul seguente codice. Ma questo non è un errore.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}Avviso PVS-Studio: V547 [CWE-570] L'espressione '!HasError' è sempre false. UnwrappedLineParser.cpp 1635
Frammento N30: Return sospetto
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();
}
....
}Avviso PVS-Studio: [CWE-670] Un 'return' incondizionato all'interno di un ciclo. R600OptimizeVectorRegisters.cpp 63
Questo è un errore o una tecnica specifica per chiarire qualcosa ai programmatori che leggono il codice. Questa costruzione non spiega nulla e sembra molto sospetta. È meglio non scriverla in questo modo! :)
Stanco? È tempo di preparare un tè o un caffè.

Difetti rilevati da nuove diagnosi
Penso che 30 attivazioni delle vecchie diagnosi siano sufficienti. Ora vediamo cosa di interessante possiamo trovare con le nuove diagnosi che sono state aggiunte all'analizzatore dopo il check. In questo periodo, l'analizzatore C++ ha aggiunto 66 diagnosi generali.
Frammento N31: Codice irraggiungibile
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();
}Avviso PVS-Studio: [CWE-561] Codice irraggiungibile rilevato. Potrebbe esserci un errore. ExecutionUtils.cpp 146
Come vedete, entrambi i rami dell'operatore in etcdhelper, che non modificherà il servizio kube-dns. terminano con la chiamata all'operatore return. Pertanto, il contenitore CtorDtorsByPriority non sarà mai svuotato.
Frammento N32: Codice irraggiungibile
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(), "kind di riepilogo inatteso");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}Avviso PVS-Studio: V779 [CWE-561] Codice irraggiungibile rilevato. Potrebbe esserci un errore. LLParser.cpp 835
Situazione interessante. Esaminiamo innanzitutto questo punto:
return ParseTypeIdEntry(SummaryID);
break;A prima vista, sembra che non ci sia errore qui. Sembra che l'operatore break sia superfluo e possa semplicemente essere rimosso. Tuttavia, non è così semplice.
L'analizzatore genera un avviso sulle righe:
Lex.setIgnoreColonInIdentifiers(false);
return false;E in effetti, questo codice è irraggiungibile. Tutti i casi nell' switch terminano con la chiamata all'operatore return. E ora un solitario break non sembra così innocuo! Forse uno dei rami deve finire con break, e non con return?
Frammento N33: Azzeramento casuale dei bit superiori
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}Avviso PVS-Studio: La dimensione della maschera di bit è inferiore alla dimensione del primo operando. Questo causerà la perdita dei bit superiori. RuntimeDyld.cpp 815
Si noti che la funzione getStubAlignment restituisce un tipo unsigned. Calcoliamo il valore dell'espressione, supponendo che la funzione restituisca il valore 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFF8u
Ora, osservate che la variabile DataSize ha un tipo unsigned a 64 bit. Sembra che durante l'operazione DataSize & 0xFFFFFFF8u tutti i trentadue bit superiori saranno azzerati. Probabilmente non era questo che voleva il programmatore. Sospetto che volesse calcolare: DataSize & 0xFFFFFFFFFFFFFFF8u.
Per correggere l'errore, bisogna scrivere:
DataSize &= ~(static_cast(getStubAlignment()) - 1);Oppure così:
DataSize &= ~(getStubAlignment() - 1ULL);Frammento N34: Cast esplicito non riuscito
template
void scaleShuffleMask(int Scale, ArrayRef Mask,
SmallVectorImpl &ScaledMask) {
assert(0 < Scale && "Fattore di scala inatteso");
int NumElts = Mask.size();
ScaledMask.assign(static_cast(NumElts * Scale), -1);
....
}Avviso PVS-Studio: [CWE-190] Possibile overflow. Considerare il casting degli operandi dell'operatore 'NumElts * Scale' al tipo 'size_t', non al risultato. X86ISelLowering.h 1577
Il cast esplicito è utilizzato per prevenire overflow durante la moltiplicazione di variabili di tipo -int. Tuttavia, qui una conversione esplicita non protegge dall'overflow. All'inizio le variabili verranno moltiplicate, e solo dopo il risultato di 32 bit verrà esteso al tipo. .
Frammento N35: Copia-Incolla non riuscita
Istruzione *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] Sono stati trovati due frammenti di codice simili. Forse, questa è una svista e la variabile ‘Op1’ dovrebbe essere utilizzata invece di ‘Op0’. InstCombineCompares.cpp 5507
Questa nuova diagnosi interessante rivela situazioni in cui un frammento di codice è stato copiato e sono stati iniziati a cambiare alcuni nomi, ma in un punto non è stato corretto.
Nota che nel secondo blocco sono stati cambiati Op0 con Op1. Ma in un punto non è stato corretto. Probabilmente, doveva essere scritto così:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}Frammento N36: Confusione tra variabili
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};Avviso PVS-Studio: [CWE-563] La variabile ‘Mode’ viene assegnata ma non viene utilizzata alla fine della funzione. SIModeRegister.cpp 48
È molto pericoloso dare agli argomenti delle funzioni gli stessi nomi dei membri della classe. È molto facile confondersi. Questo è proprio un caso. Questa espressione non ha senso:
Mode &= Mask;Si modifica l'argomento della funzione. E basta. Questo argomento non viene più utilizzato. Probabilmente, doveva essere scritto così:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};Frammento N37: Confusione tra variabili
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(Sym));
Size += this->EntrySize;
}Avviso PVS-Studio: V1001 [CWE-563] La variabile ‘Size’ è assegnata ma non viene utilizzata alla fine della funzione. Object.cpp 424
La situazione è analoga a quella precedente. Dovrebbe essere scritto:
this->Size += this->EntrySize;Frammento N38-N47: Puntatore dimenticato da controllare
In precedenza abbiamo esaminato esempi di attivazione della diagnosi . L'essenza è che il puntatore viene dereferenziato all'inizio e solo dopo controllato. Questa nuova diagnosi è concettualmente opposta, ma identifica anche molti errori. Rivela situazioni in cui il puntatore è stato controllato all'inizio, ma poi ci si è dimenticati di farlo. Esaminiamo i casi trovati all'interno di LLVM.
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}Avviso PVS-Studio: V1004 [CWE-476] Il puntatore ‘Ptr’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 729, 738. TargetTransformInfoImpl.h 738
Variabile Ptr può essere uguale a nullptr, come dimostra il controllo:
if (Ptr != nullptr)Tuttavia, più in basso questo puntatore viene dereferenziato senza un controllo preliminare:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());Esaminiamo un altro caso simile.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast(GD.getDecl());
SmallVector ArgTypes;
if (FD) // parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs()->getCallConv(); // <=
....
}Avviso PVS-Studio: V1004 [CWE-476] Il puntatore ‘FD’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 3228, 3231. CGDebugInfo.cpp 3231
Nota il puntatore FD. Sono certo che il problema è ben visibile e non richiede spiegazioni particolari.
E ancora:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast(Ptr.getType());
if (!PtrTy) { // getPointerAddressSpace()); // <=
....
}Avviso PVS-Studio: V1004 [CWE-476] Il puntatore ‘PtrTy’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 960, 965. InterleavedLoadCombinePass.cpp 965
Come proteggersi da tali errori? Fate attenzione durante la revisione del codice e usate un analizzatore statico di codice come PVS-Studio per controlli regolari.
Non ha senso presentare altri frammenti di codice con errori di questo tipo. Lascio nell'articolo solo l'elenco degli avvisi:
- V1004 [CWE-476] Il puntatore ‘Expr’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Il puntatore ‘PI’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Il puntatore ‘StatepointCall’ è stato utilizzato in modo insicuro dopo essere stato verificato contro nullptr. Controlla le righe: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Il puntatore ‘RV’ è stato utilizzato in modo non sicuro dopo essere stato verificato contro nullptr. Controlla le righe: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Il puntatore ‘CalleeFn’ è stato utilizzato in modo non sicuro dopo essere stato verificato contro nullptr. Controlla le righe: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Il puntatore ‘TC’ è stato utilizzato in modo non sicuro dopo essere stato verificato contro nullptr. Controlla le righe: 1819, 1824. Driver.cpp 1824
Frammento N48-N60: Non critico, ma presente un difetto (possibile fuga di memoria)
std::unique_ptr createISelMutator() {
....
std::vector<std::unique_ptr> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}Avviso PVS-Studio: [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Strategies’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-isel-fuzzer.cpp 58
Per aggiungere un elemento alla fine di un contenitore di tipo std::vector<std::unique_ptr> non è possibile semplicemente scrivere xxx.push_back(new X), poiché non esiste una conversione implicita da X* in std::unique_ptr.
Una soluzione comune è scrivere xxx.emplace_back(new X), poiché si compila: il metodo emplace_back costruisce l'elemento direttamente dagli argomenti e quindi può utilizzare costruttori espliciti.
Questo non è sicuro. Se il vettore è pieno, si verifica la riallocazione della memoria. L'operazione di riallocazione della memoria può fallire, generando un'eccezione std::bad_alloc. In tal caso, il puntatore andrà perso e l'oggetto creato non verrà mai eliminato.
Una soluzione sicura è creare unique_ptr, che possiederà il puntatore fino a quando il vettore non tenterà di riallocare la memoria:
xxx.push_back(std::unique_ptr(new X))A partire da C++14, è possibile utilizzare ‘std::make_unique’:
xxx.push_back(std::make_unique())Questo tipo di difetto non è critico per LLVM. Se non riesce a allocare memoria, il lavoro del compilatore si fermerà semplicemente. Tuttavia, per applicazioni con un lungo , che non possono semplicemente terminare se non riescono ad allocare memoria, questo può essere un errore davvero sgradevole.
Quindi, sebbene questo codice non rappresenti un pericolo pratico per LLVM, ho ritenuto utile parlare di questo pattern di errori e di come l'analizzatore PVS-Studio abbia imparato a identificarlo.
Altri avvisi di questo tipo:
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Passes’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. PassManager.h 546
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘AAs’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. AliasAnalysis.h 324
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Entries’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘AllEdges’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. CFGMST.h 268
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘VMaps’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Records’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. FDRLogBuilder.h 30
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘PendingSubmodules’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. ModuleMap.cpp 810
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Objects’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. DebugMap.cpp 88
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Strategies’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 685
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 686
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 688
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 689
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 690
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 691
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 692
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 693
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Modifiers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. llvm-stress.cpp 694
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Operands’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Stash’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Un puntatore senza proprietario è stato aggiunto al contenitore ‘Matchers’ tramite il metodo ’emplace_back’. Si verificherà una fuga di memoria in caso di eccezione. GlobalISelEmitter.cpp 2702
Conclusione
Ho elencato un totale di 60 avvisi, dopo di che mi sono fermato. Ci sono altri difetti rilevati dall'analizzatore PVS-Studio in LLVM? Sì, ce ne sono. Tuttavia, mentre scrivevo frammenti di codice per l'articolo, era ormai tardi, anzi, notte fonda, e ho deciso che era il momento di smettere.
Spero che vi sia piaciuto e che desideriate provare l'analizzatore PVS-Studio.
Puoi scaricare l'analizzatore e ottenere una chiave di attivazione su .
La cosa più importante è utilizzare l'analisi statica regolarmente. Controlli singoli, effettuati da noi per promuovere la metodologia di analisi statica e PVS-Studio non sono uno scenario normale.
Buona fortuna nel migliorare la qualità e l'affidabilità del codice!
Se desideri condividere questo articolo con un pubblico anglofono, ti prego di utilizzare il link alla traduzione: Andrey Karpov. .
Fonte: habr.com
