[PATCH] D54337: [ASan] Make AddressSanitizer a ModulePass

[Ali Tamur via Phabricator via llvm-commits]

tamur added a comment.

I am not an expert or have a good context, please excuse my trespassing, just as a guy reading the code and trying to understand, this is my understanding:

1. GlobalsMD.init(M); is the only expensive part of the AddressSanitizer initialization.
2. All data in GlobalsMD is obtained from a module, in GlobalsMetadata::init(Module& M).
3. (Maybe this is not obvious) Function objects have a reference to their module via Function::getParent() method.
4. Therefore, every expensive data that the function pass for AddressSanitizer needs, can be stored in Module objects:
5. GlobalsMetadata class is probably no longer needed, at worst, the GlobalVariable* --> Entry mapping can be stored within the Module class.
6. I haven't looked whether this mapping can be initialized within the constructor of Module. If yes, great. If not, some care needs to be taken to make the mapping be initialized only once on the first demand, also with care with respect to synchronization (multiple AddressSanitizers for functions of the same module may be running in parallel -if not now, in the future-), but these should be minor complications.

With these changes, AddressSanitizer can become a function level pass.

There is also an AddressSanitizerModule class in AddressSanitizer.cpp. If I understand correctly, now it is running as a module level pass before the AddressSanitizer the function level pass. The owners probably do not want to have this as a module level pass either. There may be a better way to accomplish this, but this is the simplest solution that comes to my mind:

- It seems that AddressSanitizerModule is run only for its side effects on the Module. Which is good, so we don't need to find a place to store the calculated data. (If I'm wrong please ignore the rest of my comment)
- Let's call the AddressSanitizerModule business logic ASM_BL.
- Either add a boolean instance variable to Module, or have a global Module-->bool mapping to keep track of for which modules ASM_BL code has run.
- Each address sanitizer function pass looks whether the ASM_BL for the function's module has already run, and if not runs it as the first thing.
- But be careful about synchronization, protect ASM_BL with a mutex or something, be sure that it runs only once.
- Alternatively, make AddressSanitizerModule pass a separate function level pass, make it a prerequisite of the AddressSanitizer the function level pass, and still take care of synchronization etc. i.e. hide ASM_BL behind some abstraction, so that even though every function of the module will call ASM_BL, it will actually run only once.

Any time your understanding or other reviewers' suggestions contradict mine, please feel free to ignore mine.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54337