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

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 16 16:06:30 PDT 2019


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


================
Comment at: lib/asan/scripts/asan_symbolize.py:631
+
+class SysRootFilterPlugIn(AsanSymbolizerPlugIn):
+  """
----------------
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.


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D60529





More information about the llvm-commits mailing list