[PATCH] D38427: Graph builder implementation.

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: tools/llvm-cfi-verify/GraphBuilder.h:49
+
+extern uint64_t SearchLengthForUndef;
+extern uint64_t SearchLengthForConditionalBranch;
----------------
vlad.tsyrklevich wrote:
> Do these need to be externally visible?
They're modified by the tests in order to guarantee behaviour, even if the default change.


================
Comment at: tools/llvm-cfi-verify/GraphBuilder.h:76
+  uint64_t BaseInstructionAddress;
+  std::vector<ConditionalBranchNode *> BranchNodes;
+  std::vector<ControlFlowNode *> OrphanedNodes;
----------------
vlad.tsyrklevich wrote:
> Why are these pointers?
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.


https://reviews.llvm.org/D38427