[PATCH] D105201: [hwasan] Detect use after scope within function.

[Evgenii Stepanov via Phabricator via llvm-commits]

eugenis added a comment.

Could you investigate how common multi-region lifetimes are? I.e. how much worse is use-after-scope detection in hwasan compared to asan. You can just dump variable and function names and overall numbers into llvm::errs() and compile something big.

I'm sure I've seen allocas getting merged in IR after inlining, but I can not reproduce it now. I see a comment in lib/Transforms/IPO/Inliner.cpp about it being removed. Maybe we do not need to worry about it.



================
Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerCommon.h:52
+template <typename F>
+void tagLifetimeEnd(DominatorTree *DT, PostDominatorTree *PDT,
+                    IntrinsicInst *Start, IntrinsicInst *End,
----------------
tagLifetimeEnd is not descriptive enough for namespace llvm. M/b something like ForAllReachableExits? Start and End can be just Instruction *, and RetVec should be const. Instead of erasing End I'd rather return a bool that's true if any callbacks were invoked on any of the RetVec, and caller can remove lifetime.end then.

This refactoring is better done in a separate change.


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:124
+                    cl::desc("detect use after scope within function"),
+                    cl::Hidden, cl::init(false));
+
----------------
Probably should be on by default.


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:358
   bool InstrumentStack;
+  bool UseAfterScope;
   bool UsePageAliases;
----------------
DetectUseAfterScope


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:390
   void getAnalysisUsage(AnalysisUsage &AU) const override {
-    AU.setPreservesCFG();
     if (shouldUseStackSafetyAnalysis(TargetTriple)) {
----------------
why?


================
Comment at: llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp:466
 PreservedAnalyses HWAddressSanitizerPass::run(Module &M,
                                               ModuleAnalysisManager &MAM) {
   const StackSafetyGlobalInfo *SSI = nullptr;
----------------
Please support this in the new pass manager, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105201