[PATCH] D39820: [cfi-verify] Validate there are no spills between CFI-check and instruction execution.

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added inline comments.


================
Comment at: tools/llvm-cfi-verify/lib/FileAnalysis.cpp:294
+  // Now check all branches to indirect CFs and ensure no spill happens.
+  for (const auto Branch : Graph.ConditionalBranchNodes) {
+    uint64_t Node;
----------------
I think Branch should explicitly be declared a reference to avoid copies.


================
Comment at: tools/llvm-cfi-verify/lib/FileAnalysis.cpp:306
+      for (unsigned RegNum : RegisterNumbers) {
+        if (InstrDesc.hasImplicitDefOfPhysReg(RegNum, RegisterInfo.get()))
+          return Node;
----------------
Reading the MCInstrDesc docs it makes it seem like this will ONLY check for implicit uses, not explicit uses. It's confusing why then  your `add $0, %eax` unit tests below works, maybe it' s because you're implicitly defining rax by adding to eax. Could you try using an add rax instruction instead and see if that works?


================
Comment at: tools/llvm-cfi-verify/lib/GraphBuilder.cpp:304
 
-    if (BranchTarget == Address)
+    if (BranchTarget == Address) {
       BranchNode.Target = Address;
----------------
```BranchNode.IndirectCFIsOnTargetPath = (BranchTarget == Address);
if (BranchNode.IndirectCFIsOnTargetPath) {```


https://reviews.llvm.org/D39820