[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