[PATCH] D60529: [asan_symbolize] Add a simple plugin architecture

[Dan Liew via Phabricator via llvm-commits]

delcypher marked an inline comment as done.
delcypher added inline comments.


================
Comment at: lib/asan/scripts/asan_symbolize.py:631
+
+class SysRootFilterPlugIn(AsanSymbolizerPlugIn):
+  """
----------------
delcypher wrote:
> vitalybuka wrote:
> > why do you need to extract -s into plugin?
> We don't **need** to do this, but there is a reason I did this.
> 
> I did this to keep the logic in `symbolize_address()` simple, i.e. we just have this:
> 
> ```lang=python
>  binary = self.plugin_proxy.filter_binary_path(binary)
> ```
> 
> So **all binary path filtering** goes through the `plugin_proxy`, i.e. the sys_root filter is **not a special case** that has to be handled inside `symbolize_address()`. There doesn't seem to be anything special about the sys root filter so treating it as a special case seems unnecessary.
> 
> I personally prefer the `SysRootFilterPlugIn` class because it keeps all the sys root filter stuff together in a small class. It comes at the cost of readability though because anyone reading the code has to roughly understand how the plugin interface works, whereas previously this was not a requirement.
> 
> There is a trade-off here and I've picked the one I prefer. You might have a different opinion.
@vitalybuka Does that answer your question? I see you've approved this patch but I want to be sure you're happy with everything before I land it.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D60529