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

Mitch Phillips via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 3 11:15:24 PDT 2017


hctim added inline comments.


================
Comment at: unittests/tools/llvm-cfi-verify/FileAnalysis.cpp:654
+  uint64_t PrevSearchLengthForUndef = SearchLengthForUndef;
+  SearchLengthForUndef = 5;
   EXPECT_FALSE(Analysis.isIndirectInstructionCFIProtected(0xDEADBEEF + 9));
----------------
pcc wrote:
> Maybe `SearchLengthForUndef` should be a field on `FileAnalysis`?
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?


https://reviews.llvm.org/D38658





More information about the llvm-commits mailing list