[compiler-rt] r179032 - [libsymbolized] If we can't find an address in the list of shared libraries, try to reload it.

Alexey Samsonov samsonov at google.com
Mon Apr 8 11:09:52 PDT 2013


On Mon, Apr 8, 2013 at 9:46 PM, Alexander Potapenko <glider at google.com>wrote:

> Author: glider
> Date: Mon Apr  8 12:46:34 2013
> New Revision: 179032
>
> URL: http://llvm.org/viewvc/llvm-project?rev=179032&view=rev
> Log:
> [libsymbolized] If we can't find an address in the list of shared
> libraries, try to reload it.
>
> Add a regression test for the case where such behavior helps TSan:
>   1. race is reported in the main module
>   2. new shared library is loaded
>   3. race is reported in the shared library
>
>
> Added:
>     compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/
>     compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/lit.local.cfg
>     compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/load_shared_lib-so.cc
>     compiler-rt/trunk/lib/tsan/lit_tests/load_shared_lib.cc
> Modified:
>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc?rev=179032&r1=179031&r2=179032&view=diff
>
> ==============================================================================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc
> (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_symbolizer.cc Mon Apr
>  8 12:46:34 2013
> @@ -239,6 +239,7 @@ class InternalSymbolizer {
>
>  class Symbolizer {
>   public:
> +  Symbolizer() : modules_fresh_(false) { };
>    uptr SymbolizeCode(uptr addr, AddressInfo *frames, uptr max_frames) {
>      if (max_frames == 0)
>        return 0;
> @@ -376,7 +377,8 @@ class Symbolizer {
>    }
>
>    LoadedModule *FindModuleForAddress(uptr address) {
> -    if (modules_ == 0) {
> +    bool modules_were_reloaded = false;
> +    if (modules_ == 0 || !modules_fresh_) {
>        modules_ = (LoadedModule*)(symbolizer_allocator.Allocate(
>            kMaxNumberOfModuleContexts * sizeof(LoadedModule)));
>

Do you allocate new array of modules each time address is not found (and
leak the previous array)?
This is dangerous - sizeof(array of modules) is pretty large, and TSan may
quickly leak memory in this case...
I think we may switch to InternalVector<> for storing modules here, and
call .clear() on it to unmap memory.



>        CHECK(modules_);
> @@ -384,14 +386,25 @@ class Symbolizer {
>        // FIXME: Return this check when GetListOfModules is implemented on
> Mac.
>        // CHECK_GT(n_modules_, 0);
>        CHECK_LT(n_modules_, kMaxNumberOfModuleContexts);
> +      modules_fresh_ = true;
> +      modules_were_reloaded = true;
>      }
>      for (uptr i = 0; i < n_modules_; i++) {
>        if (modules_[i].containsAddress(address)) {
>          return &modules_[i];
>        }
>      }
> +    // Reload the modules and look up again, if we haven't tried it yet.
> +    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);
> +    }
>      return 0;
>    }
> +
>    void ReportExternalSymbolizerError(const char *msg) {
>      // Don't use atomics here for now, as SymbolizeCode can't be called
>      // from multiple threads anyway.
> @@ -406,6 +419,8 @@ class Symbolizer {
>    static const uptr kMaxNumberOfModuleContexts = 1 << 14;
>    LoadedModule *modules_;  // Array of module descriptions is leaked.
>    uptr n_modules_;
> +  // If stale, need to reload the modules before looking up addresses.
> +  bool modules_fresh_;
>
>    ExternalSymbolizer *external_symbolizer_;  // Leaked.
>    InternalSymbolizer *internal_symbolizer_;  // Leaked.
>
> Added: compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/lit.local.cfg
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/lit.local.cfg?rev=179032&view=auto
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/lit.local.cfg (added)
> +++ compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/lit.local.cfg Mon Apr
>  8 12:46:34 2013
> @@ -0,0 +1,4 @@
> +# Sources in this directory are compiled as shared libraries and used by
> +# tests in parent directory.
> +
> +config.suffixes = []
>
> Added:
> compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/load_shared_lib-so.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/load_shared_lib-so.cc?rev=179032&view=auto
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/load_shared_lib-so.cc
> (added)
> +++ compiler-rt/trunk/lib/tsan/lit_tests/SharedLibs/load_shared_lib-so.cc
> Mon Apr  8 12:46:34 2013
> @@ -0,0 +1,22 @@
> +//===----------- load_shared_lib-so.cc --------------------------*- C++
> -*-===//
> +//
> +//                     The LLVM Compiler Infrastructure
> +//
> +// This file is distributed under the University of Illinois Open Source
> +// License. See LICENSE.TXT for details.
> +//
>
> +//===----------------------------------------------------------------------===//
> +//
> +// This file is a part of ThreadSanitizer (TSan), a race detector.
> +//
>
> +//===----------------------------------------------------------------------===//
> +
> +#include <stddef.h>
> +
> +int GLOB_SHARED = 0;
> +
> +extern "C"
> +void *write_from_so(void *unused) {
> +  GLOB_SHARED++;
> +  return NULL;
> +}
>
> Added: compiler-rt/trunk/lib/tsan/lit_tests/load_shared_lib.cc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/lit_tests/load_shared_lib.cc?rev=179032&view=auto
>
> ==============================================================================
> --- compiler-rt/trunk/lib/tsan/lit_tests/load_shared_lib.cc (added)
> +++ compiler-rt/trunk/lib/tsan/lit_tests/load_shared_lib.cc Mon Apr  8
> 12:46:34 2013
> @@ -0,0 +1,44 @@
> +// Check that if the list of shared libraries changes between the two race
> +// reports, the second report occurring in a new shared library is still
> +// symbolized correctly.
> +
> +// RUN: %clangxx_tsan -O1 %p/SharedLibs/load_shared_lib-so.cc \
> +// RUN:     -fPIC -shared -o %t-so.so
> +// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
> +
> +#include <dlfcn.h>
> +#include <pthread.h>
> +#include <stdio.h>
> +
> +#include <string>
> +
> +int GLOB = 0;
> +
> +void *write_glob(void *unused) {
> +  GLOB++;
> +  return NULL;
> +}
> +
> +void race_two_threads(void *(*access_callback)(void *)) {
> +  pthread_t t1, t2;
> +  pthread_create(&t1, NULL, access_callback, NULL);
> +  pthread_create(&t2, NULL, access_callback, NULL);
> +  pthread_join(t1, NULL);
> +  pthread_join(t2, NULL);
> +}
> +
> +int main(int argc, char *argv[]) {
> +  std::string path = std::string(argv[0]) + std::string("-so.so");
> +  race_two_threads(write_glob);
> +  // CHECK: write_glob
> +  void *lib = dlopen(path.c_str(), RTLD_NOW);
> +    if (!lib) {
> +    printf("error in dlopen(): %s\n", dlerror());
> +    return 1;
> +  }
> +  void *(*write_from_so)(void *);
> +  *(void **)&write_from_so = dlsym(lib, "write_from_so");
> +  race_two_threads(write_from_so);
> +  // CHECK: write_from_so
> +  return 0;
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



-- 
Alexey Samsonov, MSK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/608f1efc/attachment.html>


More information about the llvm-commits mailing list