[PATCH] tsan: ignore interceptors coming from specified libraries

Alexander Potapenko glider at google.com
Thu Oct 3 06:03:46 PDT 2013



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

================
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)
----------------
pls add spaces around "|" here and below

================
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);
----------------
Please fix the indentation

================
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"
----------------
Mind the alphabetical order of includes.

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

================
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());
----------------
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.

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

================
Comment at: sanitizer_common/sanitizer_libignore.h:66
@@ +65,3 @@
+
+  LibIgnore(const LibIgnore&);  // not implemented
+  void operator = (const LibIgnore&);  // not implemented
----------------
I suggest to remove the declarations of non-implemented functions.

================
Comment at: sanitizer_common/sanitizer_libignore.cc:29
@@ +28,3 @@
+      continue;
+    CHECK_LT(count_, kMaxLibs);
+    Lib *lib = &libs_[count_++];
----------------
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?

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


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



More information about the llvm-commits mailing list