Geekly Articles each Day

PVS-Studioアナライザーを使用してLLVM 8のバグを見つける

PVS-StudioおよびLLVM 8.0.0

PVS-Studioアナライザーを使用してLLVMプロジェクトコードを最後に検証してから2年以上が経過しました。 PVS-Studioアナライザーが、エラーや潜在的な脆弱性を検出するための主要なツールであることを確認しましょう。 これを行うには、LLVM 8.0.0リリースの新しいエラーを確認して見つけてください。

書かれる記事


正直に言うと、私はこの記事を書きたくありませんでした。 すでに繰り返しテストしたプロジェクトについて書くのは面白くありません（ 1、2、3 ）。 新しいことについて書くほうがいいのですが、私には選択肢がありません。

LLVMの新しいバージョンがリリースされるか、 Clang Static Analyzerが更新されるたびに、次のタイプの質問がメールに表示されます。

見て、Clang Static Analyzerの新しいバージョンは新しいバグを見つけることを学んだ！ PVS-Studioを使用することの関連性は低下しているように思えます。 Clangは以前よりも多くのエラーを検出し、PVS-Studioの機能に追いつきます。 これについてどう思いますか？

私はいつもこのような何かに答えたいです：

私たちも遊んでいるわけではありません！ PVS-Studioアナライザーの機能が大幅に改善されました。 心配しないでください、私たちは以前と同様にリードし続けます。

残念ながら、これは悪い答えです。 証拠はありません。 それが、私が今この記事を書いている理由です。 そのため、LLVMプロジェクトは再びテストされ、さまざまなエラーが発見されました。 私にとって面白そうに思えたものを、今からデモンストレーションします。 これらのエラーはClang Static Analyzerで見つけることができません（または、それを行うのは非常に不便です）。 そして、できる。 そして、ある夜にこれらのエラーをすべて見つけて書きました。

しかし、記事の執筆は数週間続いた。 これをすべてテキスト形式でアレンジすることはできませんでした:)。

ところで、エラーや潜在的な脆弱性を検出するためにPVS-Studioアナライザーで使用されているテクノロジーに興味がある場合は、このノートをよく理解することをお勧めします。

新旧の診断


すでに述べたように、約2年前に、LLVMプロジェクトが再度チェックされ、見つかったエラーが修正されました。 この記事では、エラーの新しい部分を紹介します。 なぜ新しいバグが見つかったのですか？ これには3つの理由があります。

  1. LLVMプロジェクトが開発され、古いコードが変更され、新しいコードが表示されます。 当然、変更および記述されたコードには新しいエラーがあります。 これは、静的解析を定期的に適用する必要があることをよく示しており、場合によっては適用しません。 私たちの記事はPVS-Studioアナライザーの機能をよく示していますが、これはコードの品質を改善し、エラーを修正するコストを削減することとは何の関係もありません。 静的コードアナライザーを定期的に使用してください！
  2. 私たちは、既存の診断を完成させ、改善しています。 したがって、アナライザは、以前のチェックで気付かなかったエラーを検出できます。
  3. PVS-Studioには、2年前ではなかった新しい診断が登場しました。 PVS-Studioの開発を明確に示すために、それらを別のセクションに分けることにしました。

2年前に存在した診断で特定された欠陥


フラグメントN1：コピーペースト

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警告： V501 [CWE-570]同一の副次式「Name.startswith（ "avx512.mask.permvar。"）」があります。「||」の左と右に 演算子。 AutoUpgrade.cpp 73

名前が部分文字列「avx512.mask.permvar。」で始まることを再確認します。 2番目のテストでは、明らかに他の何かを書きたいと思っていましたが、コピーしたテキストを修正するのを忘れていました。

フラグメントN2：タイプミス

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

PVS-Studio警告：V501「|」の左側と右側に同一のサブ式「CXNameRange_WantQualifier」があります 演算子。 CIndex.cpp 7245

タイプミスのため、同じ名前の定数CXNameRange_WantQualifierが2回使用されます。

スニペットN3：オペレーターの優先順位に関する混乱

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

PVS-Studio警告： V502 [CWE-783]「？：」演算子は、予想とは異なる方法で動作する可能性があります。 「？：」演算子の優先順位は、「==」演算子よりも低くなっています。 PPCTargetTransformInfo.cpp 404

私の意見では、これは非常に美しい間違いです。 はい、私は美について奇妙なアイデアを持っていることを知っています:)。

ここで、演算子の優先順位に従って、式は次のように計算されます。

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

実用的な観点から、この条件は次のように減らすことができるため、意味がありません。

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

これは明らかな間違いです。 おそらく、0/1は変数Indexと比較したかったのです。 コードを修正するには、三項演算子の周りに括弧を追加します。

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

ところで、三項演算子は非常に危険であり、論理エラーを引き起こします。 細心の注意を払い、括弧を付けることを欲しないでください。 このトピックについては、「演算子を恐れる？：括弧で囲む」の章で詳しく説明しました。

フラグメントN4、N5：ヌルポインター

 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警告： V522 [CWE-476]ヌルポインター「LHS」の逆参照が行われる場合があります。 TGParser.cpp 2152

LHSポインターがヌルの場合、警告が発行される必要があります。 ただし、代わりに、このNULLポインター自体の逆参照が発生します： LHS-> getAsString（） 。

これは、誰もエラーをテストしていないため、エラーがエラーハンドラーに隠されている非常に一般的な状況です。 静的アナライザーは、使用頻度に関係なく、到達可能なすべてのコードをチェックします。 これは、静的分析が他のテストおよびエラー保護技術を補完する方法の非常に良い例です。

RHSポインターを処理する同様のエラーが、コードの少し下で作成されました。V522[CWE-476] NULLポインター「RHS」の逆参照が行われる場合があります。 TGParser.cpp 2186

フラグメントN6：移動後にカーソルを使用する

 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警告：V522 [CWE-476]ヌルポインター「ProgClone」の逆参照が行われる場合があります。 Miscompilation.cpp 601

最初に、 ProgCloneスマートポインターはオブジェクトの所有を停止します。

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

実際、 ProgCloneはヌルポインターになりました。 したがって、nullポインターの逆参照は、すぐ下で発生する必要があります。

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

しかし、実際には、これは起こりません！ ループは実際には実行されていないことに注意してください。

最初に、 MiscompiledFunctionsコンテナーがクリアされます。

 MiscompiledFunctions.clear();

次に、このコンテナーのサイズがループ条件で使用されます。

 for (unsigned i = 0, e = MisCompFunctions.size(); i != e; ++i) {

ループが開始されないことは簡単にわかります。 これも間違いだと思うので、コードの書き方を変える必要があります。

非常に有名なエラーのパリティに出会ったようです！ 1つの間違いが別の間違いを隠します:)。

フラグメントN7：移動後にカーソルを使用する

 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警告：V522 [CWE-476]ヌルポインター「テスト」の逆参照が行われる場合があります。 Miscompilation.cpp 709

再び同じ状況。 最初は、オブジェクトの内容が移動され、何も起こらなかったように使用されます。 C ++で動作のセマンティクスが登場した後、プログラムコードでこの状況をますます目にしま​​す。 このために私はC ++言語が大好きです！ 自分の足を撃つ新しい方法が増えています。 PVS-Studioアナライザーは常に動作します:)。

フラグメントN8：ヌルポインター

 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警告：V522 [CWE-476]ヌルポインター「タイプ」の逆参照が行われる場合があります。 PrettyFunctionDumper.cpp 233

通常、エラーハンドラーに加えて、印刷出力をデバッグするための関数はテストされません。 私たちの前にそのような場合です。 この関数は、ユーザーの問題を解決する代わりに修正を強制されるユーザーを待っています。

正しく：

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

フラグメントN9：ヌルポインター

 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警告：V522 [CWE-476]ヌルポインター「Ty」の逆参照が行われる場合があります。 SearchableTableEmitter.cpp 614

私は思うので、すべてが明確であり、説明を必要としません。

フラグメントN10：タイプミス

 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警告： V570 「識別子->タイプ」変数はそれ自体に割り当てられます。 FormatTokenLexer.cpp 249

変数をそれ自体に割り当てることは意味がありません。 おそらく彼らは書きたいと思っていた：

 Identifier->Type = Question->Type;

フラグメントN11：不審なブレーク
 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警告： V622 [CWE-478]「switch」ステートメントの検査を検討してください。 最初の「ケース」演算子が欠落している可能性があります。 SystemZAsmParser.cpp 652

最初に非常に疑わしいブレークステートメントがあります。 ここに何か他のものを書くのを忘れましたか？

フラグメントN12：参照解除後のポインターの確認

 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警告： V595 [CWE-476] nullptrに対して検証される前に、「 呼び出し先 」ポインターが使用されました。 行を確認：172、174。AMDGPUInline.cpp 172

getTTI関数が呼び出されると 、 先頭のCalleeポインターが逆参照されます。

そして、このポインターはnullptrの等価性をチェックする必要があることがわかりました 。

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

しかし、手遅れです...

フラグメントN13-N ...：参照解除後のポインターの確認

前のコードスニペットで説明した状況は一意ではありません。 彼女はここにいます：

 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警告：V595 [CWE-476] nullptrに対して検証される前に、「CalleeFn」ポインターが使用されました。 行を確認してください：1079、1081。SimplifyLibCalls.cpp 1079

そしてここに：

 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警告：V595 [CWE-476] nullptrに対して検証される前に、「ND」ポインターが使用されました。 行を確認してください：532、534。SemaTemplateInstantiateDecl.cpp 532

そしてここに：


そして、私は番号V595の警告を研究することに興味がありませんでした。 したがって、ここにリストされているエラー以外に同様のエラーがあるかどうかはわかりません。 おそらくあります。

フラグメントN17、N18：疑わしいシフト

 static inline bool processLogicalImmediate(uint64_t Imm, unsigned RegSize, uint64_t &Encoding) { .... unsigned Size = RegSize; .... uint64_t NImms = ~(Size-1) << 1; .... }

PVS-Studio警告： V629 [CWE-190]「〜（サイズ-1）<< 1」式の検査を検討してください。 32ビット値のビットシフトと、それに続く64ビットタイプへの拡張。 AArch64AddressingModes.h 260

おそらくこれは間違いではなく、コードは意図したとおりに機能します。 しかし、これは明らかに非常に疑わしい場所であり、確認する必要があります。

Size変数が16であり、コードの作成者がNImms変数の値を取得する予定であるとします。

11111111111111111111111111111111111111111111111111111111111111111100000000

ただし、実際には、結果は次のとおりです。

0000000000000000000000000000000000001111111111111111111111111111100000

実際、すべての計算は32ビット符号なし型を使用して行われます。 そして、この32ビットの符号なしの型は、暗黙的にuint64_tに拡張されます 。 この場合、最上位ビットはゼロになります。

次のような状況を修正できます。

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

同様の状況：V629 [CWE-190]「Immr << 6」式の検査を検討してください。 32ビット値のビットシフトと、それに続く64ビットタイプへの拡張。 AArch64AddressingModes.h 269

Snippet N19： else キーワードがありませ ん か？

 void AMDGPUAsmParser::cvtDPP(MCInst &Inst, const OperandVector &Operands) { .... if (Op.isReg() && Op.Reg.RegNo == AMDGPU::VCC) { // VOP2b (v_add_u32, v_sub_u32 ...) dpp use "vcc" token. // Skip it. continue; } if (isRegOrImmWithInputMods(Desc, Inst.getNumOperands())) { // <= Op.addRegWithFPInputModsOperands(Inst, 2); } else if (Op.isDPPCtrl()) { Op.addImmOperands(Inst, 1); } else if (Op.isImm()) { // Handle optional arguments OptionalIdx[Op.getImmTy()] = I; } else { llvm_unreachable("Invalid operand type"); } .... }

PVS-Studio警告： V646 [CWE-670]アプリケーションのロジックの検査を検討してください。 「else」キーワードが欠落している可能性があります。 AMDGPUAsmParser.cpp 5655

ここに間違いはありません。 最初のifのthenブロックがcontinueで終わるため、 elseキーワードがあるかどうかは関係ありません。 いずれにしても、コードは同じように機能します。 ただし、 他をスキップすると、コードがより不明瞭で危険になります。 将来、 継続が消えると、コードはまったく異なる方法で動作を開始します。 私の意見では、 elseを追加した方が良いでしょう。

フラグメントN20：同じタイプの4つのタイプミス

 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の警告：


偶然、+ =演算子の代わりに+演算子が使用されます。 結果は無意味なデザインです。

フラグメントN21：未定義の動作

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

自分で危険なコードを見つけてください。 そして、これは気を散らすものであり、すぐに答えを見ないようにしています：

うーん...


PVS-Studio警告： V708 [CWE-758]危険な構造が使用されています： 'FeaturesMap [Op] = FeaturesMap.size（）'、 'FeaturesMap'は 'map'クラスです。 これにより、未定義の動作が発生する場合があります。 RISCVCompressInstEmitter.cpp 490

問題行：

 FeaturesMap[Op] = FeaturesMap.size();

Op要素が見つからない場合、新しい要素がマップに作成され、このマップの要素の数がそこに書き込まれます。 サイズ要素が新しい要素を追加する前に呼び出されるか、後に呼び出されるかは不明です。

フラグメントN22-N24：再割り当て

 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警告： V519 [CWE-563]「NType」変数には連続して2回値が割り当てられます。 おそらくこれは間違いです。 行をチェックします：1663、1664。MachOObjectFile.cpp 1664

ここに本当の間違いはないと思います。 無駄な繰り返し割り当て。 しかし、それでも失敗です。

同様に：


フラグメントN25-N27：追加の再割り当て

次に、わずかに異なる再割り当てオプションを検討します。

 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警告：V519 [CWE-563] 'Alignment'変数には、値が連続して2回割り当てられます。 おそらくこれは間違いです。 行を確認してください：1158、1160。LoadStoreVectorizer.cpp 1160

これは非常に奇妙なコードで、論理エラーが含まれているようです。 最初に、 Alignment変数には、条件に応じて値が割り当てられます。 そして、割り当てが再び行われますが、現在は検証が行われていません。

同様の状況はここで見ることができます：


フラグメントN28：常に真の状態

 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 警告 ： V547 [CWE-571]式 'nextByte！= 0x90'は常にtrueです。 X86DisassemblerDecoder.cpp 379

検証は意味がありません。 nextByte変数は常に0x90と等しくありません。これは前のチェックの結果です。 これはある種の論理的な間違いです。

フラグメントN29-N ...：常に真/偽の条件

アナライザーは、条件全体（ V547 ）またはその一部（ V560 ）が常にtrueまたはfalseであるという多くの警告を出します。 多くの場合、これらは本当の間違いではなく、単にずさんなコード、マクロなどの展開の結果です。 それにもかかわらず、実際の論理エラーが時々あるので、これらの警告をすべて見るのは理にかなっています。 たとえば、次のコードは疑わしいものです。

 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警告： V560 [CWE-570]条件式の一部が常にfalseです：RegNo == 0xe。 ARMDisassembler.cpp 939

定数0xEは、10進法の値14です。 RegNo == 0xeをチェックしても意味がありません。RegNo> 13の場合、関数は実行を完了するためです。

識別子V547およびV560には他にも多くの警告がありましたが、V595と同様に、これらの警告を調べることに興味はありませんでした。 記事を書くのに十分な資料があることはすでに明らかでした:)。 したがって、PVS-Studioを使用してLLVMでこのタイプのエラーをいくつ検出できるかは不明です。

これらの反応を研究するのが退屈な理由の例を挙げます。 アナライザーは、次のコードに警告を発行するのに絶対に正しいです。 しかし、これは間違いではありません。

 bool UnwrappedLineParser::parseBracedList(bool ContinueOnSemicolons, tok::TokenKind ClosingBraceKind) { bool HasError = false; .... HasError = true; if (!ContinueOnSemicolons) return !HasError; .... }

PVS-Studio警告：V547 [CWE-570]式 '！HasError'は常にfalseです。 UnwrappedLineParser.cpp 1635

フラグメントN30：不審な帰還

 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警告： V612 [CWE-670]ループ内の無条件の「戻り」。 R600OptimizeVectorRegisters.cpp 63

これは間違いか、コードを読んでいるプログラマーに何かを説明することを目的とした特定の手法です。 このデザインでは何も説明されず、非常に疑わしいように見えます。 そのように書かない方が良いです:)。

疲れた？ 次に、お茶やコーヒーを作る時間です。

コーヒー


新しい診断で検出された欠陥


古い診断の30回のトリガーで十分だと思います。 興味深いのは、 前回のチェック後にアナライザーに表示された新しい診断でわかることです。 この間に、合計66個の汎用診断がC ++アナライザーに追加されました。

フラグメントN31：到達不能コード

 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警告： V779 [CWE-561]到達不能コードが検出されました。 エラーが存在する可能性があります。 ExecutionUtils.cpp 146

ご覧のとおり、 ifステートメントの両方のブランチはreturnステートメントの呼び出しで終了しています。 したがって、 CtorDtorsByPriorityコンテナーが空になることはありません。

スニペットN32：到達不能コード

 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警告：V779 [CWE-561]到達不能コードが検出されました。 エラーが存在する可能性があります。 LLParser.cpp 835

興味深い状況。 この場所の始まりを見てみましょう。

 return ParseTypeIdEntry(SummaryID); break;

一見、間違いがないようです。 ここではbreakステートメントは不要なようで、単純に削除できます。 ただし、すべてがそれほど単純ではありません。

アナライザーは次の行に警告を生成します。

 Lex.setIgnoreColonInIdentifiers(false); return false;

実際、このコードは到達不能です。 switchのすべてのケースは、 returnステートメントの呼び出しで終了します。 そして今、無意味な孤独な休憩はそれほど無害に見えません！ おそらくブランチの1つはreturnではなくbreakで終わるべきでしょうか？

フラグメントN33：高ビットの偶発化

 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警告： V784ビットマスクのサイズは、第1オペランドのサイズより小さくなっています。 これにより、上位ビットが失われます。 RuntimeDyld.cpp 815

getStubAlignment関数は符号なしの型を返すことに注意してください。 関数が値8を返すと仮定した場合、式の値を計算します。

〜（getStubAlignment（）-1）

〜（8u-1）

0xFFFFFFF8u

ここで、 DataSize変数は64ビットの符号なし型であることに注意してください。 DataSize＆0xFFFFFFF88の操作中に、32個すべての上位ビットがリセットされることがわかりました。 おそらく、これはプログラマが望んでいたものではありません。 彼は、DataSize＆0xFFFFFFFFFFFFFFFFF8uを計算したかったのではないかと思います。

エラーを修正するには、次のように記述する必要があります。

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

または：

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

フラグメントN34：明示的なキャストに失敗しました

 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警告： V1028 [CWE-190]オーバーフローの可能性。 「NumElts * Scale」演算子のオペランドを、結果ではなく「size_t」型にキャストすることを検討してください。 X86ISelLowering.h 1577

明示的な型変換は、 int型の変数を乗算するときにオーバーフローを防ぐために使用されます。 ただし、ここでの明示的なキャストでは、オーバーフローを防止できません。 最初に変数が乗算され、32ビットの乗算結果がsize_t型に展開されます。

フラグメントN35：コピーペーストの失敗

 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]同様の2つのコードフラグメントが見つかりました。おそらく、これはタイプミスであり、「Op0」の代わりに「Op1」変数を使用する必要があります。InstCombineCompares.cpp 5507

この新しい興味深い診断により、コードフラグメントがコピーされ、一部の名前が変更され始めたが、一箇所で修正されなかった状況が明らかになります。

2番目のブロックでOp0をOp1に変更したことに注意してください。しかし、彼らは一箇所でそれを修正しませんでした。ほとんどの場合、次のように書かれているはずです。

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

フラグメントN36：変数の混乱

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

PVS-Studio警告：V1001 [CWE-563]「モード」変数が割り当てられていますが、関数の最後まで使用されていません。SIModeRegister.cpp 48

関数の引数にクラスメンバーと同じ名前を付けることは非常に危険です。混乱しやすい。私たちの前にそのような場合です。この式は意味がありません：

 Mode &= Mask;

関数の引数が変更されます。 それだけです。この引数は使用されなくなりました。ほとんどの場合、次のように記述する必要がありました。

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

フラグメントN37：変数の混乱

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

PVS-Studio警告：V1001 [CWE-563]「サイズ」変数が割り当てられていますが、関数の最後まで使用されていません。Object.cpp 424

この状況は前の状況と似ています。それは書かれるべきです：

 this->Size += this->EntrySize;

フラグメントN38-N47：ポインターがチェックするのを忘れていた

以前、V595診断トリガーの例を調べました。その本質は、ポインターが最初に間接参照され、その後のみチェックされることです。V1004の若い診断はその意味の逆ですが、多くのエラーも検出します。ポインタが最初にチェックされた状況を特定し、それを忘れていました。LLVM内で見つかったこのようなケースを検討してください。

 int getGEPCost(Type *PointeeType, const Value *Ptr, ArrayRef<const Value *> Operands) { .... if (Ptr != nullptr) { // <= assert(....); BaseGV = dyn_cast<GlobalValue>(Ptr->stripPointerCasts()); } bool HasBaseReg = (BaseGV == nullptr); auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType()); // <= .... }

PVS-Studio警告：V1004 [CWE-476] nullptrに対して検証された後、「Ptr」ポインターが安全に使用されませんでした。チェック行：729、738。TargetTransformInfoImpl.h 738 チェックで明らかなように、Ptr

変数はnullptrにすることができます。

 if (Ptr != nullptr)

ただし、以下のポインターは事前検証なしで逆参照されます。

 auto PtrSizeBits = DL.getPointerTypeSizeInBits(Ptr->getType());

別の同様のケースを検討してください。

 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警告：V1004 [CWE-476] nullptrに対して検証された後、「FD」ポインターが安全に使用されませんでした。行を確認してください：3228、3231。CGDebugInfo.cpp 3231

ポインターFDに注意してください。問題ははっきりと見えるので、特別な説明は必要ありません。

そしてまた：

 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警告：V1004 [CWE-476] nullptrに対して検証された後、「PtrTy」ポインターが安全に使用されませんでした。行を確認してください：960、965。InterleavedLoadCombinePass.cpp 965

このようなエラーから身を守る方法は？コードレビューに注意し、PVS-Studio静的アナライザーを使用してコードを定期的にチェックしてください。

このタイプのエラーを持つ他のコードフラグメントを持ち込むことは意味がありません。記事には警告のリストのみを残します。


N48-N60: , ( )

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

PVS-Studio警告：V1023 [CWE-460]「emplace_back」メソッドにより、所有者のないポインターが「Strategies」コンテナーに追加されます。例外が発生すると、メモリリークが発生します。 llvm-isel-fuzzer.cpp 58 std :: vector <std :: unique_ptr <X >>の

ようなコンテナの末尾にアイテムを追加するには、X *からstdへの暗黙的な変換がないため、xxx.push_back（新しいX）だけを書くことはできません。 ：unique_ptr <X>。一般的な解決策は、xxx.emplace_back（新しいX）をコンパイルすることです。emplace_backメソッドは引数から直接要素を構築するため、明示的なコンストラクターを使用できます。



これは安全ではありません。ベクトルがいっぱいの場合、メモリが割り当てられます。メモリの再割り当て操作が失敗し、std :: bad_alloc例外がスローされる可能性があります。この場合、ポインターは失われ、作成されたオブジェクトは削除されません。

安全な解決策はunique_ptrを作成することです。これは、ベクターがメモリを再割り当てする前にポインターを所有します。

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

C ++ 14以降では、「std :: make_unique」を使用できます。

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

このタイプの欠陥は、LLVMにとって重要ではありません。メモリを割り当てることができない場合、コンパイラは単に動作を停止します。ただし、メモリの割り当てに失敗した場合に終了できないアップタイムの長いアプリケーションの場合、これは非常に厄介な間違いです。

そのため、このコードはLLVMに実際的な危険をもたらすことはありませんが、このエラーパターンと、PVS-Studioアナライザーがそれを検出することを学んだことは有益であることがわかりました。

このタイプの他の警告：


おわりに


合計で、私は60個の警告を書き、その後停止しました。PVS-StudioアナライザーがLLVMで検出する他の欠陥はありますか？はい、あります。しかし、この記事のコードスニペットを作成したとき、それは夕方遅く、あるいは夜でさえあったので、締めくくる時間であると判断しました。

興味をお持ちで、PVS-Studioアナライザーを試してみてください。このページで

アナライザーをダウンロードして、トライアルキーを取得できます。

最も重要なことは、静的分析を定期的に使用することです。静的分析とPVS-Studioの方法論を普及させるために私たちが行った1回限りのチェックは、通常のシナリオではありません。

コードの品質と信頼性を向上させてください！



この記事を英語圏の聴衆と共有したい場合は、翻訳へのリンクを使用してください：Andrey Karpov。 PVS-Studioを使用したLLVM 8のバグの発見。

Source: https://habr.com/ru/post/J450008/

More articles:


All Articles