[PATCH] D40111: [cfi-verify] Add blame context printing, and improved print format.

[Vlad Tsyrklevich via Phabricator via llvm-commits]

vlad.tsyrklevich added inline comments.


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:67
+  if (!FileOrErr) {
+    outs() << "******* Could not open file.\n";
+    return;
----------------
errs()


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:72
+  std::unique_ptr<MemoryBuffer> File = std::move(FileOrErr.get());
+  SmallVector<StringRef, 200> Lines;
+  File->getBuffer().split(Lines, '\n');
----------------
I'd just make this a SmallVectorImpl<> or a std::vector<>, the small-size optimization here is not likely to help much since many files are >200 lines.


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:80
+    if (i == LineInfo.Line)
+      outs() << "[" << ProtectionStatusShort << "]>";
+    else
----------------
Why print this here at all? It would seem reasonable to me to just print the full protection status with the instruction, e.g. `Expected Protected Instruction: ...` and just have this print
``` 1: ...
>2: ...
 3: ...```


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:92
+                                 CFIProtectionStatus ProtectionStatus) {
+  outs() << "----------------- Begin Instruction -----------------\n";
+  outs() << stringCFIProtectionStatus(ProtectionStatus) << " "
----------------
Can we condense this to a single line and remove the text formatting? Like
```outs() << "Instruction: " << stringCfiProtectionStatus() << ": " << format_hex() << ": ";


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:108
+    if (CFIProtected) {
+      outs() << "====> Unexpected Protected\n";
+      if (PrintBlameContextAll)
----------------
Prefer to put these with instruction printing (e.g. so it gets printed even if no SCL is given.)


================
Comment at: tools/llvm-cfi-verify/llvm-cfi-verify.cpp:127
+  }
+  outs() << "-----------------------------------------------------\n";
+}
----------------
We have lots of these delimiters, could we just place them at the start or end of an instruction and eliminate the rest?


https://reviews.llvm.org/D40111