Invalidate symbolizer module list from dlopen/dlclose interceptors

[fjricci]

Summary

The module list should only be invalidated by dlopen and dlclose,
so the symbolizer should only re-generate it when we've hit one of those functions.

[fjricci]

Ping - I think this one may have slipped under the radar with the reviews for D37269

[fjricci]

ping - blocks the custom loader patch

[eugenis]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5579 ↗

(On Diff #113135)

This would not spawn addr2line or llvm-symbolizer process right away?

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

185 ↗

(On Diff #113135)

This recursion will be removed in the followup change, right?

[fjricci]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5579 ↗

(On Diff #113135)

The symbolizer process is launched by StartSymbolizerProcess(), which is only launched by SendCommand(), which is invoked by SymbolizePC() and and SymbolizeData(), so I believe this will be fine. I'll run a test and look for a symbolizer process before merging though.

lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

185 ↗

(On Diff #113135)

Correct.

[fjricci]

lib/sanitizer_common/sanitizer_common_interceptors.inc

5579 ↗

(On Diff #113135)

Verified that this runs the interceptor but doesn't spawn a symbolizer process:

#include <dlfcn.h>
int main() {
  dlopen("libc.so", RTLD_NOW);
  while (1) ;
  return 0;
}

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc

 //   COMMON_INTERCEPTOR_STRNDUP_IMPL
 //===----------------------------------------------------------------------===//
 
 #include "interception/interception.h"
 #include "sanitizer_addrhashmap.h"
 #include "sanitizer_errno.h"
 #include "sanitizer_placement_new.h"
 #include "sanitizer_platform_interceptors.h"
+#include "sanitizer_symbolizer.h"
 #include "sanitizer_tls_get_addr.h"
 
 #include <stdarg.h>
 
 #if SANITIZER_INTERCEPTOR_HOOKS
 #define CALL_WEAK_INTERCEPTOR_HOOK(f, ...) f(__VA_ARGS__);
 #define DECLARE_WEAK_INTERCEPTOR_HOOK(f, ...) \
   SANITIZER_INTERFACE_WEAK_DEF(void, f, __VA_ARGS__) {}

 
 #if SANITIZER_INTERCEPT_DLOPEN_DLCLOSE
 INTERCEPTOR(void*, dlopen, const char *filename, int flag) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER_NOIGNORE(ctx, dlopen, filename, flag);
   if (filename) COMMON_INTERCEPTOR_READ_STRING(ctx, filename, 0);
   COMMON_INTERCEPTOR_ON_DLOPEN(filename, flag);
   void *res = REAL(dlopen)(filename, flag);
+  Symbolizer::GetOrInit()->InvalidateModuleList();
   COMMON_INTERCEPTOR_LIBRARY_LOADED(filename, res);
   return res;
 }
 
 INTERCEPTOR(int, dlclose, void *handle) {
   void *ctx;
   COMMON_INTERCEPTOR_ENTER_NOIGNORE(ctx, dlclose, handle);
   int res = REAL(dlclose)(handle);
+  Symbolizer::GetOrInit()->InvalidateModuleList();
   COMMON_INTERCEPTOR_LIBRARY_UNLOADED();
   return res;
 }
 #define INIT_DLOPEN_DLCLOSE          \
   COMMON_INTERCEPT_FUNCTION(dlopen); \
   COMMON_INTERCEPT_FUNCTION(dlclose);
 #else
 #define INIT_DLOPEN_DLCLOSE

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.h

public:
   typedef void (*StartSymbolizationHook)();
   typedef void (*EndSymbolizationHook)();
   // May be called at most once.
   void AddHooks(StartSymbolizationHook start_hook,
                 EndSymbolizationHook end_hook);
 
   const LoadedModule *FindModuleForAddress(uptr address);
 
+  void InvalidateModuleList();
+
  private:
   // GetModuleNameAndOffsetForPC has to return a string to the caller.
   // Since the corresponding module might get unloaded later, we should create
   // our owned copies of the strings that we can safely return.
   // ModuleNameOwner does not provide any synchronization, thus calls to
   // its method should be protected by |mu_|.
   class ModuleNameOwner {
    public:

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc

void DataInfo::Clear() {
   InternalFree(name);
   internal_memset(this, 0, sizeof(DataInfo));
 }
 
 Symbolizer *Symbolizer::symbolizer_;
 StaticSpinMutex Symbolizer::init_mu_;
 LowLevelAllocator Symbolizer::symbolizer_allocator_;
 
+void Symbolizer::InvalidateModuleList() {
+  modules_fresh_ = false;
+}
+
 void Symbolizer::AddHooks(Symbolizer::StartSymbolizationHook start_hook,
                           Symbolizer::EndSymbolizationHook end_hook) {
   CHECK(start_hook_ == 0 && end_hook_ == 0);
   start_hook_ = start_hook;
   end_hook_ = end_hook;
 }
 
 const char *Symbolizer::ModuleNameOwner::GetOwnedCopy(const char *str) {

compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer_libcdep.cc

if (!modules_fresh_) {
     modules_fresh_ = true;
     modules_were_reloaded = true;
   }
   for (uptr i = 0; i < modules_.size(); i++) {
     if (modules_[i].containsAddress(address)) {
       return &modules_[i];
     }
   }
-  // Reload the modules and look up again, if we haven't tried it yet.
+  // dlopen/dlclose interceptors invalidate the module list, but when
+  // interception is disabled, we need to retry if the lookup fails in
+  // case the module list changed.
+#if !SANITIZER_INTERCEPT_DLOPEN_DLCLOSE
   if (!modules_were_reloaded) {
-    // FIXME: set modules_fresh_ from dlopen()/dlclose() interceptors.
-    // It's too aggressive to reload the list of modules each time we fail
-    // to find a module for a given address.
     modules_fresh_ = false;
     return FindModuleForAddress(address);
   }
+#endif
   return 0;
 }
 
 // For now we assume the following protocol:
 // For each request of the form
 //   <module_name> <module_offset>
 // passed to STDIN, external symbolizer prints to STDOUT response:
 //   <function_name>