Sono trascorsi più di due anni dall'ultimo controllo del codice del progetto LLVM utilizzando il nostro analizzatore PVS-Studio. Assicuriamoci che l'analizzatore PVS-Studio sia ancora uno strumento leader per l'identificazione di errori e potenziali vulnerabilità. Per fare ciò, controlleremo e troveremo nuovi errori nella versione LLVM 8.0.0.
Articolo da scrivere
Ad essere sincero, non volevo scrivere questo articolo. Non è interessante scrivere di un progetto che abbiamo già controllato più volte (
Ogni volta che viene rilasciata o aggiornata una nuova versione di LLVM
Guarda, la nuova versione di Clang Static Analyser ha imparato a trovare nuovi errori! Mi sembra che l'importanza dell'utilizzo di PVS-Studio stia diminuendo. Clang trova più errori di prima e raggiunge le capacità di PVS-Studio. Cosa ne pensi di questo?
A questo voglio sempre rispondere qualcosa del tipo:
Anche noi non stiamo con le mani in mano! Abbiamo migliorato significativamente le capacità dell'analizzatore PVS-Studio. Quindi non preoccuparti, continuiamo a guidare come prima.
Sfortunatamente, questa è una cattiva risposta. Non ci sono prove in esso. Ed è per questo che sto scrivendo questo articolo ora. Pertanto, il progetto LLVM è stato nuovamente controllato e in esso sono stati riscontrati numerosi errori. Ora mostrerò quelli che mi sono sembrati interessanti. Clang Static Analyser non riesce a trovare questi errori (o è estremamente scomodo farlo con il suo aiuto). Ma possiamo. Inoltre, ho trovato e annotato tutti questi errori in una sera.
Ma scrivere l’articolo ha richiesto diverse settimane. Non riuscivo proprio a mettere tutto questo nel testo :).
A proposito, se sei interessato a quali tecnologie vengono utilizzate nell'analizzatore PVS-Studio per identificare errori e potenziali vulnerabilità, ti suggerisco di familiarizzare con questo
Vecchia e nuova diagnostica
Come già notato, circa due anni fa il progetto LLVM è stato nuovamente controllato e gli errori riscontrati sono stati corretti. Ora questo articolo presenterà una nuova serie di errori. Perché sono stati trovati nuovi bug? Ci sono 3 ragioni per questo:
- Il progetto LLVM si sta evolvendo, modificando il vecchio codice e aggiungendo nuovo codice. Naturalmente ci sono nuovi errori nel codice modificato e scritto. Ciò dimostra chiaramente che l’analisi statica dovrebbe essere utilizzata regolarmente e non occasionalmente. I nostri articoli mostrano bene le capacità dell'analizzatore PVS-Studio, ma ciò non ha nulla a che fare con il miglioramento della qualità del codice e la riduzione dei costi di correzione degli errori. Utilizza regolarmente un analizzatore di codice statico!
- Stiamo finalizzando e migliorando la diagnostica esistente. Pertanto, l'analizzatore può identificare errori che non aveva notato durante le scansioni precedenti.
- In PVS-Studio sono apparse nuove diagnostiche che 2 anni fa non esistevano. Ho deciso di evidenziarli in una sezione separata per mostrare chiaramente lo sviluppo di PVS-Studio.
Difetti identificati dalla diagnostica esistente 2 anni fa
Frammento N1: Copia-Incolla
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
....
}
Avviso PVS-Studio:
Viene ricontrollato che il nome inizi con la sottostringa "avx512.mask.permvar.". Nel secondo controllo ovviamente volevano scrivere qualcos'altro, ma si sono 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;
....
}
Avvertenza PVS-Studio: V501 Sono presenti sottoespressioni identiche 'CXNameRange_WantQualifier' a sinistra e a destra di '|' operatore. CIndex.cpp 7245
A causa di un errore di battitura, la stessa costante con nome viene utilizzata due volte CXNameRange_WantQualifier.
Frammento N3: confusione con la precedenza 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:
Secondo me questo è un errore molto bello. Sì, lo so, ho strane idee sulla bellezza :).
Ora, secondo
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Dal punto di vista pratico tale condizione non ha senso, poiché può essere ridotta a:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Questo è un chiaro errore. Molto probabilmente, volevano confrontare 0/1 con una variabile Indice. 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. Stai molto attento e non essere avido con le parentesi. Ho esaminato questo argomento in modo più dettagliato
Frammento N4, N5: puntatore null
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;
}
....
}
Avviso PVS-Studio:
Se il puntatore LHS è nullo, deve essere emesso un avviso. Tuttavia, invece, questo stesso puntatore null verrà dereferenziato: LHS->getAsString().
Questa è una situazione molto tipica in cui un errore è nascosto in un gestore di errori, poiché nessuno lo verifica. Gli analizzatori statici controllano tutto il codice raggiungibile, indipendentemente dalla frequenza con cui viene utilizzato. Questo è un ottimo esempio di come l'analisi statica integra altre tecniche di test e protezione dagli errori.
Errore simile nella gestione del puntatore RHS consentito nel codice appena sotto: V522 [CWE-476] Potrebbe aver luogo il dereferenziamento del puntatore nullo 'RHS'. TGParser.cpp 2186
Frammento N6: Utilizzo del puntatore dopo lo spostamento
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);
}
....
}
Avviso PVS-Studio: V522 [CWE-476] Potrebbe verificarsi il dereferenziamento del puntatore nullo 'ProgClone'. Compilazione errata.cpp 601
All'inizio un puntatore intelligente ProgClone cessa di possedere l'oggetto:
BD.setNewProgram(std::move(ProgClone));
In effetti, adesso ProgClone è un puntatore nullo. Pertanto, un dereferenziamento del puntatore nullo dovrebbe verificarsi appena sotto:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Ma, in realtà, questo non accadrà! Si noti che il ciclo non viene effettivamente eseguito.
All'inizio del contenitore Funzioni non compilate cancellato:
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 inizia. Penso che anche questo sia un bug e il codice dovrebbe essere scritto diversamente.
Sembra che ci siamo imbattuti nella famosa parità di errori! Un errore ne maschera un altro :).
Frammento N7: Utilizzo del puntatore dopo lo spostamento
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;
}
....
}
Avviso PVS-Studio: V522 [CWE-476] Potrebbe aver luogo il dereferenziamento del puntatore nullo 'Test'. Compilazione errata.cpp 709
Di nuovo la stessa situazione. Inizialmente si sposta il contenuto dell'oggetto, quindi lo si utilizza come se nulla fosse accaduto. Vedo questa situazione sempre più spesso nel codice del programma dopo che la semantica del movimento è apparsa in C++. Questo è il motivo per cui adoro il linguaggio C++! Esistono sempre più nuovi modi per spararsi la gamba. L'analizzatore PVS-Studio funzionerà sempre :).
Frammento N8: puntatore null
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);
}
Avviso PVS-Studio: V522 [CWE-476] Potrebbe verificarsi un dereferenziamento del puntatore nullo 'Tipo'. PrettyFunctionDumper.cpp 233
Oltre ai gestori degli errori, le funzioni di debug della stampa solitamente non vengono testate. Abbiamo davanti a noi un caso del genere. La funzione attende l'utente che, invece di risolvere i suoi problemi, sarà costretto a risolverli.
correggere:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Frammento N9: puntatore null
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());
....
}
Avviso PVS-Studio: V522 [CWE-476] Potrebbe verificarsi un dereferenziamento del puntatore nullo 'Ty'. SearchableTableEmitter.cpp 614
Penso che tutto sia chiaro e non richieda 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:
Non ha senso assegnare una variabile a se stessa. Molto probabilmente volevano scrivere:
Identifier->Type = Question->Type;
Frammento N11: Rottura sospetta
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:
All'inizio c'è un operatore molto sospettoso rompere. Hai dimenticato di scrivere qualcos'altro qui?
Frammento N12: controllo di un puntatore dopo la 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:
L'indice chiamato all'inizio viene dereferenziato nel momento in cui viene chiamata la funzione prendiTTI.
E poi si scopre che questo puntatore dovrebbe essere controllato per l'uguaglianza nullptr:
if (!Callee || Callee->isDeclaration())
Ma è troppo tardi…
Frammento N13 - N...: controllo di un puntatore dopo la dereferenziazione
La situazione discussa nel frammento di codice precedente non è unica. Appare qui:
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()) { // <=
....
}
Avviso PVS-Studio: V595 [CWE-476] Il puntatore 'CalleeFn' è stato utilizzato prima di essere verificato rispetto a nullptr. Controlla 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<NamedDecl>(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <=
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember()); // <=
....
}
Avviso PVS-Studio: V595 [CWE-476] Il puntatore 'ND' è stato utilizzato prima di essere verificato rispetto a nullptr. Controlla le righe: 532, 534. SemaTemplateInstantiateDecl.cpp 532
E qui:
- V595 [CWE-476] Il puntatore 'U' è stato utilizzato prima di essere verificato rispetto a nullptr. Controllare le righe: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Il puntatore 'ND' è stato utilizzato prima di essere verificato rispetto a nullptr. Controlla le righe: 2149, 2151. SemaTemplateInstantiate.cpp 2149
E poi mi sono disinteressato a studiare gli avvertimenti con il numero V595. Quindi non so se ci sono altri errori simili oltre a quelli elencati qui. Molto probabilmente c'è.
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:
Potrebbe non essere un bug e il codice funziona esattamente come previsto. Ma questo è chiaramente un posto molto sospetto e deve essere controllato.
Diciamo la variabile Taglia è uguale a 16, e quindi l'autore del codice ha previsto di inserirlo in una variabile NImm valore:
1111111111111111111111111111111111111111111111111111111111100000
Tuttavia, in realtà il risultato sarà:
0000000000000000000000000000000011111111111111111111111111100000
Il fatto è che tutti i calcoli vengono eseguiti utilizzando il tipo senza segno a 32 bit. E solo allora, questo tipo senza segno a 32 bit verrà implicitamente espanso a uint64_t. In questo caso i bit più significativi saranno zero.
Puoi risolvere la situazione in questo modo:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Situazione simile: V629 [CWE-190] Considerare l'ispezione dell'espressione 'Immr << 6'. Bitshifting del valore a 32 bit con successiva espansione al tipo a 64 bit. AArch64AddressingModes.h 269
Frammento N19: parola chiave mancante altro?
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");
}
....
}
Avviso PVS-Studio:
Non ci sono errori qui. Dall'allora blocco del primo if finisce con continua, quindi non importa, c'è una parola chiave altro o no. In ogni caso il codice funzionerà allo stesso modo. Ancora mancato altro rende il codice più poco chiaro e pericoloso. Se in futuro continua scompare, il codice inizierà a funzionare in modo completamente diverso. Secondo me è meglio aggiungere altro.
Frammento N20: quattro errori di battitura dello stesso tipo
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;
}
Avvertenze di PVS-Studio:
- V655 [CWE-480] Le stringhe sono state concatenate ma non vengono utilizzate. Prendi in considerazione l'ispezione dell'espressione "Risultato + Nome.str()". Simbolo.cpp 32
- V655 [CWE-480] Le stringhe sono state concatenate ma non vengono utilizzate. Prendi in considerazione l'ispezione dell'espressione 'Result + "(ObjC Class)" + Name.str()'. Simbolo.cpp 35
- V655 [CWE-480] Le stringhe sono state concatenate ma non vengono utilizzate. Prendi in considerazione l'ispezione dell'espressione 'Result + "(ObjC Class EH) " + Name.str()'. Simbolo.cpp 38
- V655 [CWE-480] Le stringhe sono state concatenate ma non vengono utilizzate. Prendi in considerazione l'ispezione dell'espressione 'Result + "(ObjC IVar)" + Name.str()'. Simbolo.cpp 41
Per sbaglio, viene utilizzato l'operatore + al posto dell'operatore +=. Il risultato sono disegni 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 cannot be empty");
for (auto &Op : Ops) {
assert(!Op.empty() && "Empty operator");
if (FeaturesMap.find(Op) == FeaturesMap.end())
FeaturesMap[Op] = FeaturesMap.size();
}
}
}
Prova a trovare tu stesso il codice pericoloso. E questa è una foto per distrarre l'attenzione per non guardare subito la risposta:
Avviso PVS-Studio:
Linea del problema:
FeaturesMap[Op] = FeaturesMap.size();
Se elemento Op non viene trovato, viene creato un nuovo elemento nella mappa e lì viene scritto il numero di elementi in questa mappa. Non è noto se la funzione verrà chiamata Taglia prima o dopo l'aggiunta di un 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:
Non penso che ci sia un vero errore qui. Solo un compito ripetuto e non necessario. Ma pur sempre un errore.
Allo stesso modo:
- V519 [CWE-563] Alla variabile 'B.NDesc' vengono assegnati valori due volte consecutive. Forse questo è un errore. Controlla le righe: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Alla variabile vengono assegnati valori due volte consecutive. Forse questo è un errore. Controlla le righe: 59, 61. coff2yaml.cpp 61
Frammento N25-N27: ulteriori riassegnazioni
Ora diamo un'occhiata a una versione leggermente diversa della riassegnazione.
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] Alla variabile "Allineamento" vengono assegnati valori due volte consecutive. Forse questo è un errore. Controlla le righe: 1158, 1160. LoadStoreVectorizer.cpp 1160
Questo è un codice molto strano che apparentemente contiene un errore logico. All'inizio variabile allineamento viene assegnato un valore a seconda della condizione. E poi l'assegnazione avviene di nuovo, ma ora senza alcun controllo.
Situazioni simili possono essere viste qui:
- V519 [CWE-563] Alla variabile 'Effects' vengono assegnati valori due volte consecutive. Forse questo è un errore. Controllare le righe: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Alla variabile 'ExpectNoDerefChunk' vengono assegnati valori due volte consecutive. Forse questo è un errore. Controllare 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) // PAUSE instruction support // <=
break;
}
....
}
Avviso PVS-Studio:
Controllare non ha senso. Variabile nextByte sempre non uguale al valore 0x90, che consegue dal controllo precedente. Questo è una sorta di errore logico.
Frammento N29 - N...: Condizioni sempre vero/falso
L'analizzatore emette numerosi avvisi relativi all'intera condizione (
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:
La costante 0xE è il valore 14 in decimale. Visita medica RegNo == 0xe non ha senso perché se RegNo > 13, la funzione completerà la sua esecuzione.
C'erano molti altri avvisi con gli ID V547 e V560, ma come con
Ti farò un esempio del perché studiare questi trigger è noioso. L'analizzatore ha assolutamente ragione nell'emettere un avviso per il 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 falsa. UnwrappedLineParser.cpp 1635
Frammento N30: Ritorno 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:
Questo è un errore o una tecnica specifica intesa a spiegare qualcosa ai programmatori che leggono il codice. Questo disegno non mi spiega nulla e sembra molto sospetto. È meglio non scrivere così :).
Stanco? Poi è il momento di preparare il tè o il caffè.
Difetti identificati dalla nuova diagnostica
Penso che 30 attivazioni della vecchia diagnostica siano sufficienti. Vediamo ora quali cose interessanti si possono trovare con la nuova diagnostica apparsa successivamente nell'analizzatore
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:
Come puoi vedere, entrambi i rami dell'operatore if termina con una chiamata all'operatore ritorno. Di conseguenza, il contenitore CtorDtorsByPriority non verrà mai cancellato.
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(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}
Avviso PVS-Studio: V779 [CWE-561] Rilevato codice irraggiungibile. È possibile che sia presente un errore. LLParser.cpp 835
Situazione interessante. Diamo prima un'occhiata a questo posto:
return ParseTypeIdEntry(SummaryID);
break;
A prima vista, sembra che non ci siano errori qui. Sembra l'operatore rompere ce n'è uno in più qui e puoi semplicemente eliminarlo. Tuttavia, non tutto è così semplice.
L'analizzatore emette un avviso sulle righe:
Lex.setIgnoreColonInIdentifiers(false);
return false;
E in effetti, questo codice è irraggiungibile. Tutti i casi dentro interruttore termina con una chiamata dell'operatore ritorno. E ora sono solo senza senso rompere non sembra così innocuo! Forse uno dei rami dovrebbe finire con rompere, non su ritorno?
Frammento N33: reset casuale dei bit alti
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);
....
}
Avviso PVS-Studio:
Si prega di notare che la funzione getStubAlignment tipo di restituzione unsigned. Calcoliamo il valore dell'espressione, supponendo che la funzione restituisca il valore 8:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Ora notiamo che la variabile Dimensione dati ha un tipo senza segno a 64 bit. Risulta che quando si esegue l'operazione DataSize e 0xFFFFFFF8u, tutti i trentadue bit di ordine superiore verranno reimpostati su zero. Molto probabilmente, questo non è ciò che il programmatore voleva. Sospetto che volesse calcolare: DataSize & 0xFFFFFFFFFFFFFFFF8u.
Per correggere l'errore, dovresti scrivere questo:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
O così:
DataSize &= ~(getStubAlignment() - 1ULL);
Frammento N34: cast di tipo esplicito non riuscito
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);
....
}
Avviso PVS-Studio:
Il cast esplicito del tipo viene utilizzato per evitare l'overflow durante la moltiplicazione delle variabili di tipo int. Tuttavia, il casting esplicito del tipo in questo caso non protegge dall'overflow. Innanzitutto le variabili verranno moltiplicate e solo successivamente il risultato a 32 bit della moltiplicazione verrà espanso al tipo
Frammento N35: Copia-Incolla non riuscita
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;
}
....
}
Questa nuova interessante diagnostica identifica le situazioni in cui un pezzo di codice è stato copiato e alcuni nomi in esso contenuti hanno iniziato a essere modificati, ma in un punto non è stato corretto.
Tieni presente che nel secondo blocco sono cambiati Op0 su Op1. Ma in un posto non l’hanno risolto. Molto probabilmente andrebbe scritto così:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Frammento N36: Confusione variabile
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:
È molto pericoloso dare agli argomenti della funzione gli stessi nomi dei membri della classe. È molto facile confondersi. Abbiamo davanti a noi un caso del genere. Questa espressione non ha senso:
Mode &= Mask;
L'argomento della funzione cambia. È tutto. Questo argomento non è più utilizzato. Molto probabilmente avresti dovuto scriverlo così:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Frammento N37: Confusione variabile
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;
}
Avvertenza PVS-Studio: V1001 [CWE-563] La variabile 'Dimensione' è assegnata ma non viene utilizzata alla fine della funzione. Oggetto.cpp 424
La situazione è simile alla precedente. Dovrebbe essere scritto:
this->Size += this->EntrySize;
Frammento N38-N47: Si sono dimenticati di controllare l'indice
In precedenza, abbiamo esaminato esempi di attivazione diagnostica
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()); // <=
....
}
Avviso PVS-Studio: V1004 [CWE-476] Il puntatore 'Ptr' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controllare le righe: 729, 738. TargetTransformInfoImpl.h 738
Переменная Ptr può essere uguale nullptr, come risulta dal controllo:
if (Ptr != nullptr)
Tuttavia, di seguito questo puntatore viene dereferenziato senza controllo preliminare:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Consideriamo un altro caso simile.
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(); // <=
....
}
Avviso PVS-Studio: V1004 [CWE-476] Il puntatore 'FD' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controlla le righe: 3228, 3231. CGDebugInfo.cpp 3231
Prestare attenzione al cartello FD. Sono sicuro che il problema è chiaramente visibile e non è necessaria alcuna spiegazione speciale.
E ancora:
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()); // <=
....
}
Avviso PVS-Studio: V1004 [CWE-476] Il puntatore 'PtrTy' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controlla le righe: 960, 965. InterleavedLoadCombinePass.cpp 965
Come proteggersi da tali errori? Sii più attento alla revisione del codice e utilizza l'analizzatore statico PVS-Studio per controllare regolarmente il tuo codice.
È inutile citare altri frammenti di codice con errori di questo tipo. Lascerò solo un elenco di avvertenze nell'articolo:
- V1004 [CWE-476] Il puntatore 'Expr' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controlla le righe: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Il puntatore 'PI' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controllare le righe: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Il puntatore 'StatepointCall' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controllare le righe: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Il puntatore 'RV' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a nullptr. Controllare le righe: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Il puntatore 'CalleeFn' è stato utilizzato in modo non sicuro dopo essere stato verificato rispetto a 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 rispetto a nullptr. Controlla le righe: 1819, 1824. Driver.cpp 1824
Frammento N48-N60: non critico, ma difettoso (possibile perdita di memoria)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
Avviso PVS-Studio:
Per aggiungere un elemento alla fine di un contenitore come std::vettore > non puoi semplicemente scrivere xxx.push_back(nuova X), poiché non esiste alcuna conversione implicita da X* в std::unique_ptr.
Una soluzione comune è scrivere xxx.emplace_back(nuova X)poiché compila: metodo posto_indietro costruisce un elemento direttamente dai suoi argomenti e può quindi utilizzare costruttori espliciti.
Non è sicuro. Se il vettore è pieno, la memoria viene riallocata. L'operazione di riallocazione della memoria potrebbe non riuscire, generando un'eccezione std::bad_alloc. In questo caso il puntatore andrà perso e l'oggetto creato non verrà mai cancellato.
Una soluzione sicura è creare unico_ptrche possiederà il puntatore prima che il vettore tenti di riallocare la memoria:
xxx.push_back(std::unique_ptr<X>(new X))
A partire da C++14, puoi usare 'std::make_unique':
xxx.push_back(std::make_unique<X>())
Questo tipo di difetto non è critico per LLVM. Se non è possibile allocare memoria, il compilatore si fermerà semplicemente. Tuttavia, per le applicazioni con long
Pertanto, sebbene questo codice non rappresenti una minaccia pratica per LLVM, ho trovato utile parlare di questo modello di errore e del fatto che l'analizzatore PVS-Studio ha imparato a identificarlo.
Altri avvisi di questo tipo:
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Passes' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. PassManager.h546
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'AAs' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. AliasAnalysis.h 324
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Entries' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'AllEdges' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. CFGMST.h268
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'VMaps' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Records' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. FDRLogBuilder.h 30
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'PendingSubmodules' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. ModuleMap.cpp 810
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Oggetti' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. DebugMap.cpp 88
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Strategies' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 685
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 686
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 688
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 689
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 690
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 691
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 692
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 693
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Modifiers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. llvm-stress.cpp 694
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Operandi' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Stash' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Un puntatore senza proprietario viene aggiunto al contenitore 'Matchers' tramite il metodo 'emplace_back'. In caso di eccezione si verificherà una perdita di memoria. GlobalISelEmitter.cpp 2702
conclusione
Ho emesso 60 avvisi in totale e poi ho smesso. Ci sono altri difetti che l'analizzatore PVS-Studio rileva in LLVM? Sì. Tuttavia, mentre stavo scrivendo frammenti di codice per l'articolo, era tarda sera, o meglio addirittura notte, e ho deciso che era ora di concludere la giornata.
Spero che lo abbiate trovato interessante e che vogliate provare l'analizzatore PVS-Studio.
Puoi scaricare l'analizzatore e ottenere la chiave del dragamine su
Ancora più importante, utilizzare regolarmente l'analisi statica. Controlli una tantum, da noi condotti al fine di divulgare la metodologia dell'analisi statica e PVS-Studio non sono uno scenario normale.
Buona fortuna per migliorare la qualità e l'affidabilità del tuo codice!
Se vuoi condividere questo articolo con un pubblico di lingua inglese, utilizza il link di traduzione: Andrey Karpov.
Fonte: habr.com