Index: llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h =================================================================== --- llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h +++ llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.h @@ -44,8 +44,29 @@ namespace llvm { namespace cfi_verify { +struct GraphResult; + extern bool IgnoreDWARFFlag; +enum class CFIProtectionStatus { + // This instruction is protected by CFI. + PROTECTED, + // The instruction is not an indirect control flow instruction, and thus + // shouldn't be protected. + FAIL_NOT_INDIRECT_CF, + // There is a path to the instruction that was unexpected. + FAIL_ORPHANS, + // There is a path to the instruction from a conditional branch that does not + // properly check the destination for this vcall/icall. + FAIL_BAD_CONDITIONAL_BRANCH, + // The instruction referenced does not exist. This normally indicates an + // error in the program, where you try and validate a graph that was created + // in a different FileAnalysis object. + FAIL_INVALID_INSTRUCTION, +}; + +StringRef stringCFIProtectionStatus(CFIProtectionStatus Status); + // Disassembler and analysis tool for machine code files. Keeps track of non- // sequential control flows, including indirect control flow instructions. class FileAnalysis { @@ -69,12 +90,6 @@ FileAnalysis(const FileAnalysis &) = delete; FileAnalysis(FileAnalysis &&Other) = default; - // Check whether the provided instruction is CFI protected in this file. - // Returns false if this instruction doesn't exist in this file, if it's not - // an indirect control flow instruction, or isn't CFI protected. Returns true - // otherwise. - bool isIndirectInstructionCFIProtected(uint64_t Address) const; - // Returns the instruction at the provided address. Returns nullptr if there // is no instruction at the provided address. const Instr *getInstruction(uint64_t Address) const; @@ -122,19 +137,13 @@ const MCRegisterInfo *getRegisterInfo() const; const MCInstrInfo *getMCInstrInfo() const; const MCInstrAnalysis *getMCInstrAnalysis() const; - symbolize::LLVMSymbolizer &getSymbolizer(); - // Returns true if this class is using DWARF line tables for elimination. - bool hasLineTableInfo() const; + // Returns the inlining information for the provided address. + Expected symbolizeInlinedCode(uint64_t Address); - // Returns the line table information for the range {Address +- - // DWARFSearchRange}. Returns an empty table if the address has no valid line - // table information, or this analysis object has DWARF handling disabled. - DILineInfoTable getLineInfoForAddressRange(uint64_t Address); - - // Returns whether the provided address has valid line information for - // instructions in the range of Address +- DWARFSearchRange. - bool hasValidLineInfoForAddressRange(uint64_t Address); + // Returns whether the provided Graph represents a protected indirect control + // flow instruction in this file. + CFIProtectionStatus validateCFIProtection(const GraphResult &Graph) const; protected: // Construct a blank object with the provided triple and features. Used in Index: llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp =================================================================== --- llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp +++ llvm/trunk/tools/llvm-cfi-verify/lib/FileAnalysis.cpp @@ -54,6 +54,22 @@ "will result in false positives for 'CFI unprotected' instructions."), cl::location(IgnoreDWARFFlag), cl::init(false)); +StringRef stringCFIProtectionStatus(CFIProtectionStatus Status) { + switch (Status) { + case CFIProtectionStatus::PROTECTED: + return "PROTECTED"; + case CFIProtectionStatus::FAIL_NOT_INDIRECT_CF: + return "FAIL_NOT_INDIRECT_CF"; + case CFIProtectionStatus::FAIL_ORPHANS: + return "FAIL_ORPHANS"; + case CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH: + return "FAIL_BAD_CONDITIONAL_BRANCH"; + case CFIProtectionStatus::FAIL_INVALID_INSTRUCTION: + return "FAIL_INVALID_INSTRUCTION"; + } + llvm_unreachable("Attempted to stringify an unknown enum value."); +} + Expected FileAnalysis::Create(StringRef Filename) { // Open the filename provided. Expected> BinaryOrErr = @@ -89,32 +105,6 @@ const SubtargetFeatures &Features) : ObjectTriple(ObjectTriple), Features(Features) {} -bool FileAnalysis::isIndirectInstructionCFIProtected(uint64_t Address) const { - const Instr *InstrMetaPtr = getInstruction(Address); - if (!InstrMetaPtr) - return false; - - const auto &InstrDesc = MII->get(InstrMetaPtr->Instruction.getOpcode()); - - if (!InstrDesc.mayAffectControlFlow(InstrMetaPtr->Instruction, *RegisterInfo)) - return false; - - if (!usesRegisterOperand(*InstrMetaPtr)) - return false; - - auto Flows = GraphBuilder::buildFlowGraph(*this, Address); - - if (!Flows.OrphanedNodes.empty()) - return false; - - for (const auto &BranchNode : Flows.ConditionalBranchNodes) { - if (!BranchNode.CFIProtection) - return false; - } - - return true; -} - const Instr * FileAnalysis::getPrevInstructionSequential(const Instr &InstrMeta) const { std::map::const_iterator KV = @@ -254,7 +244,34 @@ return MIA.get(); } -LLVMSymbolizer &FileAnalysis::getSymbolizer() { return *Symbolizer; } +Expected FileAnalysis::symbolizeInlinedCode(uint64_t Address) { + assert(Symbolizer != nullptr && "Symbolizer is invalid."); + return Symbolizer->symbolizeInlinedCode(Object->getFileName(), Address); +} + +CFIProtectionStatus +FileAnalysis::validateCFIProtection(const GraphResult &Graph) const { + const Instr *InstrMetaPtr = getInstruction(Graph.BaseAddress); + if (!InstrMetaPtr) + return CFIProtectionStatus::FAIL_INVALID_INSTRUCTION; + + const auto &InstrDesc = MII->get(InstrMetaPtr->Instruction.getOpcode()); + if (!InstrDesc.mayAffectControlFlow(InstrMetaPtr->Instruction, *RegisterInfo)) + return CFIProtectionStatus::FAIL_NOT_INDIRECT_CF; + + if (!usesRegisterOperand(*InstrMetaPtr)) + return CFIProtectionStatus::FAIL_NOT_INDIRECT_CF; + + if (!Graph.OrphanedNodes.empty()) + return CFIProtectionStatus::FAIL_ORPHANS; + + for (const auto &BranchNode : Graph.ConditionalBranchNodes) { + if (!BranchNode.CFIProtection) + return CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH; + } + + return CFIProtectionStatus::PROTECTED; +} Error FileAnalysis::initialiseDisassemblyMembers() { std::string TripleName = ObjectTriple.getTriple(); Index: llvm/trunk/tools/llvm-cfi-verify/llvm-cfi-verify.cpp =================================================================== --- llvm/trunk/tools/llvm-cfi-verify/llvm-cfi-verify.cpp +++ llvm/trunk/tools/llvm-cfi-verify/llvm-cfi-verify.cpp @@ -18,6 +18,7 @@ //===----------------------------------------------------------------------===// #include "lib/FileAnalysis.h" +#include "lib/GraphBuilder.h" #include "llvm/BinaryFormat/ELF.h" #include "llvm/Support/CommandLine.h" @@ -46,13 +47,15 @@ uint64_t ExpectedUnprotected = 0; uint64_t UnexpectedUnprotected = 0; - symbolize::LLVMSymbolizer &Symbolizer = Analysis.getSymbolizer(); std::map BlameCounter; for (uint64_t Address : Analysis.getIndirectInstructions()) { const auto &InstrMeta = Analysis.getInstructionOrDie(Address); + GraphResult Graph = GraphBuilder::buildFlowGraph(Analysis, Address); - bool CFIProtected = Analysis.isIndirectInstructionCFIProtected(Address); + CFIProtectionStatus ProtectionStatus = + Analysis.validateCFIProtection(Graph); + bool CFIProtected = (ProtectionStatus == CFIProtectionStatus::PROTECTED); if (CFIProtected) outs() << "P "; @@ -72,7 +75,7 @@ continue; } - auto InliningInfo = Symbolizer.symbolizeInlinedCode(InputFilename, Address); + auto InliningInfo = Analysis.symbolizeInlinedCode(Address); if (!InliningInfo || InliningInfo->getNumberOfFrames() == 0) { errs() << "Failed to symbolise " << format_hex(Address, 2) << " with line tables from " << InputFilename << "\n"; Index: llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp =================================================================== --- llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp +++ llvm/trunk/unittests/tools/llvm-cfi-verify/FileAnalysis.cpp @@ -493,10 +493,18 @@ 0x75, 0x00, // 3: jne 5 [+0] }, 0xDEADBEEF); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 1)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADC0DE)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF); + EXPECT_EQ(CFIProtectionStatus::FAIL_NOT_INDIRECT_CF, + Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 1); + EXPECT_EQ(CFIProtectionStatus::FAIL_NOT_INDIRECT_CF, + Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(CFIProtectionStatus::FAIL_NOT_INDIRECT_CF, + Analysis.validateCFIProtection(Result)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0x12345678); + EXPECT_EQ(CFIProtectionStatus::FAIL_INVALID_INSTRUCTION, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicFallthroughToUd2) { @@ -509,7 +517,9 @@ 0xff, 0x10, // 4: callq *(%rax) }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicJumpToUd2) { @@ -522,7 +532,9 @@ 0x0f, 0x0b, // 4: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 2); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathUd2) { @@ -538,7 +550,9 @@ 0x0f, 0x0b, // 9: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathSingleUd2) { @@ -553,7 +567,9 @@ 0x0f, 0x0b, // 7: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 3); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualFailLimitUpwards) { @@ -574,7 +590,9 @@ SearchLengthForConditionalBranch; SearchLengthForConditionalBranch = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 6)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 6); + EXPECT_EQ(CFIProtectionStatus::FAIL_ORPHANS, + Analysis.validateCFIProtection(Result)); SearchLengthForConditionalBranch = PrevSearchLengthForConditionalBranch; } @@ -596,7 +614,9 @@ uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 2); + EXPECT_EQ(CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -612,7 +632,9 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(CFIProtectionStatus::FAIL_ORPHANS, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionWithUnconditionalJumpInFallthrough) { @@ -626,7 +648,9 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 4); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); } TEST_F(BasicFileAnalysisTest, CFIProtectionComplexExample) { @@ -653,7 +677,9 @@ 0xDEADBEEF); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 5; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 9)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0xDEADBEEF + 9); + EXPECT_EQ(CFIProtectionStatus::FAIL_ORPHANS, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -670,7 +696,9 @@ 0x688118); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x68811d)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0x68811d); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -699,11 +727,17 @@ 0x775e0e); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + GraphResult Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(CFIProtectionStatus::FAIL_BAD_CONDITIONAL_BRANCH, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = 2; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = 3; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + Result = GraphBuilder::buildFlowGraph(Analysis, 0x775a68); + EXPECT_EQ(CFIProtectionStatus::PROTECTED, + Analysis.validateCFIProtection(Result)); SearchLengthForUndef = PrevSearchLengthForUndef; }