[PATCH] D56470: [NewPM] Second attempt at porting ASan

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc requested changes to this revision.
chandlerc added a comment.
This revision now requires changes to proceed.

I think a key thing that this is currently missing (or I'm missing it when looking at hte patch) the core shift that we made working through how to do sanitizer stuff with MSan.

Specifically, I suspect that we don't need any analysis layer, and the module and function layer can operate entirely independently. This would require roughly the following changes to the current ASan infrastructure:

1. Change how the per-module initialization of the `AddressSanitizerFunction` pass works to resemble hwat was done for MSan and TSan, factoring it into a helper class that is used by a legacy function pass an a new function pass. This will require the get-or-create pattern for all the things touched, and registering things only when creation is necessary.

2. Make a few things in the current function pass lazy such as buildig the inline assembly needed, etc.

3. Directly port the `AddressSanitizerModule` to new PM.

4. (maybe) decouple the ctor insertion in the module pass from the ctor insertion in the function pass. but maybe no need, just the first one to need it does it.

I don't think any analysis should be required at this point?

Let me know if this isn't making sense and I can try to explain more.



================
Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerPass.h:108-123
+class AddressSanitizerParamsAnalysis
+    : public AnalysisInfoMixin<AddressSanitizerParamsAnalysis> {
+public:
+  using Result = AddressSanitizerParamsWrapper;
+
+  AddressSanitizerParamsAnalysis(bool CompileKernel = false)
+      : Params(CompileKernel) {}
----------------
Why is this analysis needed? It seems like it could just be directly computed as part of the pass?

Is there some logic shared between this and some other pass? If so, documenting what it is and how it works seems really important.


================
Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerPass.h:125-126
+
+/// Public interface to the address sanitizer pass for instrumenting code to
+/// check for various memory bugs.
+class AddressSanitizerPass : public PassInfoMixin<AddressSanitizerPass> {
----------------
Similar to the MSan and TSan cases, this should also explain *how* this will modify the IR and whether there are any oddities or constraints on it.


================
Comment at: llvm/include/llvm/Transforms/Instrumentation/AddressSanitizerPass.h:140-141
+
+class AddressSanitizerModulePass
+    : public PassInfoMixin<AddressSanitizerModulePass> {
+public:
----------------
We should have some documentation for what this does?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56470