Seit der letzten Codeüberprüfung des LLVM-Projekts mit unserem PVS-Studio-Analysator sind mehr als zwei Jahre vergangen. Stellen wir sicher, dass der PVS-Studio-Analysator weiterhin ein führendes Tool zur Identifizierung von Fehlern und potenziellen Schwachstellen ist. Dazu werden wir die Version LLVM 8.0.0 überprüfen und neue Fehler finden.
Zu schreibender Artikel
Ehrlich gesagt wollte ich diesen Artikel nicht schreiben. Es ist nicht interessant, über ein Projekt zu schreiben, das wir bereits mehrmals überprüft haben (
Jedes Mal, wenn eine neue Version von LLVM veröffentlicht oder aktualisiert wird
Schauen Sie, die neue Version von Clang Static Analyzer hat gelernt, neue Fehler zu finden! Es scheint mir, dass die Relevanz der Verwendung von PVS-Studio abnimmt. Clang findet mehr Fehler als zuvor und holt mit den Fähigkeiten von PVS-Studio auf. Was denkst du darüber?
Darauf möchte ich immer etwas antworten wie:
Auch wir bleiben nicht untätig! Wir haben die Fähigkeiten des PVS-Studio-Analysators deutlich verbessert. Also keine Sorge, wir führen weiter wie bisher.
Leider ist das eine schlechte Antwort. Es sind keine Beweise darin enthalten. Und deshalb schreibe ich jetzt diesen Artikel. Daher wurde das LLVM-Projekt noch einmal überprüft und es wurden diverse Fehler darin gefunden. Ich werde jetzt diejenigen demonstrieren, die mir interessant erschienen. Clang Static Analyzer kann diese Fehler nicht finden (oder es ist äußerst umständlich, dies mit seiner Hilfe zu tun). Aber wir können. Außerdem habe ich alle diese Fehler an einem Abend gefunden und aufgeschrieben.
Doch das Schreiben des Artikels dauerte mehrere Wochen. Ich konnte mich einfach nicht dazu durchringen, das alles in Text zu packen :).
Wenn Sie übrigens daran interessiert sind, welche Technologien im PVS-Studio-Analysator zur Identifizierung von Fehlern und potenziellen Schwachstellen verwendet werden, empfehle ich Ihnen, sich damit vertraut zu machen
Neue und alte Diagnostik
Wie bereits erwähnt, wurde das LLVM-Projekt vor etwa zwei Jahren erneut überprüft und die gefundenen Fehler behoben. Nun wird dieser Artikel eine neue Reihe von Fehlern präsentieren. Warum wurden neue Fehler gefunden? Dafür gibt es 3 Gründe:
- Das LLVM-Projekt entwickelt sich weiter, alter Code wird geändert und neuer Code hinzugefügt. Natürlich gibt es neue Fehler im geänderten und geschriebenen Code. Dies zeigt deutlich, dass die statische Analyse regelmäßig und nicht gelegentlich verwendet werden sollte. Unsere Artikel zeigen gut die Fähigkeiten des PVS-Studio-Analysators, aber das hat nichts mit der Verbesserung der Codequalität und der Reduzierung der Kosten für die Fehlerbehebung zu tun. Verwenden Sie regelmäßig einen statischen Code-Analysator!
- Wir finalisieren und verbessern die bestehende Diagnostik. Daher kann der Analysator Fehler identifizieren, die ihm bei früheren Scans nicht aufgefallen sind.
- In PVS-Studio sind neue Diagnosen aufgetaucht, die es vor 2 Jahren noch nicht gab. Ich habe beschlossen, sie in einem separaten Abschnitt hervorzuheben, um die Entwicklung von PVS-Studio deutlich zu zeigen.
Durch die Diagnose festgestellte Mängel, die vor 2 Jahren bestanden
Fragment N1: Kopieren und Einfügen
static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
if (Name == "addcarryx.u32" || // Added in 8.0
....
Name == "avx512.mask.cvtps2pd.128" || // Added in 7.0
Name == "avx512.mask.cvtps2pd.256" || // Added in 7.0
Name == "avx512.cvtusi2sd" || // Added in 7.0
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name.startswith("avx512.mask.permvar.") || // Added in 7.0 // <=
Name == "sse2.pmulu.dq" || // Added in 7.0
Name == "sse41.pmuldq" || // Added in 7.0
Name == "avx2.pmulu.dq" || // Added in 7.0
....
}
PVS-Studio-Warnung:
Es wird doppelt überprüft, ob der Name mit der Teilzeichenfolge „avx512.mask.permvar“ beginnt. Bei der zweiten Kontrolle wollte man offensichtlich etwas anderes schreiben, vergaß aber, den kopierten Text zu korrigieren.
Fragment N2: Tippfehler
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;
....
}
Warnung PVS-Studio: V501 Es gibt identische Unterausdrücke „CXNameRange_WantQualifier“ links und rechts vom „|“ Operator. CIndex.cpp 7245
Aufgrund eines Tippfehlers wird dieselbe benannte Konstante zweimal verwendet CXNameRange_WantQualifier.
Fragment N3: Verwirrung mit der Operatorpriorität
int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
....
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
return 0;
....
}
PVS-Studio-Warnung:
Meiner Meinung nach ist das ein sehr schöner Fehler. Ja, ich weiß, dass ich seltsame Vorstellungen von Schönheit habe :).
Nun, laut
(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0
Aus praktischer Sicht ist eine solche Bedingung nicht sinnvoll, da sie reduziert werden kann auf:
(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())
Das ist ein klarer Fehler. Höchstwahrscheinlich wollten sie 0/1 mit einer Variablen vergleichen Index. Um den Code zu korrigieren, müssen Sie Klammern um den ternären Operator hinzufügen:
if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))
Der ternäre Operator ist übrigens sehr gefährlich und provoziert logische Fehler. Seien Sie sehr vorsichtig damit und seien Sie nicht gierig mit Klammern. Ich habe mir dieses Thema genauer angesehen
Fragment N4, N5: Nullzeiger
Init *TGParser::ParseValue(Record *CurRec, RecTy *ItemType, IDParseMode Mode) {
....
TypedInit *LHS = dyn_cast<TypedInit>(Result);
....
LHS = dyn_cast<TypedInit>(
UnOpInit::get(UnOpInit::CAST, LHS, StringRecTy::get())
->Fold(CurRec));
if (!LHS) {
Error(PasteLoc, Twine("can't cast '") + LHS->getAsString() +
"' to string");
return nullptr;
}
....
}
PVS-Studio-Warnung:
Wenn der Zeiger LHS null ist, sollte eine Warnung ausgegeben werden. Stattdessen wird jedoch derselbe Nullzeiger dereferenziert: LHS->getAsString().
Dies ist eine sehr typische Situation, wenn ein Fehler in einem Fehlerbehandler verborgen ist, da ihn niemand testet. Statische Analysatoren überprüfen den gesamten erreichbaren Code, unabhängig davon, wie oft er verwendet wird. Dies ist ein sehr gutes Beispiel dafür, wie die statische Analyse andere Test- und Fehlerschutztechniken ergänzt.
Ähnlicher Fehler bei der Zeigerverarbeitung RHS im folgenden Code zulässig: V522 [CWE-476] Es kann zu einer Dereferenzierung des Nullzeigers „RHS“ kommen. TGParser.cpp 2186
Fragment N6: Verwendung des Zeigers nach dem Verschieben
static Expected<bool>
ExtractBlocks(....)
{
....
std::unique_ptr<Module> ProgClone = CloneModule(BD.getProgram(), VMap);
....
BD.setNewProgram(std::move(ProgClone)); // <=
MiscompiledFunctions.clear();
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first); // <=
assert(NewF && "Function not found??");
MiscompiledFunctions.push_back(NewF);
}
....
}
PVS-Studio-Warnung: V522 [CWE-476] Es kann zu einer Dereferenzierung des Nullzeigers „ProgClone“ kommen. Fehlkompilierung.cpp 601
Zu Beginn ein kluger Hinweis ProgClone hört auf, das Objekt zu besitzen:
BD.setNewProgram(std::move(ProgClone));
Tatsächlich jetzt ProgClone ist ein Nullzeiger. Daher sollte direkt darunter eine Nullzeiger-Dereferenzierung erfolgen:
Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);
Aber in Wirklichkeit wird das nicht passieren! Beachten Sie, dass die Schleife nicht tatsächlich ausgeführt wird.
Am Anfang des Containers Falsch kompilierte Funktionen gelöscht:
MiscompiledFunctions.clear();
Als nächstes wird die Größe dieses Containers in der Schleifenbedingung verwendet:
for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {
Es ist leicht zu erkennen, dass die Schleife nicht startet. Ich denke, das ist auch ein Fehler und der Code sollte anders geschrieben werden.
Es scheint, dass wir auf diese berühmte Fehlerparität gestoßen sind! Ein Fehler verdeckt einen anderen :).
Fragment N7: Verwendung des Zeigers nach dem Verschieben
static Expected<bool> TestOptimizer(BugDriver &BD, std::unique_ptr<Module> Test,
std::unique_ptr<Module> Safe) {
outs() << " Optimizing functions being tested: ";
std::unique_ptr<Module> Optimized =
BD.runPassesOn(Test.get(), BD.getPassesToRun());
if (!Optimized) {
errs() << " Error running this sequence of passes"
<< " on the input program!n";
BD.setNewProgram(std::move(Test)); // <=
BD.EmitProgressBitcode(*Test, "pass-error", false); // <=
if (Error E = BD.debugOptimizerCrash())
return std::move(E);
return false;
}
....
}
PVS-Studio-Warnung: V522 [CWE-476] Es kann zu einer Dereferenzierung des Nullzeigers „Test“ kommen. Fehlkompilierung.cpp 709
Wieder die gleiche Situation. Zunächst wird der Inhalt des Objekts verschoben und dann so verwendet, als ob nichts passiert wäre. Ich sehe diese Situation immer häufiger im Programmcode, nachdem die Bewegungssemantik in C++ aufgetaucht ist. Deshalb liebe ich die Sprache C++! Es gibt immer neue Möglichkeiten, sich das eigene Bein abzuschießen. Der PVS-Studio-Analysator wird immer funktionieren :).
Fragment N8: Nullzeiger
void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
uint32_t TypeId = Symbol.getTypeId();
auto Type = Symbol.getSession().getSymbolById(TypeId);
if (Type)
Printer << "<unknown-type>";
else
Type->dump(*this);
}
PVS-Studio-Warnung: V522 [CWE-476] Es kann zu einer Dereferenzierung des Nullzeigers „Typ“ kommen. PrettyFunctionDumper.cpp 233
Neben Fehlerbehandlern werden auch Debugging-Ausdruckfunktionen in der Regel nicht getestet. Wir haben gerade einen solchen Fall vor uns. Die Funktion wartet auf den Benutzer, der, anstatt seine Probleme zu lösen, gezwungen wird, sie zu beheben.
Richtig:
if (Type)
Type->dump(*this);
else
Printer << "<unknown-type>";
Fragment N9: Nullzeiger
void SearchableTableEmitter::collectTableEntries(
GenericTable &Table, const std::vector<Record *> &Items) {
....
RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
if (!Ty) // <=
PrintFatalError(Twine("Field '") + Field.Name + "' of table '" +
Table.Name + "' has incompatible type: " +
Ty->getAsString() + " vs. " + // <=
TI->getType()->getAsString());
....
}
PVS-Studio-Warnung: V522 [CWE-476] Es kann zu einer Dereferenzierung des Nullzeigers „Ty“ kommen. SearchableTableEmitter.cpp 614
Ich denke, alles ist klar und bedarf keiner Erklärung.
Fragment N10: Tippfehler
bool FormatTokenLexer::tryMergeCSharpNullConditionals() {
....
auto &Identifier = *(Tokens.end() - 2);
auto &Question = *(Tokens.end() - 1);
....
Identifier->ColumnWidth += Question->ColumnWidth;
Identifier->Type = Identifier->Type; // <=
Tokens.erase(Tokens.end() - 1);
return true;
}
PVS-Studio-Warnung:
Es macht keinen Sinn, eine Variable sich selbst zuzuweisen. Höchstwahrscheinlich wollten sie schreiben:
Identifier->Type = Question->Type;
Fragment N11: Verdächtiger Bruch
void SystemZOperand::print(raw_ostream &OS) const {
switch (Kind) {
break;
case KindToken:
OS << "Token:" << getToken();
break;
case KindReg:
OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
break;
....
}
PVS-Studio-Warnung:
Am Anfang steht ein sehr verdächtiger Betreiber brechen. Haben Sie vergessen, hier noch etwas zu schreiben?
Fragment N12: Überprüfung eines Zeigers nach der Dereferenzierung
InlineCost AMDGPUInliner::getInlineCost(CallSite CS) {
Function *Callee = CS.getCalledFunction();
Function *Caller = CS.getCaller();
TargetTransformInfo &TTI = TTIWP->getTTI(*Callee);
if (!Callee || Callee->isDeclaration())
return llvm::InlineCost::getNever("undefined callee");
....
}
PVS-Studio-Warnung:
Index Angerufene am Anfang wird zum Zeitpunkt des Funktionsaufrufs dereferenziert getTTI.
Und dann stellt sich heraus, dass dieser Zeiger auf Gleichheit überprüft werden sollte nullptr:
if (!Callee || Callee->isDeclaration())
Aber es ist zu spät…
Fragment N13 – N...: Überprüfung eines Zeigers nach der Dereferenzierung
Die im vorherigen Codefragment besprochene Situation ist nicht einzigartig. Es erscheint hier:
static Value *optimizeDoubleFP(CallInst *CI, IRBuilder<> &B,
bool isBinary, bool isPrecise = false) {
....
Function *CalleeFn = CI->getCalledFunction();
StringRef CalleeNm = CalleeFn->getName(); // <=
AttributeList CalleeAt = CalleeFn->getAttributes();
if (CalleeFn && !CalleeFn->isIntrinsic()) { // <=
....
}
PVS-Studio-Warnung: V595 [CWE-476] Der „CalleeFn“-Zeiger wurde verwendet, bevor er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 1079, 1081. SimplifyLibCalls.cpp 1079
Und hier:
void Sema::InstantiateAttrs(const MultiLevelTemplateArgumentList &TemplateArgs,
const Decl *Tmpl, Decl *New,
LateInstantiatedAttrVec *LateAttrs,
LocalInstantiationScope *OuterMostScope) {
....
NamedDecl *ND = dyn_cast<NamedDecl>(New);
CXXRecordDecl *ThisContext =
dyn_cast_or_null<CXXRecordDecl>(ND->getDeclContext()); // <=
CXXThisScopeRAII ThisScope(*this, ThisContext, Qualifiers(),
ND && ND->isCXXInstanceMember()); // <=
....
}
PVS-Studio-Warnung: V595 [CWE-476] Der „ND“-Zeiger wurde verwendet, bevor er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 532, 534. SemaTemplateInstantiateDecl.cpp 532
Und hier:
- V595 [CWE-476] Der „U“-Zeiger wurde verwendet, bevor er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 404, 407. DWARFormValue.cpp 404
- V595 [CWE-476] Der „ND“-Zeiger wurde verwendet, bevor er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 2149, 2151. SemaTemplateInstantiate.cpp 2149
Und dann verlor ich das Interesse daran, die Warnungen mit der Nummer V595 zu studieren. Daher weiß ich nicht, ob es außer den hier aufgeführten noch weitere ähnliche Fehler gibt. Höchstwahrscheinlich gibt es das.
Fragment N17, N18: Verdächtige Verschiebung
static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize,
uint64_t &Encoding) {
....
unsigned Size = RegSize;
....
uint64_t NImms = ~(Size-1) << 1;
....
}
PVS-Studio-Warnung:
Möglicherweise handelt es sich nicht um einen Fehler, und der Code funktioniert genau wie vorgesehen. Aber dies ist eindeutig ein sehr verdächtiger Ort und muss überprüft werden.
Sagen wir die Variable Größe ist gleich 16, und dann hat der Autor des Codes geplant, ihn in eine Variable zu bekommen NImms Wert:
1111111111111111111111111111111111111111111111111111111111100000
In Wirklichkeit wird das Ergebnis jedoch sein:
0000000000000000000000000000000011111111111111111111111111100000
Tatsache ist, dass alle Berechnungen mit dem vorzeichenlosen 32-Bit-Typ erfolgen. Und nur dann wird dieser 32-Bit-Typ ohne Vorzeichen implizit erweitert uint64_t. In diesem Fall sind die höchstwertigen Bits Null.
Sie können die Situation wie folgt beheben:
uint64_t NImms = ~static_cast<uint64_t>(Size-1) << 1;
Ähnliche Situation: V629 [CWE-190] Erwägen Sie die Überprüfung des Ausdrucks „Immr << 6“. Bitverschiebung des 32-Bit-Wertes mit anschließender Erweiterung auf den 64-Bit-Typ. AArch64AddressingModes.h 269
Fragment N19: Fehlendes Schlüsselwort sonst?
void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) {
....
if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) {
// VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token.
// Skip it.
continue;
} if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <=
Op.addRegWithFPInputModsOperands(Inst, 2);
} else if (Op.isDPPCtrl()) {
Op.addImmOperands(Inst, 1);
} else if (Op.isImm()) {
// Handle optional arguments
OptionalIdx[Op.getImmTy()] = I;
} else {
llvm_unreachable("Invalid operand type");
}
....
}
PVS-Studio-Warnung:
Hier liegt kein Fehler vor. Seit dem damaligen Block des Ersten if endet mit fortsetzen, dann spielt es keine Rolle, es gibt ein Schlüsselwort sonst oder nicht. In beiden Fällen funktioniert der Code gleich. Immer noch vermisst sonst macht den Code unklarer und gefährlicher. Wenn in der Zukunft fortsetzen verschwindet, beginnt der Code völlig anders zu funktionieren. Meiner Meinung nach ist es besser, hinzuzufügen sonst.
Fragment N20: Vier Tippfehler desselben Typs
LLVM_DUMP_METHOD void Symbol::dump(raw_ostream &OS) const {
std::string Result;
if (isUndefined())
Result += "(undef) ";
if (isWeakDefined())
Result += "(weak-def) ";
if (isWeakReferenced())
Result += "(weak-ref) ";
if (isThreadLocalValue())
Result += "(tlv) ";
switch (Kind) {
case SymbolKind::GlobalSymbol:
Result + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClass:
Result + "(ObjC Class) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCClassEHType:
Result + "(ObjC Class EH) " + Name.str(); // <=
break;
case SymbolKind::ObjectiveCInstanceVariable:
Result + "(ObjC IVar) " + Name.str(); // <=
break;
}
OS << Result;
}
PVS-Studio-Warnungen:
- V655 [CWE-480] Die Zeichenfolgen wurden verkettet, werden aber nicht verwendet. Erwägen Sie, den Ausdruck „Result + Name.str()“ zu überprüfen. Symbol.cpp 32
- V655 [CWE-480] Die Zeichenfolgen wurden verkettet, werden aber nicht verwendet. Erwägen Sie, den Ausdruck „Result + „(ObjC Class)“ + Name.str()“ zu überprüfen. Symbol.cpp 35
- V655 [CWE-480] Die Zeichenfolgen wurden verkettet, werden aber nicht verwendet. Erwägen Sie, den Ausdruck „Result + „(ObjC Class EH) „ + Name.str()“ zu überprüfen. Symbol.cpp 38
- V655 [CWE-480] Die Zeichenfolgen wurden verkettet, werden aber nicht verwendet. Erwägen Sie, den Ausdruck „Result + „(ObjC IVar)“ + Name.str()“ zu überprüfen. Symbol.cpp 41
Aus Versehen wird der Operator + anstelle des Operators += verwendet. Das Ergebnis sind bedeutungslose Designs.
Fragment N21: Undefiniertes Verhalten
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();
}
}
}
Versuchen Sie, den gefährlichen Code selbst zu finden. Und dies ist ein Bild, um die Aufmerksamkeit abzulenken, um nicht sofort auf die Antwort zu blicken:
PVS-Studio-Warnung:
Problemlinie:
FeaturesMap[Op] = FeaturesMap.size();
Wenn-Element Op nicht gefunden wird, wird ein neues Element in der Karte erstellt und die Anzahl der Elemente in dieser Karte wird dort geschrieben. Es ist nur unbekannt, ob die Funktion aufgerufen wird Größe vor oder nach dem Hinzufügen eines neuen Elements.
Fragment N22-N24: Wiederholte Aufgaben
Error MachOObjectFile::checkSymbolTable() const {
....
} else {
MachO::nlist STE = getSymbolTableEntry(SymDRI);
NType = STE.n_type; // <=
NType = STE.n_type; // <=
NSect = STE.n_sect;
NDesc = STE.n_desc;
NStrx = STE.n_strx;
NValue = STE.n_value;
}
....
}
PVS-Studio-Warnung:
Ich glaube nicht, dass hier ein echter Fehler vorliegt. Nur eine unnötige wiederholte Aufgabe. Aber immer noch ein Fehler.
In ähnlicher Weise
- V519 [CWE-563] Der Variable „B.NDesc“ werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 1488, 1489. llvm-nm.cpp 1489
- V519 [CWE-563] Der Variable werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 59, 61. coff2yaml.cpp 61
Fragment N25-N27: Weitere Neuzuweisungen
Schauen wir uns nun eine etwas andere Version der Neuzuweisung an.
bool Vectorizer::vectorizeLoadChain(
ArrayRef<Instruction *> Chain,
SmallPtrSet<Instruction *, 16> *InstructionsProcessed) {
....
unsigned Alignment = getAlignment(L0);
....
unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
StackAdjustedAlignment,
DL, L0, nullptr, &DT);
if (NewAlign != 0)
Alignment = NewAlign;
Alignment = NewAlign;
....
}
PVS-Studio-Warnung: V519 [CWE-563] Der Variable „Alignment“ werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Überprüfen Sie die Zeilen: 1158, 1160. LoadStoreVectorizer.cpp 1160
Das ist ein sehr seltsamer Code, der offenbar einen logischen Fehler enthält. Zu Beginn variabel Ausrichtung Abhängig von der Bedingung wird ein Wert zugewiesen. Und dann erfolgt die Zuordnung erneut, jedoch ohne Prüfung.
Ähnliche Situationen sind hier zu sehen:
- V519 [CWE-563] Der Variable „Effekte“ werden zweimal hintereinander Werte zugewiesen. Vielleicht ist das ein Fehler. Prüfzeilen: 152, 165. WebAssemblyRegStackify.cpp 165
- V519 [CWE-563] Der Variable „ExpectNoDerefChunk“ werden zweimal nacheinander Werte zugewiesen. Vielleicht ist das ein Fehler. Prüfzeilen: 4970, 4973. SemaType.cpp 4973
Fragment N28: Immer wahrer Zustand
static int readPrefixes(struct InternalInstruction* insn) {
....
uint8_t byte = 0;
uint8_t nextByte;
....
if (byte == 0xf3 && (nextByte == 0x88 || nextByte == 0x89 ||
nextByte == 0xc6 || nextByte == 0xc7)) {
insn->xAcquireRelease = true;
if (nextByte != 0x90) // PAUSE instruction support // <=
break;
}
....
}
PVS-Studio-Warnung:
Eine Überprüfung macht keinen Sinn. Variable nextByte immer nicht gleich dem Wert 0x90, was sich aus der vorherigen Prüfung ergibt. Das ist eine Art logischer Fehler.
Fragment N29 – N...: Immer wahr/falsch-Bedingungen
Der Analysator gibt viele Warnungen aus, dass der gesamte Zustand (
static DecodeStatus DecodeGPRPairRegisterClass(MCInst &Inst, unsigned RegNo,
uint64_t Address, const void *Decoder) {
DecodeStatus S = MCDisassembler::Success;
if (RegNo > 13)
return MCDisassembler::Fail;
if ((RegNo & 1) || RegNo == 0xe)
S = MCDisassembler::SoftFail;
....
}
PVS-Studio-Warnung:
Die Konstante 0xE ist der Wert 14 in Dezimalzahl. Untersuchung RegNo == 0xe macht keinen Sinn, denn wenn RegNr > 13, dann schließt die Funktion ihre Ausführung ab.
Es gab viele weitere Warnungen mit den IDs V547 und V560, aber wie bei
Ich gebe Ihnen ein Beispiel dafür, warum es langweilig ist, diese Auslöser zu studieren. Der Analysator hat völlig Recht, wenn er für den folgenden Code eine Warnung ausgibt. Aber das ist kein Fehler.
bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons,
tok::TokenKind ClosingBraceKind) {
bool HasError = false;
....
HasError = true;
if (!ContinueOnSemicolons)
return !HasError;
....
}
PVS-Studio-Warnung: V547 [CWE-570] Ausdruck „!HasError“ ist immer falsch. UnwrappedLineParser.cpp 1635
Fragment N30: Verdächtige Rückkehr
static bool
isImplicitlyDef(MachineRegisterInfo &MRI, unsigned Reg) {
for (MachineRegisterInfo::def_instr_iterator It = MRI.def_instr_begin(Reg),
E = MRI.def_instr_end(); It != E; ++It) {
return (*It).isImplicitDef();
}
....
}
PVS-Studio-Warnung:
Hierbei handelt es sich entweder um einen Fehler oder um eine bestimmte Technik, die Programmierern, die den Code lesen, etwas erklären soll. Dieses Design erklärt mir nichts und sieht sehr verdächtig aus. Es ist besser, nicht so zu schreiben :).
Müde? Dann ist es Zeit, Tee oder Kaffee zuzubereiten.
Durch neue Diagnose festgestellte Mängel
Ich denke, 30 Aktivierungen der alten Diagnose reichen aus. Sehen wir uns nun an, welche interessanten Dinge sich mit den neuen Diagnosen finden lassen, die danach im Analysator erschienen sind
Fragment N31: Nicht erreichbarer Code
Error CtorDtorRunner::run() {
....
if (auto CtorDtorMap =
ES.lookup(JITDylibSearchList({{&JD, true}}), std::move(Names),
NoDependenciesToRegister, true))
{
....
return Error::success();
} else
return CtorDtorMap.takeError();
CtorDtorsByPriority.clear();
return Error::success();
}
PVS-Studio-Warnung:
Wie Sie sehen, sind beide Zweige des Betreibers if endet mit einem Anruf beim Operator Rückkehr. Dementsprechend der Behälter CtorDtorsByPriority wird nie gelöscht.
Fragment N32: Nicht erreichbarer Code
bool LLParser::ParseSummaryEntry() {
....
switch (Lex.getKind()) {
case lltok::kw_gv:
return ParseGVEntry(SummaryID);
case lltok::kw_module:
return ParseModuleEntry(SummaryID);
case lltok::kw_typeid:
return ParseTypeIdEntry(SummaryID); // <=
break; // <=
default:
return Error(Lex.getLoc(), "unexpected summary kind");
}
Lex.setIgnoreColonInIdentifiers(false); // <=
return false;
}
PVS-Studio-Warnung: V779 [CWE-561] Nicht erreichbarer Code erkannt. Möglicherweise liegt ein Fehler vor. LLParser.cpp 835
Interessante Situation. Schauen wir uns zunächst diesen Ort an:
return ParseTypeIdEntry(SummaryID);
break;
Auf den ersten Blick scheint hier kein Fehler vorzuliegen. Es sieht aus wie der Betreiber brechen Hier gibt es noch ein weiteres, das Sie einfach löschen können. Allerdings ist nicht alles so einfach.
Der Analysator gibt eine Warnung in den Zeilen aus:
Lex.setIgnoreColonInIdentifiers(false);
return false;
Und tatsächlich ist dieser Code nicht erreichbar. Alle Fälle in wechseln endet mit einem Anruf des Operators Rückkehr. Und jetzt allein sinnlos brechen sieht nicht so harmlos aus! Vielleicht sollte einer der Zweige mit enden brechennicht auf Rückkehr?
Fragment N33: Zufälliges Zurücksetzen der High-Bits
unsigned getStubAlignment() override {
if (Arch == Triple::systemz)
return 8;
else
return 1;
}
Expected<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
const SectionRef &Section,
bool IsCode) {
....
uint64_t DataSize = Section.getSize();
....
if (StubBufSize > 0)
DataSize &= ~(getStubAlignment() - 1);
....
}
PVS-Studio-Warnung:
Bitte beachten Sie, dass die Funktion getStubAlignment Gibt den Typ zurück ohne Vorzeichen. Berechnen wir den Wert des Ausdrucks unter der Annahme, dass die Funktion den Wert 8 zurückgibt:
~(getStubAlignment() - 1)
~(8u-1)
0xFFFFFFFF8u
Beachten Sie nun, dass die Variable Datengröße hat einen 64-Bit-Typ ohne Vorzeichen. Es stellt sich heraus, dass bei der Ausführung der Operation „DataSize & 0xFFFFFFF8u“ alle zweiunddreißig höherwertigen Bits auf Null zurückgesetzt werden. Höchstwahrscheinlich ist dies nicht das, was der Programmierer wollte. Ich vermute, dass er berechnen wollte: DataSize & 0xFFFFFFFFFFFFFFF8u.
Um den Fehler zu beheben, sollten Sie Folgendes schreiben:
DataSize &= ~(static_cast<uint64_t>(getStubAlignment()) - 1);
Oder so:
DataSize &= ~(getStubAlignment() - 1ULL);
Fragment N34: Explizite Typumwandlung fehlgeschlagen
template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
SmallVectorImpl<T> &ScaledMask) {
assert(0 < Scale && "Unexpected scaling factor");
int NumElts = Mask.size();
ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
....
}
PVS-Studio-Warnung:
Die explizite Typumwandlung wird verwendet, um einen Überlauf beim Multiplizieren von Typvariablen zu vermeiden int. Eine explizite Typumwandlung schützt hier jedoch nicht vor einem Überlauf. Zunächst werden die Variablen multipliziert und erst dann das 32-Bit-Ergebnis der Multiplikation zum Typ erweitert
Fragment N35: Fehler beim Kopieren und Einfügen
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;
}
....
}
Diese neue interessante Diagnose identifiziert Situationen, in denen ein Teil des Codes kopiert wurde und einige Namen darin geändert wurden, an einer Stelle jedoch nicht korrigiert wurde.
Bitte beachten Sie, dass sie sich im zweiten Block geändert haben Op0 auf Op1. Aber an einer Stelle haben sie es nicht repariert. Höchstwahrscheinlich hätte es so geschrieben werden sollen:
if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
return &I;
}
Fragment N36: Variablenverwirrung
struct Status {
unsigned Mask;
unsigned Mode;
Status() : Mask(0), Mode(0){};
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
Mode &= Mask;
};
....
};
PVS-Studio-Warnung:
Es ist sehr gefährlich, Funktionsargumenten dieselben Namen zu geben wie Klassenmitgliedern. Es ist sehr leicht, verwirrt zu werden. Wir haben gerade einen solchen Fall vor uns. Dieser Ausdruck ergibt keinen Sinn:
Mode &= Mask;
Das Funktionsargument ändert sich. Und alle. Dieses Argument wird nicht mehr verwendet. Höchstwahrscheinlich hättest du es so schreiben sollen:
Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
this->Mode &= Mask;
};
Fragment N37: Variablenverwirrung
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;
}
Warnung PVS-Studio: V1001 [CWE-563] Die Variable „Size“ ist zugewiesen, wird aber am Ende der Funktion nicht verwendet. Object.cpp 424
Die Situation ähnelt der vorherigen. Es sollte geschrieben werden:
this->Size += this->EntrySize;
Fragment N38-N47: Sie haben vergessen, den Index zu überprüfen
Zuvor haben wir uns Beispiele für die Diagnoseauslösung angesehen
int getGEPCost(Type *PointeeType, const Value *Ptr,
ArrayRef<const Value *> Operands) {
....
if (Ptr != nullptr) { // <=
assert(....);
BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts());
}
bool HasBaseReg = (BaseGV == nullptr);
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <=
....
}
PVS-Studio-Warnung: V1004 [CWE-476] Der „Ptr“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 729, 738. TargetTransformInfoImpl.h 738
Variable ptr kann gleich sein nullptr, wie aus dem Scheck hervorgeht:
if (Ptr != nullptr)
Im Folgenden wird dieser Zeiger jedoch ohne vorherige Prüfung dereferenziert:
auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());
Betrachten wir einen anderen ähnlichen Fall.
llvm::DISubprogram *CGDebugInfo::getFunctionFwdDeclOrStub(GlobalDecl GD,
bool Stub) {
....
auto *FD = dyn_cast<FunctionDecl>(GD.getDecl());
SmallVector<QualType, 16> ArgTypes;
if (FD) // <=
for (const ParmVarDecl *Parm : FD->parameters())
ArgTypes.push_back(Parm->getType());
CallingConv CC = FD->getType()->castAs<FunctionType>()->getCallConv(); // <=
....
}
PVS-Studio-Warnung: V1004 [CWE-476] Der „FD“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 3228, 3231. CGDebugInfo.cpp 3231
Achten Sie auf das Schild FD. Ich bin sicher, dass das Problem klar erkennbar ist und keiner besonderen Erklärung bedarf.
Und noch mehr:
static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
Value *&BasePtr,
const DataLayout &DL) {
PointerType *PtrTy = dyn_cast<PointerType>(Ptr.getType());
if (!PtrTy) { // <=
Result = Polynomial();
BasePtr = nullptr;
}
unsigned PointerBits =
DL.getIndexSizeInBits(PtrTy->getPointerAddressSpace()); // <=
....
}
PVS-Studio-Warnung: V1004 [CWE-476] Der „PtrTy“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 960, 965. InterleavedLoadCombinePass.cpp 965
Wie schützt man sich vor solchen Fehlern? Seien Sie bei der Codeüberprüfung aufmerksamer und verwenden Sie den statischen Analysator von PVS-Studio, um Ihren Code regelmäßig zu überprüfen.
Es macht keinen Sinn, andere Codefragmente mit Fehlern dieser Art zu zitieren. Ich werde im Artikel nur eine Liste von Warnungen hinterlassen:
- V1004 [CWE-476] Der „Expr“-Zeiger wurde unsicher verwendet, nachdem er anhand von nullptr überprüft wurde. Überprüfen Sie die Zeilen: 1049, 1078. DebugInfoMetadata.cpp 1078
- V1004 [CWE-476] Der „PI“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 733, 753. LegacyPassManager.cpp 753
- V1004 [CWE-476] Der „StatepointCall“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 4371, 4379. Verifier.cpp 4379
- V1004 [CWE-476] Der „RV“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 2263, 2268. TGParser.cpp 2268
- V1004 [CWE-476] Der „CalleeFn“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Überprüfen Sie die Zeilen: 1081, 1096. SimplifyLibCalls.cpp 1096
- V1004 [CWE-476] Der „TC“-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr überprüft wurde. Prüflinien: 1819, 1824. Driver.cpp 1824
Fragment N48-N60: Nicht kritisch, aber ein Defekt (möglicher Speicherverlust)
std::unique_ptr<IRMutator> createISelMutator() {
....
std::vector<std::unique_ptr<IRMutationStrategy>> Strategies;
Strategies.emplace_back(
new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
....
}
PVS-Studio-Warnung:
Um ein Element am Ende eines Containers hinzuzufügen, z std::vector > Du kannst nicht einfach schreiben xxx.push_back(neues X), da es keine implizite Konvertierung von gibt X* в std::unique_ptr.
Eine gängige Lösung ist das Schreiben xxx.emplace_back(neues X)da es kompiliert: Methode emplace_back Konstruiert ein Element direkt aus seinen Argumenten und kann daher explizite Konstruktoren verwenden.
Es ist nicht sicher. Wenn der Vektor voll ist, wird der Speicher neu zugewiesen. Der Speicherneuzuweisungsvorgang schlägt möglicherweise fehl, was dazu führt, dass eine Ausnahme ausgelöst wird std::bad_alloc. In diesem Fall geht der Zeiger verloren und das erstellte Objekt wird niemals gelöscht.
Eine sichere Lösung ist das Erstellen einzigartiger_ptrwelches den Zeiger besitzt, bevor der Vektor versucht, Speicher neu zuzuweisen:
xxx.push_back(std::unique_ptr<X>(new X))
Seit C++14 können Sie „std::make_unique“ verwenden:
xxx.push_back(std::make_unique<X>())
Diese Art von Fehler ist für LLVM nicht kritisch. Wenn kein Speicher zugewiesen werden kann, stoppt der Compiler einfach. Allerdings für Anwendungen mit langer
Obwohl dieser Code also keine praktische Bedrohung für LLVM darstellt, fand ich es nützlich, über dieses Fehlermuster zu sprechen und dass der PVS-Studio-Analysator gelernt hat, es zu identifizieren.
Weitere Warnungen dieser Art:
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Passes“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. PassManager.h 546
- V1023 [CWE-460] Ein Zeiger ohne Eigentümer wird durch die Methode „emplace_back“ zum Container „AAs“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. AliasAnalysis.h 324
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Entries“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. DWARFDebugFrame.cpp 519
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „AllEdges“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. CFGMST.h 268
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „VMaps“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. SimpleLoopUnswitch.cpp 2012
- V1023 [CWE-460] Ein Zeiger ohne Eigentümer wird durch die Methode „emplace_back“ zum Container „Records“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. FDRLogBuilder.h 30
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „PendingSubmodules“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. ModuleMap.cpp 810
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Objects“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. DebugMap.cpp 88
- V1023 [CWE-460] Ein Zeiger ohne Eigentümer wird durch die Methode „emplace_back“ zum Container „Strategies“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-isel-fuzzer.cpp 60
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 685
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 686
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 688
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 689
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 690
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 691
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 692
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 693
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Modifiers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. llvm-stress.cpp 694
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Operanden“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. GlobalISelEmitter.cpp 1911
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum „Stash“-Container hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. GlobalISelEmitter.cpp 2100
- V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode „emplace_back“ zum Container „Matchers“ hinzugefügt. Im Falle einer Ausnahme tritt ein Speicherverlust auf. GlobalISelEmitter.cpp 2702
Abschluss
Insgesamt habe ich 60 Warnungen ausgegeben und dann angehalten. Gibt es andere Fehler, die der PVS-Studio-Analysator in LLVM erkennt? Ja, gibt es. Als ich jedoch Codefragmente für den Artikel schrieb, war es später Abend oder besser gesagt sogar Nacht, und ich beschloss, dass es an der Zeit war, Schluss zu machen.
Ich hoffe, Sie fanden es interessant und möchten den PVS-Studio-Analysator ausprobieren.
Sie können den Analysator herunterladen und den Minesweeper-Schlüssel erhalten unter
Am wichtigsten ist, dass Sie die statische Analyse regelmäßig verwenden. Einmalige Schecks, die wir durchführen, um die Methodik der statischen Analyse und PVS-Studio bekannt zu machen, sind kein normales Szenario.
Viel Glück bei der Verbesserung der Qualität und Zuverlässigkeit Ihres Codes!
Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Übersetzungslink: Andrey Karpov.
Source: habr.com