[PATCH] [ASan] Add process basename to log name and error message to simplify analysis of sanitized systems logs.

Kostya Serebryany kcc at google.com
Tue Mar 3 16:05:45 PST 2015


REPOSITORY
  rL LLVM

================
Comment at: lib/sanitizer_common/sanitizer_common.cc:357
@@ +356,3 @@
+void CacheBinaryName() {
+  BlockingMutexLock l(&binary_name_cache_lock);
+  if (!binary_name_cache_initialized) {
----------------
ygribov wrote:
> kcc wrote:
> > So why do you still need the Mutex if you are calling this only once at init time? 
> > Just add an asssertion that you've never initialized this before. 
> > Then you also don't need binary_name_cache_initialized, you can use binary_name_cache_str[0], 
> > which in turn could be function-scope because no one else needs it. 
> > 
> I thought about that but I decided to keep CacheBinaryName more generic so that it could be used in other contexts besides pure program startup. I could remove this if you think it's unnecessary.
Yes, I'd prefer to simplify it and make sure it crashes if called twice, or while other threads are running (or at least after the init is done)

================
Comment at: lib/sanitizer_common/sanitizer_symbolizer_posix_libcdep.cc:647
@@ -646,7 +646,3 @@
   void PrepareForSandboxing() override {
-#if SANITIZER_LINUX && !SANITIZER_ANDROID
-    BlockingMutexLock l(&mu_);
-    // Cache /proc/self/exe on Linux.
     CacheBinaryName();
   }
----------------
ygribov wrote:
> kcc wrote:
> > So, do we still need this here? 
> I don't know the whole sandboxing thing well enough so I kept it.
This should be safe, especially if the tests pass :) 
Also, if you ensure that CacheBinaryName asserts on the 2-nd call, this call site will stop working. 

================
Comment at: test/asan/TestCases/verbose-log-path_test.cc:16
@@ +15,3 @@
+int main(int argc, char **argv) {
+  if (argc > 2) return 0;
+  char *x = (char*)malloc(10);
----------------
ygribov wrote:
> kcc wrote:
> > this test could be smaller, e.g. 
> > int glob[10];
> > int main(int argc, char **argv) {
> >   return glob[argc * 10]; 
> > }
> That's a blind copy-paste of similar log_path test without log_exe_name.
Sure. Please make this one simpler.

http://reviews.llvm.org/D7333

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list