Graph builder implementation.

[hctim]

Summary

Implement a localised graph builder for indirect control flow instructions. Main interface is through GraphBuilder::buildFlowGraph, which will build a flow graph around an indirect CF instruction. Various modifications to FileVerifier are also made to const-expose some members needed for machine code analysis done by the graph builder.

[hctim]

Removed comments from CMakeLists.txt

[hctim]

Update to only diff the prev in stack.

[hctim]

Fix various warnings.

[vlad.tsyrklevich]

I'm having difficulty following this because you document what the GraphBuilder methods do but you never say how and I get lost before I figure that out. Maybe that could go in the GraphBuilder.cpp file header comments?

tools/llvm-cfi-verify/GraphBuilder.cpp

1 ↗

(On Diff #117450)

Missing header

tools/llvm-cfi-verify/GraphBuilder.h

49 ↗

(On Diff #117450)

Do these need to be externally visible?

67 ↗

(On Diff #117450)

Should be a method of ConditionalBranchNode (also, prefer to accept a raw_ostream& instead of hardcoding outs() for print methods, dump() typically writes to dbgs() I think)

68 ↗

(On Diff #117450)

ditto

76 ↗

(On Diff #117450)

Why are these pointers?

[hctim]

Merged changes from previous in stack, updated for vlad.tsyrklevich's comments.

[hctim]

Merged wrong CL, putting this back to the way it should be.

[hctim]

Merged changes from upstream.

[hctim]

Updated to fix vlad's comments.

[hctim]

tools/llvm-cfi-verify/GraphBuilder.h

49 ↗

(On Diff #117450)

They're modified by the tests in order to guarantee behaviour, even if the default change.

76 ↗

(On Diff #117450)

Don't want to mix storage (stack/heap) of nodes. Intermediate nodes cannot be stored on stack, as they are neither a branch node or an orphaned node. Consider the following:

[0x0: jmp 2]
       \
  [0x2: jmpq %rax]

0x2 is neither a branch node, nor an orphaned node. The only reference to it is via 0x0, which is a branch node.

Having all these nodes be heap-allocated makes the clean up much easier.

[hctim]

Added header to GraphBuilder.cpp

[hctim]

Backported D38564

[hctim]

format was from prev in stack, put it down there.

[vlad.tsyrklevich]

tools/llvm-cfi-verify/GraphBuilder.cpp

232 ↗

(On Diff #117881)

Reword this.

293 ↗

(On Diff #117881)

Since the if() statement continue;s as the end, place the below without an else statement to avoid nesting (makes it a bit easier to read and understand.)

310 ↗

(On Diff #117881)

Reword 'fall back on itself'

313 ↗

(On Diff #117881)

Replace with std::find_if() ?

326 ↗

(On Diff #117881)

Why not use createParentNode() here?

335 ↗

(On Diff #117881)

Place the error first, and also avoid the nesting here as well.

tools/llvm-cfi-verify/GraphBuilder.h

53 ↗

(On Diff #117881)

just print(

54 ↗

(On Diff #117881)

just flatten(

62 ↗

(On Diff #117881)

just print(

66 ↗

(On Diff #117881)

instrument CFI protection?

104 ↗

(On Diff #117881)

Would be nice to have a comment here that briefly explains the approach (e.g. there are some bits that build the graph backwards and some that build it forwards)

76 ↗

(On Diff #117450)

Would it make sense to just place ConditionalBranchNodes in a vector without dynamically allocating them so they don't have to be manually freed?

[hctim]

Updated with Vlad's comments.

tools/llvm-cfi-verify/GraphBuilder.h

76 ↗

(On Diff #117450)

Yep, done :)

[hctim]

Backported usesRegisterOperand.

[hctim]

Merged updates from stack.

[vlad.tsyrklevich]

tools/llvm-cfi-verify/GraphBuilder.h

75 ↗

(On Diff #118255)

Could we represent this as (1) a uint64_t SrcAddr -> uint64_t DstAddr mapping and (2) a std::vector<uint64_t> of orphaned nodes? Then you would not need the Next pointers, and presumably FlowLength can simply be maintained as an argument to buildFlowGraphImpl(). You could maintain O(1) lookup by using llvm::DenseMap which is hash-table based and lighter-weight than std::map (see http://llvm.org/docs/ProgrammersManual.html#llvm-adt-densemap-h) and that seems cleaner to me than the current structure, WDYT?

[hctim]

Patched in previous changes to unit tests.

[hctim]

Made GraphResult's members automatic storage duration.

[vlad.tsyrklevich]

tools/llvm-cfi-verify/lib/GraphBuilder.cpp

96 ↗

(On Diff #118988)

Why not just pass this in?

108 ↗

(On Diff #118988)

s/chain/basic block/?

152 ↗

(On Diff #118988)

ditto s/chain/basic block/?

154 ↗

(On Diff #118988)

ditto

176 ↗

(On Diff #118988)

ditto

196 ↗

(On Diff #118988)

nit: eliminate braces

227 ↗

(On Diff #118988)

This is executed here, 240, 253, 260, and 270. Any reason it can't just be done once above? Is it to avoid the ConditionalBranchNode case?

tools/llvm-cfi-verify/lib/GraphBuilder.h

57 ↗

(On Diff #118988)

Could this be reworded? I'm still not sure what it means.

86 ↗

(On Diff #118988)

Sentence cuts off

94 ↗

(On Diff #118988)

fully evaluated, or enumerated? Not sure what evaluated might refer to in this context.

108 ↗

(On Diff #118988)

s/acyclic/cycle/

[vlad.tsyrklevich]

tools/llvm-cfi-verify/lib/GraphBuilder.h

62 ↗

(On Diff #118988)

type: posible

68 ↗

(On Diff #118988)

It's not all keys, just keys for all valid instructions in that range right?

unittests/tools/llvm-cfi-verify/GraphBuilder.cpp

60 ↗

(On Diff #118988)

Why template it just for std::vector?

131 ↗

(On Diff #118988)

This is never reached since an unhandled Error (e.g. at destruction time) cause an immediate failure.

453 ↗

(On Diff #118988)

???

+----------+     +--------------+
|    20    | <-- |      0       |
+----------+     +--------------+
  |                |
  |                |
  v                v
+----------+     +--------------+
|    21    |     |      2       |
+----------+     +--------------+
  |                |
  |                |
  v                v
+----------+     +--------------+
| 22 (ud2) |  +> |      7       |
+----------+  |  +--------------+
  ^           |    |
  |           |    |
  |           |    v
+----------+  |  +--------------+
|    4     |  |  |      8       |
+----------+  |  +--------------+
  |           |    |
  |           |    |
  v           |    v
+----------+  |  +--------------+     +------------+
|    6     | -+  | 9 (indirect) | <-- |     13     |
+----------+     +--------------+     +------------+
                   ^                    |
                   |                    |
                   |                    v
                 +--------------+     +------------+
                 |      11      |     | 15 (error) |
                 +--------------+     +------------+

[hctim]

Updated with Vlad's comments. Also patched the unit tests to succeed when x86 targets are not built, instead of not compiling.

tools/llvm-cfi-verify/lib/GraphBuilder.cpp

108 ↗

(On Diff #118988)

I've changed them all /s/chain/block. These blocks may contain direct unconditional branches as well :)

227 ↗

(On Diff #118988)

Yep, don't want to add a conditional branch to IntermediateNodes. I can't really see a good way to not repeat this LOC without introcuding a status variable which results in the same reuse.

tools/llvm-cfi-verify/lib/GraphBuilder.h

68 ↗

(On Diff #118988)

I think it's all keys, each key-value pair is built from the cross-references to the value. Invalid instructions (e.g. an indirect branch) will be an entry in OrphanedNodes, with a key-value pair between the invalid instruction -> its child.

unittests/tools/llvm-cfi-verify/GraphBuilder.cpp

60 ↗

(On Diff #118988)

Was initially using it to print IntermediateNodes (type: DenseMap) as well, now it's just printing vector's I'll remove the templating. Thanks!

453 ↗

(On Diff #118988)

It's beautiful!

[hctim]

Rebased over master. Forgot to add the unit test patches to this CL.

[vlad.tsyrklevich]

unittests/tools/llvm-cfi-verify/FileAnalysis.cpp

68 ↗

(On Diff #119725)

Could we (1) break the x86 test changes out into a follow-up CL and (2) check the error message to make sure we only skip tests for the specific error message we expect to see? I want to make sure we don't break something else in the future that accidentally disables unit tests without us realizing.

[hctim]

Patched out x86 test changes.

[skatkov]

This patch causes the same unittest compilation failure which was fixed in https://reviews.llvm.org/D39020.

Please fix it asap or revert the patch.

llvm/trunk/unittests/tools/llvm-cfi-verify/CMakeLists.txt

17 ↗

(On Diff #119928)

what is the reason you removed this line?

[skatkov]

Actually I took a liberty to submit https://reviews.llvm.org/rL316422 to unblock myself. Feel free to re-write it as you need but please ensure that it does not restore the issue.

tools/llvm-cfi-verify/FileAnalysis.h

public:
   const Instr &getInstructionOrDie(uint64_t Address) const;
 
   // Returns a pointer to the previous/next instruction in sequence,
   // respectively. Returns nullptr if the next/prev instruction doesn't exist,
   // or if the provided instruction doesn't exist.
   const Instr *getPrevInstructionSequential(const Instr &InstrMeta) const;
   const Instr *getNextInstructionSequential(const Instr &InstrMeta) const;
 
-  // Returns the list of indirect instructions.
+  // Returns whether this instruction is used by CFI to trap the program.
+  bool isCFITrap(const Instr &InstrMeta) const;
+
+  // Returns whether this function can fall through to the next instruction.
+  // Undefined (and bad) instructions cannot fall through, and instruction that
+  // modify the control flow can only fall through if they are conditional
+  // branches or calls.
+  bool canFallThrough(const Instr &InstrMeta) const;
+
+  // Returns the definitive next instruction. This is different from the next
+  // instruction sequentially as it will follow unconditional branches (assuming
+  // they can be resolved at compile time, i.e. not indirect). This method
+  // returns nullptr if the provided instruction does not transfer control flow
+  // to exactly one instruction that is known deterministically at compile time.
+  // Also returns nullptr if the deterministic target does not exist in this
+  // file.
+  const Instr *getDefiniteNextInstruction(const Instr &InstrMeta) const;
+
+  // Get a list of deterministic control flows that lead to the provided
+  // instruction. This list includes all static control flow cross-references as
+  // well as the previous instruction if it can fall through.
+  std::set<const Instr *> getControlFlowXRefs(const Instr &InstrMeta) const;
+
   const std::set<uint64_t> &getIndirectInstructions() const;
 
   const MCRegisterInfo *getRegisterInfo() const;
   const MCInstrInfo *getMCInstrInfo() const;
   const MCInstrAnalysis *getMCInstrAnalysis() const;
 
 protected:
   // Construct a blank object with the provided triple and features. Used in

tools/llvm-cfi-verify/FileAnalysis.cpp

 }
 
 const Instr &FileAnalysis::getInstructionOrDie(uint64_t Address) const {
   const auto &InstrKV = Instructions.find(Address);
   assert(InstrKV != Instructions.end() && "Address doesn't exist.");
   return InstrKV->second;
 }
 
+bool FileAnalysis::isCFITrap(const Instr &InstrMeta) const {
+  return MII->getName(InstrMeta.Instruction.getOpcode()) == "TRAP";
+}
+
+bool FileAnalysis::canFallThrough(const Instr &InstrMeta) const {
+  if (!InstrMeta.Valid)
+    return false;
+
+  if (isCFITrap(InstrMeta))
+    return false;
+
+  const auto &InstrDesc = MII->get(InstrMeta.Instruction.getOpcode());
+  if (InstrDesc.mayAffectControlFlow(InstrMeta.Instruction, *RegisterInfo))
+    return InstrDesc.isConditionalBranch();
+
+  return true;
+}
+
+const Instr *
+FileAnalysis::getDefiniteNextInstruction(const Instr &InstrMeta) const {
+  if (!InstrMeta.Valid)
+    return nullptr;
+
+  if (isCFITrap(InstrMeta))
+    return nullptr;
+
+  const auto &InstrDesc = MII->get(InstrMeta.Instruction.getOpcode());
+  if (InstrDesc.mayAffectControlFlow(InstrMeta.Instruction, *RegisterInfo)) {
+    if (InstrDesc.isConditionalBranch())
+      return nullptr;
+
+    uint64_t Target;
+    if (!MIA->evaluateBranch(InstrMeta.Instruction, InstrMeta.VMAddress,
+                             InstrMeta.InstructionSize, Target))
+      return nullptr;
+
+    const auto &NextKV = Instructions.find(Target);
+    if (NextKV == Instructions.end())
+      return nullptr;
+
+    if (!NextKV->second.Valid)
+      return nullptr;
+
+    return &NextKV->second;
+  }
+
+  const auto &NextKV =
+      Instructions.find(InstrMeta.VMAddress + InstrMeta.InstructionSize);
+  if (NextKV == Instructions.end())
+    return nullptr;
+
+  if (!NextKV->second.Valid)
+    return nullptr;
+
+  return &NextKV->second;
+}
+
+std::set<const Instr *>
+FileAnalysis::getControlFlowXRefs(const Instr &InstrMeta) const {
+  std::set<const Instr *> CFCrossReferences;
+  const Instr *PrevInstruction = getPrevInstructionSequential(InstrMeta);
+
+  if (PrevInstruction && canFallThrough(*PrevInstruction))
+    CFCrossReferences.insert(PrevInstruction);
+
+  const auto &TargetRefsKV = StaticBranchTargetings.find(InstrMeta.VMAddress);
+  if (TargetRefsKV == StaticBranchTargetings.end())
+    return CFCrossReferences;
+
+  for (uint64_t SourceInstrAddress : TargetRefsKV->second) {
+    const auto &SourceInstrKV = Instructions.find(SourceInstrAddress);
+    if (SourceInstrKV == Instructions.end()) {
+      errs() << "Failed to find source instruction at address "
+             << format_hex(SourceInstrAddress, 2)
+             << " for the cross-reference to instruction at address "
+             << format_hex(InstrMeta.VMAddress, 2) << ".\n";
+      continue;
+    }
+
+    CFCrossReferences.insert(&SourceInstrKV->second);
+  }
+
+  return CFCrossReferences;
+}
+
 const std::set<uint64_t> &FileAnalysis::getIndirectInstructions() const {
   return IndirectInstructions;
 }
 
 const MCRegisterInfo *FileAnalysis::getRegisterInfo() const {
   return RegisterInfo.get();
 }
 

tools/llvm-cfi-verify/unittests/FileAnalysis.cpp

 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/TargetSelect.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <cstdlib>
 
 using Instr = ::llvm::cfi_verify::FileAnalysis::Instr;
 using ::testing::Eq;
+using ::testing::Field;
 
 namespace llvm {
 namespace cfi_verify {
 namespace {
 class ELFx86TestFileAnalysis : public FileAnalysis {
 public:
   ELFx86TestFileAnalysis()
       : FileAnalysis(Triple("x86_64--"), SubtargetFeatures()) {}
TEST_F(BasicFileAnalysisTest, PrevAndNextFromBadInst) {
   EXPECT_EQ(1u, GoodInstrMeta->InstructionSize);
 
   GoodInstrMeta = Verifier.getNextInstructionSequential(BadInstrMeta);
   EXPECT_NE(nullptr, GoodInstrMeta);
   EXPECT_EQ(0xDEADBEEF + 2, GoodInstrMeta->VMAddress);
   EXPECT_EQ(1u, GoodInstrMeta->InstructionSize);
 }
 
+TEST_F(BasicFileAnalysisTest, CFITrapTest) {
+  Verifier.parseSectionContents(
+      {
+          0x90,                   // 0: nop
+          0xb0, 0x00,             // 1: mov $0x0, %al
+          0x48, 0x89, 0xe5,       // 3: mov %rsp, %rbp
+          0x48, 0x83, 0xec, 0x18, // 6: sub $0x18, %rsp
+          0x48, 0xbe, 0xc4, 0x07, 0x40,
+          0x00, 0x00, 0x00, 0x00, 0x00, // 10: movabs $0x4007c4, %rsi
+          0x2f,                         // 20: (bad)
+          0x41, 0x0e,                   // 21: rex.B (bad)
+          0x62, 0x72, 0x65, 0x61, 0x6b, // 23: (bad) {%k1}
+          0x0f, 0x0b                    // 28: ud2
+      },
+      0xDEADBEEF);
+
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 3)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 6)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 10)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 20)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 21)));
+  EXPECT_FALSE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 23)));
+  EXPECT_TRUE(Verifier.isCFITrap(
+      Verifier.getInstructionOrDie(0xDEADBEEF + 28)));
+}
+
+TEST_F(BasicFileAnalysisTest, FallThroughTest) {
+  Verifier.parseSectionContents(
+      {
+          0x90,                         // 0: nop
+          0xb0, 0x00,                   // 1: mov $0x0, %al
+          0x2f,                         // 3: (bad)
+          0x0f, 0x0b,                   // 4: ud2
+          0xff, 0x20,                   // 6: jmpq *(%rax)
+          0xeb, 0x00,                   // 8: jmp +0
+          0xe8, 0x45, 0xfe, 0xff, 0xff, // 10: callq [some loc]
+          0xff, 0x10,                   // 15: callq *(rax)
+          0x75, 0x00,                   // 17: jne +0
+          0xc3,                         // 19: retq
+      },
+      0xDEADBEEF);
+
+  EXPECT_TRUE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF)));
+  EXPECT_TRUE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 1)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 3)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 4)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 6)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 8)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 10)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 15)));
+  EXPECT_TRUE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 17)));
+  EXPECT_FALSE(
+      Verifier.canFallThrough(Verifier.getInstructionOrDie(0xDEADBEEF + 19)));
+}
+
+TEST_F(BasicFileAnalysisTest, DefiniteNextInstructionTest) {
+  Verifier.parseSectionContents(
+      {
+          0x90,                         // 0: nop
+          0xb0, 0x00,                   // 1: mov $0x0, %al
+          0x2f,                         // 3: (bad)
+          0x0f, 0x0b,                   // 4: ud2
+          0xff, 0x20,                   // 6: jmpq *(%rax)
+          0xeb, 0x00,                   // 8: jmp 10 [+0]
+          0xeb, 0x05,                   // 10: jmp 17 [+5]
+          0xe8, 0x00, 0x00, 0x00, 0x00, // 12: callq 17 [+0]
+          0xe8, 0x78, 0x56, 0x34, 0x12, // 17: callq 0x1234569f [+0x12345678]
+          0xe8, 0x04, 0x00, 0x00, 0x00, // 22: callq 31 [+4]
+          0xff, 0x10,                   // 27: callq *(rax)
+          0x75, 0x00,                   // 29: jne 31 [+0]
+          0x75, 0xe0,                   // 31: jne 1 [-32]
+          0xc3,                         // 33: retq
+          0xeb, 0xdd,                   // 34: jmp 1 [-35]
+          0xeb, 0xdd,                   // 36: jmp 3 [-35]
+          0xeb, 0xdc,                   // 38: jmp 4 [-36]
+      },
+      0xDEADBEEF);
+
+  const auto *Current = Verifier.getInstruction(0xDEADBEEF);
+  const auto *Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 1, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 1);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 3);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 4);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 6);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 8);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 10, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 10);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 17, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 12);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 17, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 17);
+  // Note, definite next instruction address is out of range and should fail.
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 22);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 31, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 27);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+  Current = Verifier.getInstruction(0xDEADBEEF + 29);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+  Current = Verifier.getInstruction(0xDEADBEEF + 31);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+  Current = Verifier.getInstruction(0xDEADBEEF + 33);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 34);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 1, Next->VMAddress);
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 36);
+  EXPECT_EQ(nullptr, Verifier.getDefiniteNextInstruction(*Current));
+
+  Current = Verifier.getInstruction(0xDEADBEEF + 38);
+  Next = Verifier.getDefiniteNextInstruction(*Current);
+  EXPECT_NE(nullptr, Next);
+  EXPECT_EQ(0xDEADBEEF + 4, Next->VMAddress);
+}
+
+TEST_F(BasicFileAnalysisTest, ControlFlowXRefsTest) {
+  Verifier.parseSectionContents(
+      {
+          0x90,                         // 0: nop
+          0xb0, 0x00,                   // 1: mov $0x0, %al
+          0x2f,                         // 3: (bad)
+          0x0f, 0x0b,                   // 4: ud2
+          0xff, 0x20,                   // 6: jmpq *(%rax)
+          0xeb, 0x00,                   // 8: jmp 10 [+0]
+          0xeb, 0x05,                   // 10: jmp 17 [+5]
+          0xe8, 0x00, 0x00, 0x00, 0x00, // 12: callq 17 [+0]
+          0xe8, 0x78, 0x56, 0x34, 0x12, // 17: callq 0x1234569f [+0x12345678]
+          0xe8, 0x04, 0x00, 0x00, 0x00, // 22: callq 31 [+4]
+          0xff, 0x10,                   // 27: callq *(rax)
+          0x75, 0x00,                   // 29: jne 31 [+0]
+          0x75, 0xe0,                   // 31: jne 1 [-32]
+          0xc3,                         // 33: retq
+          0xeb, 0xdd,                   // 34: jmp 1 [-35]
+          0xeb, 0xdd,                   // 36: jmp 3 [-35]
+          0xeb, 0xdc,                   // 38: jmp 4 [-36]
+      },
+      0xDEADBEEF);
+  const auto *InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF);
+  std::set<const Instr *> XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(XRefs.empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 1);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF)),
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 31)),
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 34))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 3);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 1)),
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 36))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 4);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 38))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 6);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 8);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 10);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 8))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 12);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 17);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 10)),
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 12))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 22);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 27);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 29);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 31);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 22)),
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 29))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 33);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_THAT(XRefs, UnorderedElementsAre(
+                         Field(&Instr::VMAddress, Eq(0xDEADBEEF + 31))));
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 34);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 36);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+
+  InstrMetaPtr = &Verifier.getInstructionOrDie(0xDEADBEEF + 38);
+  XRefs = Verifier.getControlFlowXRefs(*InstrMetaPtr);
+  EXPECT_TRUE(Verifier.getControlFlowXRefs(*InstrMetaPtr).empty());
+}
+
+
 } // anonymous namespace
 } // end namespace cfi_verify
 } // end namespace llvm
 
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);
   llvm::cl::ParseCommandLineOptions(argc, argv);
 
   llvm::InitializeAllTargetInfos();
   llvm::InitializeAllTargetMCs();
   llvm::InitializeAllAsmParsers();
   llvm::InitializeAllDisassemblers();
 
   return RUN_ALL_TESTS();
 }