[compiler-rt] r287632 - tsan: switch libignore from /proc/self/maps to dl_iterate_phdr

Diana Picus via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 22 03:22:35 PST 2016


Cool, thanks for the quick turnaround.

On 22 November 2016 at 13:19, Dmitry Vyukov <dvyukov at google.com> wrote:
> 287634 should fix it
>
> On Tue, Nov 22, 2016 at 12:17 PM, Dmitry Vyukov <dvyukov at google.com> wrote:
>> Looking
>>
>> Thanks
>>
>> On Tue, Nov 22, 2016 at 12:13 PM, Diana Picus <diana.picus at linaro.org> wrote:
>>> Hi Dmitry,
>>>
>>> Can you please have a look at this failure:
>>> http://lab.llvm.org:8011/builders/clang-cmake-aarch64-42vma/builds/1198
>>> ?
>>>
>>> Thanks,
>>> Diana
>>>
>>> On 22 November 2016 at 11:49, Dmitry Vyukov via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>>> Author: dvyukov
>>>> Date: Tue Nov 22 03:49:11 2016
>>>> New Revision: 287632
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=287632&view=rev
>>>> Log:
>>>> tsan: switch libignore from /proc/self/maps to dl_iterate_phdr
>>>>
>>>> /proc/self/maps can't be read atomically, this leads to episodic
>>>> crashes in libignore as it thinks that a module is loaded twice.
>>>> See the new test for an example.
>>>> dl_iterate_phdr does not have this problem.
>>>> Switch libignore to dl_iterate_phdr.
>>>>
>>>>
>>>> Added:
>>>>     compiler-rt/trunk/test/tsan/ignore_lib5.cc
>>>>     compiler-rt/trunk/test/tsan/ignore_lib5.cc.supp
>>>> Modified:
>>>>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc
>>>>
>>>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc?rev=287632&r1=287631&r2=287632&view=diff
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc (original)
>>>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_libignore.cc Tue Nov 22 03:49:11 2016
>>>> @@ -50,23 +50,23 @@ void LibIgnore::OnLibraryLoaded(const ch
>>>>    }
>>>>
>>>>    // Scan suppressions list and find newly loaded and unloaded libraries.
>>>> -  MemoryMappingLayout proc_maps(/*cache_enabled*/false);
>>>> -  InternalScopedString module(kMaxPathLength);
>>>> +  ListOfModules modules;
>>>> +  modules.init();
>>>>    for (uptr i = 0; i < count_; i++) {
>>>>      Lib *lib = &libs_[i];
>>>>      bool loaded = false;
>>>> -    proc_maps.Reset();
>>>> -    uptr b, e, off, prot;
>>>> -    while (proc_maps.Next(&b, &e, &off, module.data(), module.size(), &prot)) {
>>>> -      if ((prot & MemoryMappingLayout::kProtectionExecute) == 0)
>>>> -        continue;
>>>> -      if (TemplateMatch(lib->templ, module.data()) ||
>>>> -          (lib->real_name &&
>>>> -          internal_strcmp(lib->real_name, module.data()) == 0)) {
>>>> +    for (const auto &mod : modules) {
>>>> +      for (const auto &range : mod.ranges()) {
>>>> +        if (!range.executable)
>>>> +          continue;
>>>> +        if (!TemplateMatch(lib->templ, mod.full_name()) &&
>>>> +            !(lib->real_name &&
>>>> +            internal_strcmp(lib->real_name, mod.full_name()) == 0))
>>>> +          continue;
>>>>          if (loaded) {
>>>>            Report("%s: called_from_lib suppression '%s' is matched against"
>>>>                   " 2 libraries: '%s' and '%s'\n",
>>>> -                 SanitizerToolName, lib->templ, lib->name, module.data());
>>>> +                 SanitizerToolName, lib->templ, lib->name, mod.full_name());
>>>>            Die();
>>>>          }
>>>>          loaded = true;
>>>> @@ -75,13 +75,14 @@ void LibIgnore::OnLibraryLoaded(const ch
>>>>          VReport(1,
>>>>                  "Matched called_from_lib suppression '%s' against library"
>>>>                  " '%s'\n",
>>>> -                lib->templ, module.data());
>>>> +                lib->templ, mod.full_name());
>>>>          lib->loaded = true;
>>>> -        lib->name = internal_strdup(module.data());
>>>> +        lib->name = internal_strdup(mod.full_name());
>>>>          const uptr idx = atomic_load(&loaded_count_, memory_order_relaxed);
>>>> -        code_ranges_[idx].begin = b;
>>>> -        code_ranges_[idx].end = e;
>>>> +        code_ranges_[idx].begin = range.beg;
>>>> +        code_ranges_[idx].end = range.end;
>>>>          atomic_store(&loaded_count_, idx + 1, memory_order_release);
>>>> +        break;
>>>>        }
>>>>      }
>>>>      if (lib->loaded && !loaded) {
>>>>
>>>> Added: compiler-rt/trunk/test/tsan/ignore_lib5.cc
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib5.cc?rev=287632&view=auto
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/test/tsan/ignore_lib5.cc (added)
>>>> +++ compiler-rt/trunk/test/tsan/ignore_lib5.cc Tue Nov 22 03:49:11 2016
>>>> @@ -0,0 +1,65 @@
>>>> +// RUN: %clangxx_tsan -O1 %s -DLIB -fPIC -fno-sanitize=thread -shared -o %T/libignore_lib1.so
>>>> +// RUN: %clangxx_tsan -O1 %s -o %t
>>>> +// RUN: echo running w/o suppressions:
>>>> +// RUN: %deflake %run %t | FileCheck %s --check-prefix=CHECK-NOSUPP
>>>> +// RUN: echo running with suppressions:
>>>> +// RUN: %env_tsan_opts=suppressions='%s.supp' %run %t 2>&1 | FileCheck %s --check-prefix=CHECK-WITHSUPP
>>>> +
>>>> +// REQUIRES: stable-runtime
>>>> +
>>>> +// Previously the test episodically failed with:
>>>> +//   ThreadSanitizer: called_from_lib suppression '/libignore_lib1.so$' is
>>>> +//   matched against 2 libraries: '/libignore_lib1.so' and '/libignore_lib1.so'
>>>> +// This was caused by non-atomicity of reading of /proc/self/maps.
>>>> +
>>>> +#ifndef LIB
>>>> +
>>>> +#include <dlfcn.h>
>>>> +#include <sys/mman.h>
>>>> +#include <stdlib.h>
>>>> +#include <stdio.h>
>>>> +#include <errno.h>
>>>> +#include <libgen.h>
>>>> +#include <string>
>>>> +#include "test.h"
>>>> +
>>>> +void *thr(void *arg) {
>>>> +  // This thread creates lots of separate mappings in /proc/self/maps before
>>>> +  // the ignored library.
>>>> +  for (int i = 0; i < 10000; i++) {
>>>> +    if (i == 5000)
>>>> +      barrier_wait(&barrier);
>>>> +    mmap(0, 4096, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE | MAP_32BIT, -1 , 0);
>>>> +    mmap(0, 4096, PROT_WRITE, MAP_ANONYMOUS | MAP_PRIVATE | MAP_32BIT, -1 , 0);
>>>> +  }
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +int main(int argc, char **argv) {
>>>> +  barrier_init(&barrier, 2);
>>>> +  pthread_t th;
>>>> +  pthread_create(&th, 0, thr, 0);
>>>> +  barrier_wait(&barrier);
>>>> +  std::string lib = std::string(dirname(argv[0])) + "/libignore_lib1.so";
>>>> +  void *h = dlopen(lib.c_str(), RTLD_GLOBAL | RTLD_NOW);
>>>> +  if (h == 0)
>>>> +    exit(printf("failed to load the library (%d)\n", errno));
>>>> +  void (*f)() = (void(*)())dlsym(h, "libfunc");
>>>> +  if (f == 0)
>>>> +    exit(printf("failed to find the func (%d)\n", errno));
>>>> +  pthread_join(th, 0);
>>>> +  f();
>>>> +}
>>>> +
>>>> +#else  // #ifdef LIB
>>>> +
>>>> +#include "ignore_lib_lib.h"
>>>> +
>>>> +#endif  // #ifdef LIB
>>>> +
>>>> +// CHECK-NOSUPP: WARNING: ThreadSanitizer: data race
>>>> +// CHECK-NOSUPP: OK
>>>> +
>>>> +// CHECK-WITHSUPP-NOT: WARNING: ThreadSanitizer: data race
>>>> +// CHECK-WITHSUPP: OK
>>>> +
>>>>
>>>> Added: compiler-rt/trunk/test/tsan/ignore_lib5.cc.supp
>>>> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/ignore_lib5.cc.supp?rev=287632&view=auto
>>>> ==============================================================================
>>>> --- compiler-rt/trunk/test/tsan/ignore_lib5.cc.supp (added)
>>>> +++ compiler-rt/trunk/test/tsan/ignore_lib5.cc.supp Tue Nov 22 03:49:11 2016
>>>> @@ -0,0 +1,2 @@
>>>> +called_from_lib:/libignore_lib1.so$
>>>> +
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list