[PATCH] tsan: ignore interceptors coming from specified libraries

Dmitry Vyukov dvyukov at google.com
Thu Oct 3 06:16:56 PDT 2013



================
Comment at: sanitizer_common/sanitizer_libignore.h:15
@@ +14,3 @@
+//
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_LIBIGNORE_H
----------------
Alexander Potapenko wrote:
> Need a newline after this comment.
Done

================
Comment at: sanitizer_common/sanitizer_libignore.h:66
@@ +65,3 @@
+
+  LibIgnore(const LibIgnore&);  // not implemented
+  void operator = (const LibIgnore&);  // not implemented
----------------
Alexander Potapenko wrote:
> I suggest to remove the declarations of non-implemented functions.
Then it will be possible to copy objects of this class, something that I want to prevent.

================
Comment at: sanitizer_common/sanitizer_libignore.cc:48
@@ +47,3 @@
+      if ((prot & MemoryMappingLayout::kProtectionExecute) != 0
+          && TemplateMatch(lib->templ, fn.data())) {
+        if (loaded) {
----------------
Alexander Potapenko wrote:
> Shouldn't '&&' be on the previous line?
Done

================
Comment at: sanitizer_common/sanitizer_libignore.cc:51
@@ +50,3 @@
+          Report("%s: called_from_lib suppression '%s' is matched against"
+              " 2 libraries: '%s' and '%s'\n",
+              SanitizerToolName, lib->templ, lib->name, fn.data());
----------------
Alexander Potapenko wrote:
> Style nit: the quotes will look much prettier if you align them with the previous line (if you choose to do this, make sure to align the next line similarly). Ditto below.
Done

================
Comment at: sanitizer_common/sanitizer_libignore.cc:56
@@ +55,3 @@
+        loaded = true;
+        if (lib->loaded == false) {
+          lib->loaded = true;
----------------
Alexander Potapenko wrote:
> (!lib->loaded) ?
Done

================
Comment at: sanitizer_common/sanitizer_libignore.cc:18
@@ +17,3 @@
+
+LibIgnore::LibIgnore(LinkerInitialized) {
+}
----------------
Alexander Potapenko wrote:
> Perhaps move the ctor into the header?
Why?

================
Comment at: sanitizer_common/sanitizer_libignore.cc:29
@@ +28,3 @@
+      continue;
+    CHECK_LT(count_, kMaxLibs);
+    Lib *lib = &libs_[count_++];
----------------
Alexander Potapenko wrote:
> This check can be easily triggered by the user if he adds many suppressions to the file.
> Can you print a meaningful error report in this case?
Done

================
Comment at: tsan/lit_tests/ignore_lib1.cc:22
@@ +21,3 @@
+  std::string lib = std::string(dirname(argv[0])) + "/libignore_lib1.so";
+  void *h = dlopen(lib.c_str(), RTLD_GLOBAL|RTLD_NOW);
+  if (h == 0)
----------------
Alexander Potapenko wrote:
> pls add spaces around "|" here and below
Done

================
Comment at: tsan/lit_tests/ignore_lib_lib.h:21
@@ +20,3 @@
+  len = 10;
+    __atomic_store_n(&mem, malloc(len), __ATOMIC_RELEASE);
+  pthread_join(t, 0);
----------------
Alexander Potapenko wrote:
> Please fix the indentation
Done

================
Comment at: tsan/rtl/tsan_rtl.h:34
@@ -33,2 +33,3 @@
 #include "sanitizer_common/sanitizer_thread_registry.h"
+#include "sanitizer_common/sanitizer_libignore.h"
 #include "tsan_clock.h"
----------------
Alexander Potapenko wrote:
> Mind the alphabetical order of includes.
Done


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



More information about the llvm-commits mailing list