[PATCH] D75935: Add RET-hardening Support to X86 to mitigate Load Value Injection (LVI) [3/6]

[Zola Bridges via Phabricator via llvm-commits]

zbrid added a comment.

Thanks! Looks great. Please wait for another LGTM from someone more versed in LLVM conventions, but LGTM.



================
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionRetHardening.cpp:71
+
+  LLVM_DEBUG(dbgs() << "***** " << getPassName() << " : " << MF.getName()
+                    << " *****\n");
----------------
I think this should be at the top of the function.


================
Comment at: llvm/lib/Target/X86/X86LoadValueInjectionRetHardening.cpp:132
+  if (Modified)
+    ++NumFunctionsMitigated;
+  return Modified;
----------------
nit: Would it make sense to have this incremented when you set Modified to true instead of checking Modified here?


================
Comment at: llvm/test/CodeGen/X86/O0-pipeline.ll:78
 ; CHECK-NEXT:       Check CFA info and insert CFI instructions if needed
+; CHECK-NEXT:       X86 Load Value Injection (LVI) Ret-Hardening Pass
 ; CHECK-NEXT:       Lazy Machine Block Frequency Analysis
----------------
nit: Same as prior pass: remove "pass" from the name and update here and in the other test for the O3 pipeline


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75935/new/

https://reviews.llvm.org/D75935