[PATCH] D38428: Add FileVerifier::isCFIProtected().

[Mitch Phillips via Phabricator via llvm-commits]

hctim added inline comments.


================
Comment at: tools/llvm-cfi-verify/FileVerifier.cpp:112
+
+  if (!Flows.OrphanedNodes.empty())
+    return false;
----------------
vlad.tsyrklevich wrote:
> This would be a surprising reason to see 'Protected? No.' Perhaps worth making this an enum return value with no/yes/could-not-be-determined?
I'm not sure it would be surprising. Orphaned nodes are those which have no static cross-references to them, meaning the control flow graph dies with them.

```
[0x0: ud2]                   [0x1000: movb $0x1, %rax]
    |                               |
[0x1: nop] <-----------------[0x1002: callq %rax]
    |
[0x2: jmpq %rax]
```

In this example, the graph starts at `0x2`, building up to `0x1`. There are no xrefs to `0x1` (as `0x0` cannot fall through, and `0x1002`'s target is non-static), and hence `0x1` is added as an "orphaned node". 

My assumption is that, in order for an indirect CF to be CFI protected, **all** possible ways to reach the indirect CF must be CFI protected. If a vcall then immediately makes another vcall without CFI checking (emulated in the example above), shouldn't it be flagged? 

You have a better understanding of this than me though, feel free to correct me :)


https://reviews.llvm.org/D38428