[PATCH] [ASan] Allow the users of SymbolizationLoop to make use of the -dsym_hint option in llvm-symbolizer

Alexander Potapenko glider at google.com
Fri Nov 21 04:54:43 PST 2014


Addressed the comments.
Please note that this revision has been abandoned before you've started adding the comments.
I've uploaded a new one: http://reviews.llvm.org/D6310

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:73
@@ +72,3 @@
+    self.default_arch = default_arch
+    self.system = system
+    self.dsym_hints = dsym_hints
----------------
earthdok wrote:
> how about "os"?
The same property of SymbolizationLoop is already called "system"

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:86
@@ -82,1 +85,3 @@
+      for hint in self.dsym_hints:
+        cmd.append('-dsym-hint=%s' % hint)
     if DEBUG:
----------------
earthdok wrote:
> for consistency this should be "--dsym-hint"
Done.

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:357
@@ -348,4 +356,3 @@
   def symbolize_address(self, addr, binary, offset):
-    # Initialize llvm-symbolizer lazily.
-    if not self.llvm_symbolizer:
-      self.llvm_symbolizer = LLVMSymbolizerFactory(self.system, addr)
+    # On non-Darwin (i.e. on platforms without .dSYM debug info) always use
+    # a single symbolizer binary.
----------------
earthdok wrote:
> I think those comments should be moved into the code.
Not sure they'll become easier to follow then.

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:361
@@ +360,3 @@
+    #  1. check whether we've seen this binary already; if so,
+    #     use |llvm_symbolizers[binary]|;
+    #  2. otherwise check if we've seen this hint already; if so,
----------------
earthdok wrote:
> Needs a comment on why we don't want to just use the last symbolizer always.
Done.

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:363
@@ +362,3 @@
+    #  2. otherwise check if we've seen this hint already; if so,
+    #     reuse |last_llvm_symbolizer|;
+    #  3. otherwise create a new symbolizer and pass all currently known
----------------
earthdok wrote:
> ...which has the full set of hints.
Done.

================
Comment at: projects/compiler-rt/lib/asan/scripts/asan_symbolize.py:370
@@ +369,3 @@
+        dsym_hint = self.dsym_hint_producer(binary)
+        if dsym_hint and not dsym_hint in self.dsym_hints:
+          use_last_symbolizer = False
----------------
earthdok wrote:
> "if dsym_hint and (dsym_hint not in self.dsym_hints)" would be more readable I think
Done. However this code has changed a bit in http://reviews.llvm.org/D6310

http://reviews.llvm.org/D6309






More information about the llvm-commits mailing list