[PATCH] Overhaul the symbolizer interface.

Alexey Samsonov samsonov at google.com
Fri Oct 25 08:27:46 PDT 2013


  LGTM modulo the following comments. Thanks for patience and, once again, for working on this!


================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:315
@@ -314,3 +314,3 @@
 #if !SANITIZER_GO
-  getSymbolizer()->PrepareForSandboxing();
+  if (Symbolizer *sym = Symbolizer::GetOrNull()) sym->PrepareForSandboxing();
 #endif
----------------
Use
  if (Symbolizer *sym = ...)
    sym->...

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.h:73
@@ +72,3 @@
+class Symbolizer {
+  static atomic_uintptr_t symbolizer_;
+
----------------
Generally in sanitizers code we use different order: public methods, private methods, private members. I would also prefer no blank lines between declarations, especially if they belong to the same group (Get, GetOrNull, GetOrInit etc) and double slash for C++ comments for consistency.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.h:95
@@ +94,3 @@
+  /// Returns platform-specific implementation of Symbolizer.  Will
+  /// automatically initialize symbolizer as if by calling init(0) if needed.
+  static Symbolizer *GetOrInit();
----------------
s/init(0)/Init(0)

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:357
@@ -359,3 +356,3 @@
  public:
   static InternalSymbolizer *get() { return 0; }
   char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
----------------
Match the new InternalSymbolizer::get() arguments here.

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:565
@@ -573,2 +564,3 @@
   bool modules_fresh_;
+  StaticSpinMutex mu_;
 
----------------
BlockingMutex is more appropriate here - symbolizer functions are not hot and can be quite slow - it's better to block waiting threads in this case.


http://llvm-reviews.chandlerc.com/D1985

BRANCH
  symbolizer

ARCANIST PROJECT
  compiler-rt



More information about the llvm-commits mailing list