[PATCH] D52739: [PassManager/Sanitizer] Port of AddresSanitizer pass from legacy to new PassManager

[Philip Pfaffe via Phabricator via llvm-commits]

philip.pfaffe added a comment.

We're getting there! Some more tiny nits.



================
Comment at: include/llvm/Transforms/Instrumentation/AddressSanitizerPass.h:32
+  PreservedAnalyses run(Function &F, AnalysisManager<Function> &AM);
+  PreservedAnalyses run(Module &M, AnalysisManager<Module> &AM);
+
----------------
These are called FunctionAnalysisManager and ModuleAnalysisManager. Coincidentally these are just typedefs for the same things, but that's not guaranteed, e.g. AnalysisManager<Loop> != LoopAnalysisManager.


================
Comment at: include/llvm/Transforms/Instrumentation/AddressSanitizerPass.h:38
+  bool UseAfterScope;
+  bool ModuleUseAfterScope;
+};
----------------
No, this shouldn't be needed. You will be using one instance per PM type.


================
Comment at: lib/Passes/PassRegistry.def:43
 MODULE_PASS("always-inline", AlwaysInlinerPass())
+MODULE_PASS("asan-module", AddressSanitizerPass())
 MODULE_PASS("called-value-propagation", CalledValuePropagationPass())
----------------
The module suffix is superfluous.


================
Comment at: lib/Transforms/Instrumentation/AddressSanitizer.cpp:2538
 
-bool AddressSanitizer::runOnFunction(Function &F) {
+bool AddressSanitizer::instrument(Function &F, DominatorTree *DTree,
+                                  const TargetLibraryInfo *TLI) {
----------------
I'd find it nicer if the DomintarTree were passed during construction instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D52739