[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