[cfi-verify] Detect more protected calls using cross-DSO.

[jgalenson]

Summary

When using cross-DSO, some indirect calls are not guarded by a branch to a trap but instead follow a call to __cfi_slowpath. For example:

if (!InlinedFastCheck(f)) {

call *f

} else {

__cfi_slowpath(CallSiteTypeId, f);
call *f

}

In this case, the second call to f is not marked as protected by the current code. We thus recognize if an indirect call directly follows a call to a function that will trap on CFI violations and treat them as protected.

We also ignore indirect calls in the PLT, since on AArch64 each entry contains an indirect call that should not be protected by CFI.

[jgalenson]

Note that this could be combined with its parent patch, as that is now about supporting cross-DSO.

[vlad.tsyrklevich]

Remind me, was there a reason why you wanted to use binary test inputs instead of assembling them?

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

448

Why?

[jgalenson]

In D49675#1183229, @vlad.tsyrklevich wrote:

Remind me, was there a reason why you wanted to use binary test inputs instead of assembling them?

The same reason as in https://reviews.llvm.org/D49383:

Second, I don't know the best way of testing this. I need correctly-linked files to populate the symbol table and PLT, but I don't think I can assume a linker, and llvm-mc doesn't seem to handle calls to undefined functions well. So for now I just updated a binary with a valid PLT, but I'd much prefer to use .s and unit tests like the others. Is there a good way to do that, or does this suffice?

As an aside, when I finish the rewrite suggested in https://reviews.llvm.org/D49383, I'm planning to merge this into that patch so that it adds proper support for cross-DSO mode.

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

448

AArch64 plt entries look like:

adrp
ldr x17
add
br x17

So of course they're unprotected indirect calls.

What I left off this comment is that this is only needed when using the -ignore-dwarf flag. Without that, these entries are filtered out because they have no line information. But when using that flag, something like this is very helpful at removing spurious failures.

[jgalenson]

I've rolled this into https://reviews.llvm.org/D49383.

test/tools/llvm-cfi-verify/X86/function_only_check.s

+# RUN: llvm-cfi-verify %S/Inputs/function-only-check.o | FileCheck %s
+
+# CHECK-LABEL: {{^Instruction: .* \(PROTECTED\)}}
+
+# CHECK: Expected Protected: 1 (100.00%)
+# CHECK: Unexpected Protected: 0 (0.00%)
+# CHECK: Expected Unprotected: 0 (0.00%)
+# CHECK: Unexpected Unprotected (BAD): 0 (0.00%)

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

if (!LineInfoValid)
           inconvertibleErrorCode());
   }
 
   for (const object::SectionRef &Section : Object->sections()) {
     // Ensure only executable sections get analysed.
     if (!(object::ELFSectionRef(Section).getFlags() & ELF::SHF_EXECINSTR))
       continue;
 
+    // Avoid checking the PLT since it produces spurious failures on AArch64.
+    StringRef SectionName;
+    if (!Section.getName(SectionName) && SectionName == ".plt")
+      continue;
+
     StringRef SectionContents;
     if (Section.getContents(SectionContents))
       return make_error<StringError>("Failed to retrieve section contents",
                                      inconvertibleErrorCode());
 
     ArrayRef<uint8_t> SectionBytes((const uint8_t *)SectionContents.data(),
                                    Section.getSize());
     parseSectionContents(SectionBytes, Section.getAddress());

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

for (const auto *ParentMetaPtr : CFCrossRefs) {
     else
       BranchNode.Fallthrough = Address;
 
     HasValidCrossRef = true;
     buildFlowsToUndefined(Analysis, Result, BranchNode, ParentMeta);
     Result.ConditionalBranchNodes.push_back(BranchNode);
   }
 
+  // When using cross-DSO, some indirect calls are not guarded by a branch to a
+  // trap but instead follow a call to __cfi_slowpath.  For example:
+  // if (!InlinedFastCheck(f))
+  //    call *f
+  //  else {
+  //    __cfi_slowpath(CallSiteTypeId, f);
+  //    call *f
+  //  }
+  // To mark the second call as protected, we recognize indirect calls that
+  // directly follow calls to functions that will trap on CFI violations.
+  if (CFCrossRefs.empty()) {
+    const Instr *PrevInstr = Analysis.getPrevInstructionSequential(ChildMeta);
+    if (PrevInstr && Analysis.willTrapOnCFIViolation(*PrevInstr)) {
+      Result.IntermediateNodes[PrevInstr->VMAddress] = Address;
+      HasValidCrossRef = true;
+    }
+  }
+
   if (!HasValidCrossRef)
     Result.OrphanedNodes.push_back(Address);
 
   OpenedNodes.erase(Address);
 }
 
 } // namespace cfi_verify
 } // namespace llvm