[cfi-verify] Add an interesting unit test where undef search length changes result.

[hctim]

Summary

Add an interesting unit test, found by changing --search-length-undef from the default. Program handles it correctly but good for ensuring correctness on further changes :)

[hctim]

Updated with Vlad's comments, and rebased against master.

[hctim]

Accidentally updated the wrong patch - reverted last diff.

[pcc]

LGTM

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

654 ↗

(On Diff #121356)

Maybe SearchLengthForUndef should be a field on FileAnalysis?

[hctim]

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

654 ↗

(On Diff #121356)

My thoughts are that SearchLengthForUndef should be a singleton across all instances of FileAnalysis. It just so happens that FileAnalysis is always a singleton (apart from these unit tests), and I'm not sure that duplicating this data is neccessary. WDYT?

[pcc]

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

654 ↗

(On Diff #121356)

Up to you, but it seems that making it a field would make your tests less error-prone, which to me means that it's probably the "right" architecture.

[hctim]

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

654 ↗

(On Diff #121356)

I think I'd prefer to have the flag closer to the place that it's used, rather than be a flag that's copied into each FileAnalysis instance at runtime and then passed through options injection into GraphBuilder. I think it makes the usage of it too far from the definition for somehting that (in a non-test environment) is unchanged after startup.

Feel free to overwrite me on this though, I'll follow it up in a further patch if that's what you desire :)

[hctim]

Merge in HEAD for submission.

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

Analysis.parseSectionContents(
           0xeb, 0xfc,                   // 11: jmp 9 [-4]
           0x75, 0xfa,                   // 13: jne 9 [-6]
           0xe8, 0x78, 0x56, 0x34, 0x12, // 15: callq OUTOFBOUNDS [+0x12345678]
           0x90,                         // 20: nop
           0x90,                         // 21: nop
           0x0f, 0x0b,                   // 22: ud2
       },
       0xDEADBEEF);
+  uint64_t PrevSearchLengthForUndef = SearchLengthForUndef;
+  SearchLengthForUndef = 5;
   EXPECT_FALSE(Analysis.isCFIProtected(0xDEADBEEF + 9));
+  SearchLengthForUndef = PrevSearchLengthForUndef;
+}
+
+TEST_F(BasicFileAnalysisTest, UndefSearchLengthOneTest) {
+  Analysis.parseSectionContents(
+      {
+          0x77, 0x0d,                   // 0x688118: ja 0x688127 [+12]
+          0x48, 0x89, 0xdf,             // 0x68811a: mov %rbx, %rdi
+          0xff, 0xd0,                   // 0x68811d: callq *%rax
+          0x48, 0x89, 0xdf,             // 0x68811f: mov %rbx, %rdi
+          0xe8, 0x09, 0x00, 0x00, 0x00, // 0x688122: callq 0x688130
+          0x0f, 0x0b,                   // 0x688127: ud2
+      },
+      0x688118);
+  uint64_t PrevSearchLengthForUndef = SearchLengthForUndef;
+  SearchLengthForUndef = 1;
+  EXPECT_TRUE(Analysis.isCFIProtected(0x68811d));
+  SearchLengthForUndef = PrevSearchLengthForUndef;
+}
+
+TEST_F(BasicFileAnalysisTest, UndefSearchLengthOneTestFarAway) {
+  Analysis.parseSectionContents(
+      {
+          0x74, 0x73,                         // 0x7759eb: je 0x775a60
+          0xe9, 0x1c, 0x04, 0x00, 0x00, 0x00, // 0x7759ed: jmpq 0x775e0e
+      },
+      0x7759eb);
+
+  Analysis.parseSectionContents(
+      {
+          0x0f, 0x85, 0xb2, 0x03, 0x00, 0x00, // 0x775a56: jne    0x775e0e
+          0x48, 0x83, 0xc3, 0xf4, // 0x775a5c: add    $0xfffffffffffffff4,%rbx
+          0x48, 0x8b, 0x7c, 0x24, 0x10, // 0x775a60: mov    0x10(%rsp),%rdi
+          0x48, 0x89, 0xde,             // 0x775a65: mov    %rbx,%rsi
+          0xff, 0xd1,                   // 0x775a68: callq  *%rcx
+      },
+      0x775a56);
+
+  Analysis.parseSectionContents(
+      {
+          0x0f, 0x0b, // 0x775e0e: ud2
+      },
+      0x775e0e);
+  uint64_t PrevSearchLengthForUndef = SearchLengthForUndef;
+  SearchLengthForUndef = 1;
+  EXPECT_FALSE(Analysis.isCFIProtected(0x775a68));
+  SearchLengthForUndef = 2;
+  EXPECT_TRUE(Analysis.isCFIProtected(0x775a68));
+  SearchLengthForUndef = 3;
+  EXPECT_TRUE(Analysis.isCFIProtected(0x775a68));
+  SearchLengthForUndef = PrevSearchLengthForUndef;
 }
 
 } // anonymous namespace
 } // end namespace cfi_verify
 } // end namespace llvm
 
 int main(int argc, char **argv) {
   ::testing::InitGoogleTest(&argc, argv);