Index: tools/llvm-cfi-verify/lib/FileAnalysis.h =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.h +++ tools/llvm-cfi-verify/lib/FileAnalysis.h @@ -44,8 +44,24 @@ namespace llvm { namespace cfi_verify { +struct GraphResult; + extern bool IgnoreDWARFFlag; +enum CFIProtectionStatus { + PROTECTED, // This instruction is protected by CFI. + FAIL_NOT_INDIRECT_CF, // The instruction is not an indirect control flow + // instruction, and thus shouldn't be protected. + FAIL_ORPHANS, // There is a path to the instruction that was unexpected. + FAIL_BAD_CONDITIONAL_BRANCH, // There is a path to the instruction from a + // conditional branch that does not properly + // check the destination for this vcall/icall. + UNKNOWN, // Something strange happened, e.g. the instruction was not in this + // file. +}; + +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 +85,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 +132,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 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 the inlining information for the provided address. + Expected symbolizeInlinedCode(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: tools/llvm-cfi-verify/lib/FileAnalysis.cpp =================================================================== --- tools/llvm-cfi-verify/lib/FileAnalysis.cpp +++ 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 PROTECTED: + return "PROTECTED"; + case FAIL_NOT_INDIRECT_CF: + return "FAIL_NOT_INDIRECT_CF"; + case FAIL_ORPHANS: + return "FAIL_ORPHANS"; + case FAIL_BAD_CONDITIONAL_BRANCH: + return "FAIL_BAD_CONDITIONAL_BRANCH"; + case UNKNOWN: + return "UNKNOWN"; + } + return "UNKNOWN"; +} + 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 UNKNOWN; + + const auto &InstrDesc = MII->get(InstrMetaPtr->Instruction.getOpcode()); + if (!InstrDesc.mayAffectControlFlow(InstrMetaPtr->Instruction, *RegisterInfo)) + return FAIL_NOT_INDIRECT_CF; + + if (!usesRegisterOperand(*InstrMetaPtr)) + return FAIL_NOT_INDIRECT_CF; + + if (!Graph.OrphanedNodes.empty()) + return FAIL_ORPHANS; + + for (const auto &BranchNode : Graph.ConditionalBranchNodes) { + if (!BranchNode.CFIProtection) + return FAIL_BAD_CONDITIONAL_BRANCH; + } + + return PROTECTED; +} Error FileAnalysis::initialiseDisassemblyMembers() { std::string TripleName = ObjectTriple.getTriple(); Index: tools/llvm-cfi-verify/llvm-cfi-verify.cpp =================================================================== --- tools/llvm-cfi-verify/llvm-cfi-verify.cpp +++ 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 == 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: unittests/tools/llvm-cfi-verify/FileAnalysis.cpp =================================================================== --- unittests/tools/llvm-cfi-verify/FileAnalysis.cpp +++ unittests/tools/llvm-cfi-verify/FileAnalysis.cpp @@ -50,6 +50,12 @@ ELFx86TestFileAnalysis() : FileAnalysis(Triple("x86_64--"), SubtargetFeatures()) {} + // Helper function for graph building/validation. + bool isCFIProtected(uint64_t Address) { + return validateCFIProtection(GraphBuilder::buildFlowGraph( + *this, Address)) == PROTECTED; + } + // Expose this method publicly for testing. void parseSectionContents(ArrayRef SectionBytes, uint64_t SectionAddress) { @@ -493,10 +499,11 @@ 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_FALSE(Analysis.isCFIProtected(0xDEADBEEF)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 1)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 3)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADC0DE)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicFallthroughToUd2) { @@ -509,7 +516,7 @@ 0xff, 0x10, // 4: callq *(%rax) }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + EXPECT_TRUE(Analysis.isCFIProtected(0xDEADBEEF + 4)); } TEST_F(BasicFileAnalysisTest, CFIProtectionBasicJumpToUd2) { @@ -522,7 +529,7 @@ 0x0f, 0x0b, // 4: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + EXPECT_TRUE(Analysis.isCFIProtected(0xDEADBEEF + 2)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathUd2) { @@ -538,7 +545,7 @@ 0x0f, 0x0b, // 9: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + EXPECT_TRUE(Analysis.isCFIProtected(0xDEADBEEF + 3)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualPathSingleUd2) { @@ -553,7 +560,7 @@ 0x0f, 0x0b, // 7: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 3)); + EXPECT_TRUE(Analysis.isCFIProtected(0xDEADBEEF + 3)); } TEST_F(BasicFileAnalysisTest, CFIProtectionDualFailLimitUpwards) { @@ -574,7 +581,7 @@ SearchLengthForConditionalBranch; SearchLengthForConditionalBranch = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 6)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 6)); SearchLengthForConditionalBranch = PrevSearchLengthForConditionalBranch; } @@ -596,7 +603,7 @@ uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 2; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 2)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 2)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -612,7 +619,7 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 4)); } TEST_F(BasicFileAnalysisTest, CFIProtectionWithUnconditionalJumpInFallthrough) { @@ -626,7 +633,7 @@ 0x0f, 0x0b, // 6: ud2 }, 0xDEADBEEF); - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 4)); + EXPECT_TRUE(Analysis.isCFIProtected(0xDEADBEEF + 4)); } TEST_F(BasicFileAnalysisTest, CFIProtectionComplexExample) { @@ -653,7 +660,7 @@ 0xDEADBEEF); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 5; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 9)); + EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 9)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -670,7 +677,7 @@ 0x688118); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x68811d)); + EXPECT_TRUE(Analysis.isCFIProtected(0x68811d)); SearchLengthForUndef = PrevSearchLengthForUndef; } @@ -699,11 +706,11 @@ 0x775e0e); uint64_t PrevSearchLengthForUndef = SearchLengthForUndef; SearchLengthForUndef = 1; - EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + EXPECT_FALSE(Analysis.isCFIProtected(0x775a68)); SearchLengthForUndef = 2; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + EXPECT_TRUE(Analysis.isCFIProtected(0x775a68)); SearchLengthForUndef = 3; - EXPECT_TRUE(Analysis.isIndirectInstructionCFIProtected(0x775a68)); + EXPECT_TRUE(Analysis.isCFIProtected(0x775a68)); SearchLengthForUndef = PrevSearchLengthForUndef; }