Wir finden Fehler in LLVM 8 mit dem PVS-Studio-Analyzer

Wir finden Bugs in LLVM 8 mit Hilfe des PVS-Studio-Analyzers.
Es sind ĂŒber zwei Jahre seit der letzten CodeĂŒberprĂŒfung des LLVM-Projekts mit unserem PVS-Studio-Analyzer vergangen. Lassen Sie uns sicherstellen, dass der PVS-Studio-Analyzer nach wie vor das fĂŒhrende Tool zur Auffindung von Fehlern und potenziellen SicherheitsanfĂ€lligkeiten ist. Dazu werden wir neue Fehler in der Version 8.0.0 von LLVM ĂŒberprĂŒfen und finden.

Ein Artikel, der geschrieben werden sollte

Um ehrlich zu sein, wollte ich diesen Artikel nicht schreiben. Es ist nicht besonders spannend, ĂŒber ein Projekt zu berichten, das wir bereits mehrmals ĂŒberprĂŒft haben (1, 2, 3). Es wĂ€re besser, ĂŒber etwas Neues zu schreiben, aber ich habe keine Wahl.

Jedes Mal, wenn eine neue Version von LLVM veröffentlicht oder aktualisiert wird Clang Static Analyzer, erhalten wir in unserer E-Mail Fragen wie die folgenden:

Sehen Sie, die neue Version des Clang Static Analyzer hat neue Fehler gefunden! Ich habe das GefĂŒhl, dass die Relevanz von PVS-Studio abnimmt. Clang findet mehr Fehler als frĂŒher und holt bei den FĂ€higkeiten von PVS-Studio auf. Was denken Sie darĂŒber?

Darauf antworte ich immer gerne mit etwas in dieser Art:

Wir sitzen auch nicht tatenlos herum! Wir haben die Möglichkeiten des PVS-Studio-Analyzers erheblich verbessert. Keine Sorge, wir bleiben weiterhin fĂŒhrend, wie eh und je.

Leider ist das eine schlechte Antwort. Es fehlen die Beweise. Genau aus diesem Grund schreibe ich diesen Artikel jetzt. Das LLVM-Projekt wurde erneut ĂŒberprĂŒft und es wurden vielfĂ€ltigste Fehler gefunden. Die, die ich interessant fand, werde ich jetzt zeigen. Diese Fehler kann der Clang Static Analyzer nicht finden (oder es ist Ă€ußerst umstĂ€ndlich, dies damit zu tun). Aber wir können das. Ich habe all diese Fehler an einem Abend gefunden und aufgeschrieben.

Das Schreiben des Artikels hat sich jedoch ĂŒber mehrere Wochen hingezogen. Ich konnte mich einfach nicht dazu bringen, all das in Textform zu bringen :).

Übrigens, falls Sie interessiert sind, welche Technologien im PVS-Studio-Analyzer verwendet werden, um Fehler und potenzielle SicherheitsanfĂ€lligkeiten zu identifizieren, schlage ich vor, sich mit diesem Artikel.

Neue und alte Diagnosen

Wie bereits erwĂ€hnt, wurde das LLVM-Projekt vor etwa zwei Jahren erneut ĂŒberprĂŒft und die gefundenen Fehler behoben. In diesem Artikel wird nun eine neue Reihe von Fehlern vorgestellt. Warum wurden neue Fehler gefunden? DafĂŒr gibt es drei GrĂŒnde:

  1. Das LLVM-Projekt entwickelt sich weiter, der alte Code wird geĂ€ndert und neuer Code entsteht. NatĂŒrlich gibt es im geĂ€nderten und neu geschriebenen Code neue Fehler. Dies zeigt deutlich, dass statische Analysen regelmĂ€ĂŸig und nicht nur sporadisch durchgefĂŒhrt werden sollten. Unsere Artikel veranschaulichen die Möglichkeiten des PVS-Studio-Analyzers, jedoch hĂ€ngt dies nicht mit der Verbesserung der CodequalitĂ€t und der Reduzierung der Fehlerbehebungskosten zusammen. Verwenden Sie den Code-Analyzer regelmĂ€ĂŸig!
  2. Wir ĂŒberarbeiten und verbessern bereits bestehende Diagnosen. Daher kann der Analyzer Fehler aufdecken, die bei vorherigen PrĂŒfungen ĂŒbersehen wurden.
  3. Im PVS-Studio sind neue Diagnosen hinzugekommen, die es vor 2 Jahren noch nicht gab. Ich habe beschlossen, diese in einen separaten Abschnitt zu heben, um die Entwicklung von PVS-Studio deutlich zu zeigen.

Fehler, die von Diagnosen entdeckt wurden, die es vor 2 Jahren gab

Fragment N1: Copy-Paste

static bool ShouldUpgradeX86Intrinsic(Function *F, StringRef Name) {
  if (Name == "addcarryx.u32" || // HinzugefĂŒgt in 8.0
    ....
    Name == "avx512.mask.cvtps2pd.128" || // HinzugefĂŒgt in 7.0
    Name == "avx512.mask.cvtps2pd.256" || // HinzugefĂŒgt in 7.0
    Name == "avx512.cvtusi2sd" || // HinzugefĂŒgt in 7.0
    Name.startswith("avx512.mask.permvar.") || // HinzugefĂŒgt in 7.0     // <=
    Name.startswith("avx512.mask.permvar.") || // HinzugefĂŒgt in 7.0     // <=
    Name == "sse2.pmulu.dq" || // HinzugefĂŒgt in 7.0
    Name == "sse41.pmuldq" || // HinzugefĂŒgt in 7.0
    Name == "avx2.pmulu.dq" || // HinzugefĂŒgt in 7.0
  ....
}

Warnung PVS-Studio: V501 [CWE-570] Es gibt identische TeilausdrĂŒcke ‘Name.startswith(«avx512.mask.permvar.»)’ links und rechts des ‘||’ Operators. AutoUpgrade.cpp 73

Es wird zweimal geprĂŒft, ob der Name mit der Teilfolge «avx512.mask.permvar.» beginnt. Bei der zweiten PrĂŒfung wollte man offensichtlich etwas anderes schreiben, hat aber vergessen, den kopierten Text zu Ă€ndern.

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 TeilausdrĂŒcke ‘CXNameRange_WantQualifier’ links und rechts des ‘|’ Operators. CIndex.cpp 7245

Wegen eines Tippfehlers wird dieselbe benannte Konstante zweimal verwendet. CXNameRange_WantQualifier.

Fragment N3: Verwirrung mit OperatorprioritÀten

int PPCTTIImpl::getVectorInstrCost(unsigned Opcode, Type *Val, unsigned Index) {
  ....
  if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian() ? 1 : 0)
    return 0;
  ....
}

Warnung PVS-Studio: V502 [CWE-783] Vielleicht funktioniert der ‘?:’ Operator anders als erwartet. Der ‘?:’ Operator hat eine geringere PrioritĂ€t als der ‘==’ Operator. PPCTargetTransformInfo.cpp 404

Ich finde, es ist ein sehr schöner Fehler. Ja, ich weiß, dass ich seltsame Vorstellungen von Schönheit habe :).

Momentan, gemĂ€ĂŸ den OperatorprioritĂ€ten, wird der Ausdruck wie folgt ausgewertet:

(ISD == ISD::EXTRACT_VECTOR_ELT && (Index == ST->isLittleEndian())) ? 1 : 0

Praktisch betrachtet macht eine solche Bedingung keinen Sinn, da sie auf folgendes vereinfacht werden kann:

(ISD == ISD::EXTRACT_VECTOR_ELT && Index == ST->isLittleEndian())

Das ist ein offensichtlicher Fehler. Wahrscheinlich wollte man 0/1 mit der Variablen Indexvergleichen. Um den Code zu korrigieren, mĂŒssen Klammern um den ternĂ€ren Operator hinzugefĂŒgt werden:

if (ISD == ISD::EXTRACT_VECTOR_ELT && Index == (ST->isLittleEndian() ? 1 : 0))

Übrigens, der ternĂ€re Operator ist sehr gefĂ€hrlich und kann logische Fehler verursachen. Seien Sie sehr vorsichtig damit und scheuen Sie sich nicht, runde Klammern zu verwenden. Mehr zu diesem Thema habe ich behandelt hier, in dem Kapitel „FĂŒrchten Sie den Operator ?: und setzen Sie ihn in runde Klammern“.

Fragment N4, N5: Nullzeiger

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("kann '") + LHS->getAsString() +
                    "' nicht in einen String umwandeln");
    return nullptr;
  }
  ....
}

Warnung PVS-Studio: V522 [CWE-476] Dereferenzierung des Nullzeigers ‚LHS‘ könnte stattfinden. TGParser.cpp 2152

Wenn der Zeiger LHS null ist, sollte eine Warnung ausgegeben werden. Stattdessen erfolgt jedoch die Dereferenzierung dieses Nullzeigers: LHS->getAsString().

Dies ist eine typische Situation, in der der Fehler im Fehlerhandler verborgen ist, da dieser nicht getestet wird. 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 statische Analyse andere Testmethoden und Fehlervermeidung ergĂ€nzt.

Ein Ă€hnlicher Fehler bei der Zeigerbehandlung RHS tritt im Code nur wenig weiter unten auf: V522 [CWE-476] Dereferenzierung des Nullzeigers ‚RHS‘ könnte stattfinden. TGParser.cpp 2186

Fragment N6: Verwendung eines Zeigers nach der Verschiebung

static Expected
ExtractBlocks(....)
{
  ....
  std::unique_ptr ProgClone = CloneModule(BD.getProgram(), VMap);
  ....
  BD.setNewProgram(std::move(ProgClone));                                // getFunction(MisCompFunctions[i].first);  // <=
    assert(NewF && "Function not found??");
    MiscompiledFunctions.push_back(NewF);
  }
  ....
}

Warnung PVS-Studio: V522 [CWE-476] Dereferenzierung des Nullzeigers 'ProgClone' könnte stattfinden. Miscompilation.cpp 601

Am Anfang der Smart Pointer ProgClone hört auf, das Objekt zu besitzen:

BD.setNewProgram(std::move(ProgClone));

TatsĂ€chlich ist dies jetzt ProgClone — ein Nullzeiger. Daher sollte weiter unten eine Dereferenzierung des Nullzeigers stattfinden:

Function *NewF = ProgClone->getFunction(MisCompFunctions[i].first);

Aber in Wirklichkeit wird das nicht geschehen! Beachten Sie, dass die Schleife tatsĂ€chlich nicht ausgefĂŒhrt wird.

Zu Beginn wird der Container MiscompiledFunctions geleert:

MiscompiledFunctions.clear();

Dann 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 gestartet wird. Ich denke, das ist ebenfalls ein Fehler, und der Code sollte anders geschrieben werden.

Es scheint, dass wir auf die berĂŒhmte FehlerparitĂ€t gestoßen sind! Ein Fehler maskiert den anderen :).

Fragment N7: Verwendung des Zeigers nach der Verschiebung

static Expected TestOptimizer(BugDriver &BD, std::unique_ptr Test,
                                    std::unique_ptr Safe) {
  outs() << "  Optimierung der getesteten Funktionen: ";
  std::unique_ptr Optimized =
      BD.runPassesOn(Test.get(), BD.getPassesToRun());
  if (!Optimized) {
    errs() << " Fehler bei der AusfĂŒhrung dieser Folge von Passes"
           << " auf dem Eingabeprogramm!n";
    BD.setNewProgram(std::move(Test));                       // <=
    BD.EmitProgressBitcode(*Test, "pass-error", false);      // <=
    if (Error E = BD.debugOptimizerCrash())
      return std::move(E);
    return false;
  }
  ....
}

Warnung PVS-Studio: V522 [CWE-476] Dereferenzierung des Nullzeigers 'Test' könnte stattfinden. Miscompilation.cpp 709

Wieder das gleiche Szenario. ZunĂ€chst wird der Inhalt des Objekts verschoben, und dann wird er wie gewohnt verwendet. Ich stoße immer hĂ€ufiger auf diese Situation im Programmcode, nachdem C++ die Semantik der Verschiebung eingefĂŒhrt hat. DafĂŒr liebe ich die Sprache C++! Es gibt immer wieder neue Wege, sich ins eigene Bein zu schießen. Der PVS-Studio-Analyzer wird nie arbeitslos sein :).

Fragment N8: Null-Zeiger

void FunctionDumper::dump(const PDBSymbolTypeFunctionArg &Symbol) {
  uint32_t TypeId = Symbol.getTypeId();
  auto Type = Symbol.getSession().getSymbolById(TypeId);
  if (Type)
    Printer << "";
  else
    Type->dump(*this);
}

PVS-Studio-Warnung: V522 [CWE-476] Dereferenzierung des Nullzeigers 'Type' könnte auftreten. PrettyFunctionDumper.cpp 233

Neben Fehlerbehandlungsroutinen werden oft auch Funktionen fĂŒr Debug-Ausgaben nicht getestet. Dies ist genau ein solcher Fall. Die Funktion erwartet einen Benutzer, der anstelle seiner Probleme die Korrektur dieser Funktion ĂŒbernehmen muss.

Korrekt:

if (Type)
  Type->dump(*this);
else
  Printer << "";

Fragment N9: Nullzeiger

void SearchableTableEmitter::collectTableEntries(
    GenericTable &Table, const std::vector &Items) {
  ....
  RecTy *Ty = resolveTypes(Field.RecType, TI->getType());
  if (!Ty)                                                              // getAsString() + " vs. " +                       // getType()->getAsString());
   ....
}

PVS-Studio-Warnung: V522 [CWE-476] Dereferenzierung des Nullzeigers 'Ty' könnte auftreten. SearchableTableEmitter.cpp 614

Ich denke, alles ist klar und bedarf keiner weiteren 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;
}

Warnung PVS-Studio: V570 Die Variable 'Identifier->Type' wird sich selbst zugewiesen. FormatTokenLexer.cpp 249

Es macht keinen Sinn, eine Variable sich selbst zuzuweisen. Wahrscheinlich wollte man schreiben:

Identifier->Type = Question->Type;

Fragment N11: VerdÀchtiger break

void SystemZOperand::print(raw_ostream &OS) const {
  switch (Kind) {
    break;
  case KindToken:
    OS << "Token:" << getToken();
    break;
  case KindReg:
    OS << "Reg:" << SystemZInstPrinter::getRegisterName(getReg());
    break;
  ....
}

Warnung PVS-Studio: V622 [CWE-478] Bitte prĂŒfen Sie die 'switch'-Anweisung. Es ist möglich, dass der erste 'case'-Operator fehlt. SystemZAsmParser.cpp 652

Zu Beginn gibt es einen sehr verdÀchtigen Operator break. Haben Sie hier vielleicht etwas vergessen?

Fragment N12: ÜberprĂŒfung des Zeigers nach 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("undefinierter callee");
  ....
}

Warnung PVS-Studio: V595 [CWE-476] Der 'Callee'-Zeiger wurde verwendet, bevor er auf nullptr ĂŒberprĂŒft wurde. ÜberprĂŒfen Sie die Zeilen: 172, 174. AMDGPUInline.cpp 172

Zeiger Callee wird zu Beginn beim Funktionsaufruf dereferenziert getTTI.

Es stellt sich dann heraus, dass dieser Zeiger auf Gleichheit ĂŒberprĂŒft werden sollte nullptr:

if (!Callee || Callee->isDeclaration())

Doch es ist bereits zu spÀt


Fragment N13 — N
: ÜberprĂŒfung des Zeigers nach Dereferenzierung

Die im vorherigen Codeabschnitt behandelte Situation ist nicht einzigartig. Sie tritt hier auf:

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()) {               // <=
  ....
}

Warnung PVS-Studio: 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(New);
  CXXRecordDecl *ThisContext =
    dyn_cast_or_null(ND->getDeclContext());         // isCXXInstanceMember());     // <=
  ....
}

Warnung PVS-Studio: 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. DWARFFormValue.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

Dann wurde es fĂŒr mich uninteressant, die Warnungen mit der Nummer V595 zu untersuchen. Daher weiß ich nicht, ob es noch weitere Ă€hnliche Fehler gibt, außer den hier aufgefĂŒhrten. Wahrscheinlich gibt es noch welche.

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;
  ....
}

Warnung PVS-Studio: V629 [CWE-190] Ziehen Sie in Betracht, den Ausdruck ‘~(Size — 1) << 1’ zu ĂŒberprĂŒfen. Bitverschiebung des 32-Bit-Werts mit anschließender Erweiterung auf den 64-Bit-Typ. AArch64AddressingModes.h 260

Es ist möglich, dass dies kein Fehler ist und der Code genau so funktioniert, wie vorgesehen. Aber es ist eindeutig ein sehr verdĂ€chtiger Punkt und sollte ĂŒberprĂŒft werden.

Angenommen, die Variable Size ist gleich 16, und der Autor des Codes beabsichtigte, in der Variablen NImms den Wert zu erhalten:

1111111111111111111111111111111111111111111111111111111111100000

In Wirklichkeit erhÀlt man jedoch den Wert:

0000000000000000000000000000000011111111111111111111111111100000

Das Problem ist, dass alle Berechnungen unter Verwendung des 32-Bit-unsigned Typs durchgefĂŒhrt werden. Und erst dann wird dieser 32-Bit-unsigned Typ implizit auf uint64_terweitert. Dabei sind die höheren Bits null.

Die Situation kann so behoben werden:

uint64_t NImms = ~static_cast(Size-1) << 1;

Ähnliche Situation: V629 [CWE-190] Ziehen Sie in Betracht, den Ausdruck ‘Immr << 6’ zu ĂŒberprĂŒfen. Bitverschiebung des 32-Bit-Werts mit anschließender Erweiterung auf den 64-Bit-Typ. AArch64AddressingModes.h 269

Fragment N19: SchlĂŒsselwort fehlt 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 verwendet das "vcc" Token.
    // Überspringen.
    continue;
  } if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) {    // <=
    Op.addRegWithFPInputModsOperands(Inst, 2);
  } else if (Op.isDPPCtrl()) {
    Op.addImmOperands(Inst, 1);
  } else if (Op.isImm()) {
    // Behandle optionale Argumente
    OptionalIdx[Op.getImmTy()] = I;
  } else {
    llvm_unreachable("UngĂŒltiger Operandtyp");
  }
  ....
}

Warnung PVS-Studio: V646 [CWE-670] ErwĂ€gen Sie eine ÜberprĂŒfung der Logik der Anwendung. Möglicherweise fehlt das SchlĂŒsselwort 'else'. AMDGPUAsmParser.cpp 5655

Hier gibt es keinen Fehler. Da der then-Block des ersten wenn mit continue, endet, spielt es keine Rolle, ob das SchlĂŒsselwort else vorhanden ist oder nicht. In jedem Fall wird der Code gleich funktionieren. Dennoch macht das Fehlen eines else den Code unklarer und gefĂ€hrlicher. Wenn es spĂ€ter continue verschwinden sollte, wird der Code ganz anders funktionieren. Meiner Meinung nach ist es besser, hinzu zu fĂŒgen else.

Fragment N20: Vier Àhnliche Tippfehler

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 Klasse) " + Name.str();      // <=
    break;
  case SymbolKind::ObjectiveCClassEHType:
    Result + "(ObjC Klasse EH) " + Name.str();   // <=
    break;
  case SymbolKind::ObjectiveCInstanceVariable:
    Result + "(ObjC IVar) " + Name.str();       // <=
    break;
  }
  OS << Result;
}

PVS-Studio Warnungen:

  • V655 [CWE-480] Die Strings wurden verkettet, aber nicht verwendet. ÜberprĂŒfen Sie den ‚Result + Name.str()‘ Ausdruck. Symbol.cpp 32
  • V655 [CWE-480] Die Strings wurden verkettet, aber nicht verwendet. ÜberprĂŒfen Sie den ‚Result + "(ObjC Klasse) " + Name.str()‘ Ausdruck. Symbol.cpp 35
  • V655 [CWE-480] Die Strings wurden verkettet, aber nicht verwendet. ÜberprĂŒfen Sie den ‚Result + "(ObjC Klasse EH) " + Name.str()‘ Ausdruck. Symbol.cpp 38
  • V655 [CWE-480] Die Strings wurden verkettet, aber nicht verwendet. ÜberprĂŒfen Sie den ‚Result + "(ObjC IVar) " + Name.str()‘ Ausdruck. Symbol.cpp 41

Statt des Operators += wird zufÀllig der Operator + verwendet. Infolgedessen entstehen sinnlose Konstruktionen.

Fragment N21: Unbestimmtes Verhalten

static void getReqFeatures(std::map &FeaturesMap,
                           const std::vector &ReqFeatures) {
  for (auto &R : ReqFeatures) {
    StringRef AsmCondString = R->getValueAsString("AssemblerCondString");

    SmallVector Ops;
    SplitString(AsmCondString, Ops, ",");
    assert(!Ops.empty() && "AssemblerCondString darf nicht leer sein");

    for (auto &Op : Ops) {
      assert(!Op.empty() && "Leerer Operator");
      if (FeaturesMap.find(Op) == FeaturesMap.end())
        FeaturesMap[Op] = FeaturesMap.size();
    }
  }
}

Versuchen Sie selbst, den gefÀhrlichen Code zu finden. Und das ist ein Bild zur Ablenkung, sodass Sie nicht sofort die Antwort sehen:

Wir finden Bugs in LLVM 8 mit Hilfe des PVS-Studio-Analyzers.

Warnung PVS-Studio: V708 [CWE-758] GefĂ€hrliche Konstruktion wird verwendet: ‘FeaturesMap[Op] = FeaturesMap.size()’, wobei ‘FeaturesMap’ vom Typ ‘map’ ist. Dies kann zu undefiniertem Verhalten fĂŒhren. RISCVCompressInstEmitter.cpp 490

Problematische Zeile:

FeaturesMap[Op] = FeaturesMap.size();

Wenn das Element Op nicht gefunden wird, wird ein neues Element in der Karte erstellt und die Anzahl der Elemente in dieser Karte gespeichert. Unklar ist jedoch, ob die Funktion GrĂ¶ĂŸe vor oder nach dem HinzufĂŒgen des neuen Elements aufgerufen wird.

Fragment N22-N24: Wiederholte Zuweisungen

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;
  }
  ....
}

Warnung PVS-Studio: V519 [CWE-563] Die Variable ‚NType‘ wird zweimal hintereinander zugewiesen. Das könnte ein Fehler sein. ÜberprĂŒfen Sie die Zeilen: 1663, 1664. MachOObjectFile.cpp 1664

Ich denke, hier liegt kein echter Fehler vor. Es handelt sich nur um eine ĂŒberflĂŒssige wiederholte Zuweisung. Trotzdem ein Fauxpas.

Ähnlich:

  • V519 [CWE-563] Die Variable ‚B.NDesc‘ wird zweimal hintereinander zugewiesen. Das könnte ein Fehler sein. ÜberprĂŒfen Sie die Zeilen: 1488, 1489. llvm-nm.cpp 1489
  • V519 [CWE-563] Die Variable wird zweimal hintereinander zugewiesen. Das könnte ein Fehler sein. ÜberprĂŒfen Sie die Zeilen: 59, 61. coff2yaml.cpp 61

Fragment N25-N27: Weitere wiederholte Zuweisungen

Betrachten wir nun eine etwas andere Variante der wiederholten Zuweisung.

bool Vectorizer::vectorizeLoadChain(
    ArrayRef Chain,
    SmallPtrSet *InstructionsProcessed) {
  ....
  unsigned Alignment = getAlignment(L0);
  ....
  unsigned NewAlign = getOrEnforceKnownAlignment(L0->getPointerOperand(),
                                                 StackAdjustedAlignment,
                                                 DL, L0, nullptr, &DT);
  if (NewAlign != 0)
    Alignment = NewAlign;
  Alignment = NewAlign;
  ....
}

Warnung PVS-Studio: V519 [CWE-563] Die Variable ‚Alignment‘ wird zweimal hintereinander zugewiesen. Das könnte ein Fehler sein. ÜberprĂŒfen Sie die Zeilen: 1158, 1160. LoadStoreVectorizer.cpp 1160

Das ist ein sehr seltsamer Code, der offensichtlich einen logischen Fehler enthĂ€lt. ZunĂ€chst wird der Variable Alignment ein Wert abhĂ€ngig von einer Bedingung zugewiesen. Danach erfolgt erneut eine Zuweisung, jedoch ohne jegliche ÜberprĂŒfung.

Ähnliche Situationen finden Sie hier:

  • V519 [CWE-563] Die Variable 'Effects' wird zweimal hintereinander zugewiesen. Möglicherweise handelt es sich um einen Fehler. ÜberprĂŒfen Sie die Zeilen: 152, 165. WebAssemblyRegStackify.cpp 165
  • V519 [CWE-563] Die Variable 'ExpectNoDerefChunk' wird zweimal hintereinander zugewiesen. Möglicherweise handelt es sich um einen Fehler. ÜberprĂŒfen Sie die Zeilen: 4970, 4973. SemaType.cpp 4973

Fragment N28: Immer wahre Bedingung

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) // UnterstĂŒtzung fĂŒr PAUSE-Anweisung             // <=
      break;
  }
  ....
}

Warnung PVS-Studio: V547 [CWE-571] Ausdruck 'nextByte != 0x90' ist immer wahr. X86DisassemblerDecoder.cpp 379

Die ÜberprĂŒfung ergibt keinen Sinn. Die Variable nextByte ist immer ungleich dem Wert 0x90, was sich aus der vorherigen ÜberprĂŒfung ergibt. Das ist ein logischer Fehler.

Fragment N29 — N
: Immer wahre/falsche Bedingungen

Der Analyzer gibt viele Warnungen aus, dass die gesamte Bedingung (V547) oder ein Teil davon (V560) ist immer wahr oder falsch. Oft sind das keine echten Fehler, sondern einfach unordentlicher Code, das Ergebnis von Makro-Deployments und Àhnlichem. Dennoch lohnt es sich, all diese Warnungen zu betrachten, da gelegentlich tatsÀchlich logische Fehler auftreten. Zum Beispiel ist dieser Codeabschnitt verdÀchtig:

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;
  ....
}

Warnung PVS-Studio: V560 [CWE-570] Ein Teil des bedingten Ausdrucks ist immer falsch: RegNo == 0xe. ARMDisassembler.cpp 939

Die Konstante 0xE hat den Wert 14 im Dezimalsystem. Die ÜberprĂŒfung RegNo == 0xe macht keinen Sinn, da wenn RegNo > 13, die Funktion ihre AusfĂŒhrung beendet.

Es gab viele andere Warnungen mit den Identifikatoren V547 und V560, aber Ă€hnlich wie bei V595, war es mir uninteressant, diese Warnungen zu untersuchen. Es war klar, dass ich genug Material fĂŒr einen Artikel hatte :). Daher ist unklar, wie viele solcher Fehler in LLVM mittels PVS-Studio aufgezeigt werden können.

Ich erklĂ€re, warum es langweilig ist, diese Auslösungen zu studieren. Der Analyzer hat vollkommen recht, wenn er eine Warnung fĂŒr den folgenden Code ausgibt. Aber das stellt auch keinen Fehler dar.

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Àchtiges return

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();
  }
  ....
}

Warnung PVS-Studio: V612 [CWE-670] Ein bedingungsloses ‘return’ innerhalb einer Schleife. R600OptimizeVectorRegisters.cpp 63

Das ist entweder ein Fehler oder eine spezifische Technik, die etwas fĂŒr die Programmierer erschließen soll, die den Code lesen. FĂŒr mich erklĂ€rt diese Konstruktion nichts und wirkt sehr verdĂ€chtig. Man sollte so besser nicht schreiben :).

MĂŒde? Dann ist es Zeit, Tee oder Kaffee zu machen.

Wir finden Bugs in LLVM 8 mit Hilfe des PVS-Studio-Analyzers.

Defekte, die durch neue Diagnosen entdeckt wurden

Ich denke, 30 Auslösungen der alten Diagnosen sind genĂŒgend. Lassen Sie uns jetzt sehen, was Interessantes durch die neuen Diagnosen gefunden werden kann, die im Analyzer bereits nachtrĂ€glich hinzugekommen sind. dem vorherigen ÜberprĂŒfungen. In dieser Zeit wurden im C++-Analyzer 66 allgemeine Diagnosen hinzugefĂŒgt.

Fragment N31: Unerreichbarer 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();
}

Warnung PVS-Studio: V779 [CWE-561] Unerreichbarer Code erkannt. Es könnte ein Fehler vorliegen. ExecutionUtils.cpp 146

Wie Sie sehen, enden beide Zweige des Operators wenn mit einem Aufruf des Operators return. Dementsprechend wird der Container CtorDtorsByPriority nie geleert.

Fragment N32: Unerreichbarer 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(), "unerwarteter Zusammenfassungsart");
  }
  Lex.setIgnoreColonInIdentifiers(false);                      
  return false;
}

Warnung PVS-Studio: V779 [CWE-561] Unerreichbarer Code erkannt. Es könnte ein Fehler vorliegen. LLParser.cpp 835

Eine interessante Situation. Lassen Sie uns zunÀchst diesen Punkt betrachten:

return ParseTypeIdEntry(SummaryID);
break;

Auf den ersten Blick scheint es keinen Fehler zu geben. Es sieht so aus, als wĂŒrde der Operator break Hier ist etwas ĂŒberflĂŒssig und kann einfach entfernt werden. Doch so einfach ist es nicht.

Der Analysator gibt eine Warnung fĂŒr die Zeilen aus:

Lex.setIgnoreColonInIdentifiers(false);
return false;

Und tatsÀchlich ist dieser Code unerreichbar. Alle FÀlle in switch enden mit einem Aufruf des Operators return. Und jetzt sieht der sinnlose einsame break nicht mehr so harmlos aus! Vielleicht sollte einer der Zweige mit break, und nicht mit return?

Fragment N33: ZufÀllige Nullierung der höheren Bits

unsigned getStubAlignment() override {
  if (Arch == Triple::systemz)
    return 8;
  else
    return 1;
}

Erwartet<unsigned>
RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
                             const SectionRef &Section,
                             bool IsCode) {
  ....
  uint64_t DataSize = Section.getSize();
  ....
  if (StubBufSize > 0)
    DataSize &= ~(getStubAlignment() - 1);
  ....
}

Warnung PVS-Studio: V784 Die GrĂ¶ĂŸe der Bitmaske ist kleiner als die GrĂ¶ĂŸe des ersten Operanden. Dies wird zu einem Verlust höherer Bits fĂŒhren. RuntimeDyld.cpp 815

Bitte beachten Sie, dass die Funktion getStubAlignment den Typ unsigned. Lassen Sie uns den Wert des Ausdrucks berechnen, wenn wir annehmen, dass die Funktion den Wert 8 zurĂŒckgibt:

~(getStubAlignment() - 1)

~(8u-1)

0xFFFFFFF8‬u

Jetzt beachten Sie bitte, dass die Variable DataSize verwendet einen 64-Bit-unsigned-Typ. Das bedeutet, dass bei der AusfĂŒhrung der Operation DataSize & 0xFFFFFFF8‬u alle oberen dreißig Bits auf Null gesetzt werden. Höchstwahrscheinlich ist das nicht das, was der Programmierer beabsichtigt hat. Ich vermute, er wollte berechnen: DataSize & 0xFFFFFFFFFFFFFFF8‬u.

Um den Fehler zu beheben, sollte man Folgendes schreiben:

DataSize & = ~(static_cast<uint64_t>(getStubAlignment()) - 1);

Oder so:

DataSize & = ~(getStubAlignment() - 1ULL);

Fragment N34: Misslungenes explizites Typcasting

template <typename T>
void scaleShuffleMask(int Scale, ArrayRef<T> Mask,
                      SmallVectorImpl<T> & ScaledMask) {
  assert(0 < Scale && "Unerwarteter Skalierungsfaktor");
  int NumElts = Mask.size();
  ScaledMask.assign(static_cast<size_t>(NumElts * Scale), -1);
  ....
}

Warnung PVS-Studio: V1028 [CWE-190] Möglicher Überlauf. ErwĂ€gen Sie, die Operanden des ‘NumElts * Scale’-Operators in den ‘size_t’-Typ zu casten, nicht das Ergebnis. X86ISelLowering.h 1577

Explizites Typcasting wird verwendet, um einen Überlauf bei der Multiplikation von Variablen vom Typ zu vermeiden. intHier schĂŒtzt das explizite Typcasting jedoch nicht vor einem Überlauf. ZunĂ€chst werden die Variablen multipliziert, und erst danach wird das 32-Bit-Ergebnis der Multiplikation auf den Typ size_t.

Fragment N35: Misslungene Copy-Paste

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;
  }
  ....
}

V778 [CWE-682] Zwei Àhnliche Codefragmente wurden gefunden. Möglicherweise handelt es sich um einen Tippfehler, und die Variable 'Op1' sollte anstelle von 'Op0' verwendet werden. InstCombineCompares.cpp 5507

Diese neue interessante Diagnose identifiziert Situationen, in denen ein Codefragment kopiert wurde und einige Namen geÀndert wurden, in einem Bereich jedoch nicht angepasst wurden.

Bitte beachten Sie, dass im zweiten Block Op0 findet man Op1geĂ€ndert wurde. Wahrscheinlich hĂ€tte es so geschrieben werden mĂŒssen:

if (!match(Op1, m_PosZeroFP()) && isKnownNeverNaN(Op1, &TLI)) {
  I.setOperand(1, ConstantFP::getNullValue(Op1->getType()));
  return &I;
}

Fragment N36: Verwirrung bei den Variablen

struct Status {
  unsigned Mask;
  unsigned Mode;

  Status() : Mask(0), Mode(0){};

  Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
    Mode &= Mask;
  };
  ....
};

Warnung PVS-Studio: V1001 [CWE-563] Die Variable 'Mode' wird zugewiesen, aber am Ende der Funktion nicht verwendet. SIModeRegister.cpp 48

Es ist sehr gefÀhrlich, den Funktionsargumenten dieselben Namen zu geben wie den Klassenmitgliedern. Es ist sehr leicht, sich zu verwirren. Dies ist genau der Fall. Dieser Ausdruck ist nicht sinnvoll:

Mode &= Mask;

Der Funktionsparameter wird verÀndert. Das war's. Dieser Parameter wird nicht weiter verwendet. Wahrscheinlich hÀtte man es so schreiben sollen:

Status(unsigned Mask, unsigned Mode) : Mask(Mask), Mode(Mode) {
  this->Mode &= Mask;
};

Fragment N37: Verwechslung der Variablen

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;
}

Warnung PVS-Studio: V1001 [CWE-563] Die Variable 'Size' wird zugewiesen, aber am Ende der Funktion nicht verwendet. Object.cpp 424

Die Situation ist Àhnlich wie zuvor. Es sollte wie folgt geschrieben werden:

this->Size += this->EntrySize;

Fragment N38-N47: Der Zeiger wurde vergessen zu ĂŒberprĂŒfen

FrĂŒher haben wir Beispiele fĂŒr das Auslösen der Diagnose betrachtet V595. Der Kern ist, dass der Zeiger zunĂ€chst dereferenziert und dann ĂŒberprĂŒft wird. Eine neue Diagnose V1004 ist semantisch ihr entgegengesetzt, deckt jedoch auch viele Fehler auf. Sie identifiziert Situationen, in denen der Zeiger zu Beginn ĂŒberprĂŒft, dann aber vergessen wurde. Betrachten wir solche FĂ€lle, die innerhalb von LLVM gefunden wurden.

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());  // <=
  ....
}

Warnung PVS-Studio: V1004 [CWE-476] Der 'Ptr'-Zeiger wurde unsicher verwendet, nachdem er gegen nullptr verifiziert wurde. ÜberprĂŒfen Sie die Zeilen: 729, 738. TargetTransformInfoImpl.h 738

Die Variable Ptr kann gleich sein nullptr, was durch die ÜberprĂŒfung belegt wird:

if (Ptr != nullptr)

Allerdings wird dieser Zeiger unten bereits ohne vorherige ÜberprĂŒ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(GD.getDecl());
  SmallVector ArgTypes;
  if (FD)                                                                // parameters())
      ArgTypes.push_back(Parm->getType());
  CallingConv CC = FD->getType()->castAs()->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 den Zeiger FD. Ich bin mir sicher, dass das Problem gut sichtbar ist und keine speziellen ErklÀrungen erforderlich sind.

Und noch:

static void computePolynomialFromPointer(Value &Ptr, Polynomial &Result,
                                         Value *&BasePtr,
                                         const DataLayout &DL) {
  PointerType *PtrTy = dyn_cast(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 kann man sich vor solchen Fehlern schĂŒtzen? Seien Sie aufmerksamer bei Code-Reviews und verwenden Sie regelmĂ€ĂŸig den statischen Code-Analyzer PVS-Studio zur ÜberprĂŒfung.

Es macht keinen Sinn, andere Codefragmente mit Fehlern dieser Art anzugeben. Ich lasse im Artikel nur die Liste der Warnungen stehen:

  • V1004 [CWE-476] Der ‚Expr‘-Zeiger wurde unsicher verwendet, nachdem er gegen 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. ÜberprĂŒfen Sie die Zeilen: 1819, 1824. Driver.cpp 1824

Fragment N48-N60: Nicht kritisch, aber ein Defekt (möglicher Speicherleck)

std::unique_ptr createISelMutator() {
  ....
  std::vector<std::unique_ptr> Strategies;
  Strategies.emplace_back(
      new InjectorIRStrategy(InjectorIRStrategy::getDefaultOps()));
  ....
}

Warnung PVS-Studio: V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode 'emplace_back' zum 'Strategies'-Container hinzugefĂŒgt. Bei einer Ausnahme tritt ein Speicherleck auf. llvm-isel-fuzzer.cpp 58

Um ein Element am Ende eines Containers vom Typ std::vector<std::unique_ptr> kann man nicht einfach schreiben xxx.push_back(new X), da es keine implizite Umwandlung von X* in std::unique_ptr.

Eine gÀngige Lösung besteht darin, zu schreiben xxx.emplace_back(new X), da es kompiliert: die Methode emplace_back stellt das Element direkt aus den Argumenten her und kann daher explizite Konstruktoren verwenden.

Das ist unsicher. Wenn der Vektor voll ist, erfolgt eine Speicherneuverteilung. Bei der Neuzuweisung kann es zu einem Fehler kommen, was zu einer Ausnahme fĂŒhrt. std::bad_alloc. In diesem Fall geht der Zeiger verloren und das erstellte Objekt wird niemals gelöscht.

Eine sichere Lösung besteht darin, unique_ptr, das den Zeiger besitzt, bevor der Vektor versucht, den Speicher neu zuzuweisen:

xxx.push_back(std::unique_ptr<X>(new X))

Seit C++14 kann 'std::make_unique' verwendet werden:

xxx.push_back(std::make_unique<X>())

Dieser Defekt ist fĂŒr LLVM nicht kritisch. Wenn der Speicher nicht zugewiesen werden kann, wird die Arbeit des Compilers einfach gestoppt. FĂŒr Anwendungen mit langem Uptime, die nicht einfach beendet werden können, wenn der Speicher nicht zugewiesen werden konnte, kann dies jedoch ein ernsthaftes Problem darstellen.

Obwohl dieser Code also keine praktische Gefahr fĂŒr LLVM darstellt, fand ich es nĂŒtzlich, ĂŒber dieses Fehlermuster zu berichten und dass der PVS-Studio-Analyser gelernt hat, es zu identifizieren.

Andere Warnungen dieses Typs:

  • V1023 [CWE-460] Ein Zeiger ohne EigentĂŒmer wird durch die Methode 'emplace_back' zum 'Passes'-Container hinzugefĂŒgt. Bei einer Ausnahme tritt ein Speicherleck auf. PassManager.h 546
  • V1023 [CWE-460] Ein Zeiger ohne EigentĂŒmer wird durch die Methode 'emplace_back' zum 'AAs'-Container hinzugefĂŒgt. Bei einer Ausnahme tritt ein Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck auf. SimpleLoopUnswitch.cpp 2012
  • V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode 'emplace_back' zum Container ‚Records‘ hinzugefĂŒgt. Im Falle einer Ausnahme tritt ein Speicherleck 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 Speicherleck 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 Speicherleck auf. DebugMap.cpp 88
  • V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode 'emplace_back' zum Container ‚Strategies‘ hinzugefĂŒgt. Im Falle einer Ausnahme tritt ein Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck 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 Speicherleck auf. llvm-stress.cpp 694
  • V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode ‚emplace_back‘ zum Container ‚Operands‘ hinzugefĂŒgt. Im Falle einer Ausnahme tritt ein Speicherleck auf. GlobalISelEmitter.cpp 1911
  • V1023 [CWE-460] Ein Zeiger ohne Besitzer wird durch die Methode ‚emplace_back‘ zum Container ‚Stash‘ hinzugefĂŒgt. Im Falle einer Ausnahme tritt ein Speicherleck 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 Speicherleck auf. GlobalISelEmitter.cpp 2702

Fazit

Insgesamt habe ich 60 Warnungen ausgegeben, danach habe ich aufgehört. Gibt es weitere Fehler, die der PVS-Studio-Analyzer in LLVM entdeckt? Ja, die gibt es. Allerdings war es spĂ€t am Abend, eher sogar Nacht, als ich die Codefragmente fĂŒr diesen Artikel aufschrieb, und ich entschied, dass es Zeit war, aufzuhören.

Ich hoffe, es war fĂŒr Sie interessant und Sie möchten den PVS-Studio-Analyzer ausprobieren.

Sie können den Analyzer herunterladen und einen LizenzschlĂŒssel erhalten auf auf dieser Seite.

Am wichtigsten ist, verwenden Sie die statische Analyse regelmĂ€ĂŸig. Einmalige PrĂŒfungen, die wir zum Zweck der Popularisierung der statischen Analysemethodik und von PVS-Studio durchfĂŒhren, sind kein normales Szenario.

Viel Erfolg bei der Verbesserung der QualitÀt und ZuverlÀssigkeit Ihres Codes!

Wir finden Bugs in LLVM 8 mit Hilfe des PVS-Studio-Analyzers.

Wenn Sie diesen Artikel mit einem englischsprachigen Publikum teilen möchten, verwenden Sie bitte den Link zur Übersetzung: Andrey Karpov. Finding Bugs in LLVM 8 with PVS-Studio.

Quelle: habr.com

Kaufen Sie zuverlĂ€ssiges Hosting fĂŒr Websites mit DDoS-Schutz, VPS VDS-Server đŸ”„ Kaufen Sie zuverlĂ€ssiges Hosting fĂŒr Websites mit DDoS-Schutz, VPS VDS-Server | ProHoster