tsan: symbolize global variables

llvm-svn: 172181

8 files changed, 174 insertions, 129 deletions

diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc
index d52cd07499a..a1d95ae0e0b 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cc
@@ -68,7 +68,18 @@ static const char *ExtractInt(const char *str, const char *delims,
   char *buff;
   const char *ret = ExtractToken(str, delims, &buff);
   if (buff != 0) {
-    *result = internal_atoll(buff);
+    *result = (int)internal_atoll(buff);
+  }
+  InternalFree(buff);
+  return ret;
+}
+
+static const char *ExtractUptr(const char *str, const char *delims,
+                               uptr *result) {
+  char *buff;
+  const char *ret = ExtractToken(str, delims, &buff);
+  if (buff != 0) {
+    *result = (uptr)internal_atoll(buff);
   }
   InternalFree(buff);
   return ret;
@@ -98,66 +109,15 @@ class ExternalSymbolizer {
     CHECK_NE(output_fd_, kInvalidFd);
   }
 
-  // Returns the number of frames for a given address, or zero if
-  // symbolization failed.
-  uptr SymbolizeCode(uptr addr, const char *module_name, uptr module_offset,
-                     AddressInfo *frames, uptr max_frames) {
+  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
     CHECK(module_name);
-    // FIXME: Make sure this buffer always has sufficient size to hold
-    // large debug info.
-    static const int kMaxBufferSize = 4096;
-    InternalScopedBuffer<char> buffer(kMaxBufferSize);
-    char *buffer_data = buffer.data();
-    internal_snprintf(buffer_data, kMaxBufferSize, "%s 0x%zx\n",
-                      module_name, module_offset);
-    if (!writeToSymbolizer(buffer_data, internal_strlen(buffer_data)))
+    internal_snprintf(buffer_, kBufferSize, "%s%s 0x%zx\n",
+                      is_data ? "DATA " : "", module_name, module_offset);
+    if (!writeToSymbolizer(buffer_, internal_strlen(buffer_)))
       return 0;
-
-    if (!readFromSymbolizer(buffer_data, kMaxBufferSize))
+    if (!readFromSymbolizer(buffer_, kBufferSize))
       return 0;
-    const char *str = buffer_data;
-    uptr frame_id;
-    CHECK_GT(max_frames, 0);
-    for (frame_id = 0; frame_id < max_frames; frame_id++) {
-      AddressInfo *info = &frames[frame_id];
-      char *function_name = 0;
-      str = ExtractToken(str, "\n", &function_name);
-      CHECK(function_name);
-      if (function_name[0] == '\0') {
-        // There are no more frames.
-        break;
-      }
-      info->Clear();
-      info->FillAddressAndModuleInfo(addr, module_name, module_offset);
-      info->function = function_name;
-      // Parse <file>:<line>:<column> buffer.
-      char *file_line_info = 0;
-      str = ExtractToken(str, "\n", &file_line_info);
-      CHECK(file_line_info);
-      const char *line_info = ExtractToken(file_line_info, ":", &info->file);
-      line_info = ExtractInt(line_info, ":", &info->line);
-      line_info = ExtractInt(line_info, "", &info->column);
-      InternalFree(file_line_info);
-
-      // Functions and filenames can be "??", in which case we write 0
-      // to address info to mark that names are unknown.
-      if (0 == internal_strcmp(info->function, "??")) {
-        InternalFree(info->function);
-        info->function = 0;
-      }
-      if (0 == internal_strcmp(info->file, "??")) {
-        InternalFree(info->file);
-        info->file = 0;
-      }
-    }
-    if (frame_id == 0) {
-      // Make sure we return at least one frame.
-      AddressInfo *info = &frames[0];
-      info->Clear();
-      info->FillAddressAndModuleInfo(addr, module_name, module_offset);
-      frame_id = 1;
-    }
-    return frame_id;
+    return buffer_;
   }
 
   bool Restart() {
@@ -191,6 +151,7 @@ class ExternalSymbolizer {
     }
     return true;
   }
+
   bool writeToSymbolizer(const char *buffer, uptr length) {
     if (length == 0)
       return true;
@@ -206,6 +167,9 @@ class ExternalSymbolizer {
   int input_fd_;
   int output_fd_;
 
+  static const uptr kBufferSize = 16 * 1024;
+  char buffer_[kBufferSize];
+
   static const uptr kMaxTimesRestarted = 5;
   uptr times_restarted_;
 };
@@ -222,30 +186,8 @@ class Symbolizer {
       return 0;
     const char *module_name = module->full_name();
     uptr module_offset = addr - module->base_address();
-    uptr actual_frames = 0;
-    if (external_symbolizer_ == 0) {
-      ReportExternalSymbolizerError(
-          "WARNING: Trying to symbolize code, but external "
-          "symbolizer is not initialized!\n");
-    } else {
-      while (true) {
-        actual_frames = external_symbolizer_->SymbolizeCode(
-            addr, module_name, module_offset, frames, max_frames);
-        if (actual_frames > 0) {
-          // Symbolization was successful.
-          break;
-        }
-        // Try to restart symbolizer subprocess. If we don't succeed, forget
-        // about it and don't try to use it later.
-        if (!external_symbolizer_->Restart()) {
-          ReportExternalSymbolizerError(
-              "WARNING: Failed to use and restart external symbolizer!\n");
-          external_symbolizer_ = 0;
-          break;
-        }
-      }
-    }
-    if (external_symbolizer_ == 0) {
+    const char *str = SendCommand(false, module_name, module_offset);
+    if (str == 0) {
       // External symbolizer was not initialized or failed. Fill only data
       // about module name and offset.
       AddressInfo *info = &frames[0];
@@ -253,17 +195,66 @@ class Symbolizer {
       info->FillAddressAndModuleInfo(addr, module_name, module_offset);
       return 1;
     }
-    // Otherwise, the data was filled by external symbolizer.
-    return actual_frames;
+    uptr frame_id = 0;
+    for (frame_id = 0; frame_id < max_frames; frame_id++) {
+      AddressInfo *info = &frames[frame_id];
+      char *function_name = 0;
+      str = ExtractToken(str, "\n", &function_name);
+      CHECK(function_name);
+      if (function_name[0] == '\0') {
+        // There are no more frames.
+        break;
+      }
+      info->Clear();
+      info->FillAddressAndModuleInfo(addr, module_name, module_offset);
+      info->function = function_name;
+      // Parse <file>:<line>:<column> buffer.
+      char *file_line_info = 0;
+      str = ExtractToken(str, "\n", &file_line_info);
+      CHECK(file_line_info);
+      const char *line_info = ExtractToken(file_line_info, ":", &info->file);
+      line_info = ExtractInt(line_info, ":", &info->line);
+      line_info = ExtractInt(line_info, "", &info->column);
+      InternalFree(file_line_info);
+
+      // Functions and filenames can be "??", in which case we write 0
+      // to address info to mark that names are unknown.
+      if (0 == internal_strcmp(info->function, "??")) {
+        InternalFree(info->function);
+        info->function = 0;
+      }
+      if (0 == internal_strcmp(info->file, "??")) {
+        InternalFree(info->file);
+        info->file = 0;
+      }
+    }
+    if (frame_id == 0) {
+      // Make sure we return at least one frame.
+      AddressInfo *info = &frames[0];
+      info->Clear();
+      info->FillAddressAndModuleInfo(addr, module_name, module_offset);
+      frame_id = 1;
+    }
+    return frame_id;
   }
 
-  bool SymbolizeData(uptr addr, AddressInfo *frame) {
+  bool SymbolizeData(uptr addr, DataInfo *info) {
     LoadedModule *module = FindModuleForAddress(addr);
     if (module == 0)
       return false;
     const char *module_name = module->full_name();
     uptr module_offset = addr - module->base_address();
-    frame->FillAddressAndModuleInfo(addr, module_name, module_offset);
+    internal_memset(info, 0, sizeof(*info));
+    info->address = addr;
+    info->module = internal_strdup(module_name);
+    info->module_offset = module_offset;
+    const char *str = SendCommand(true, module_name, module_offset);
+    if (str == 0)
+      return true;
+    str = ExtractToken(str, "\n", &info->name);
+    str = ExtractUptr(str, " ", &info->start);
+    str = ExtractUptr(str, "\n", &info->size);
+    info->start += module->base_address();
     return true;
   }
 
@@ -278,6 +269,29 @@ class Symbolizer {
   }
 
  private:
+  char *SendCommand(bool is_data, const char *module_name, uptr module_offset) {
+    if (external_symbolizer_ == 0) {
+      ReportExternalSymbolizerError(
+          "WARNING: Trying to symbolize code, but external "
+          "symbolizer is not initialized!\n");
+      return 0;
+    }
+    for (;;) {
+      char *reply = external_symbolizer_->SendCommand(is_data, module_name,
+          module_offset);
+      if (reply)
+        return reply;
+      // Try to restart symbolizer subprocess. If we don't succeed, forget
+      // about it and don't try to use it later.
+      if (!external_symbolizer_->Restart()) {
+        ReportExternalSymbolizerError(
+            "WARNING: Failed to use and restart external symbolizer!\n");
+        external_symbolizer_ = 0;
+        return 0;
+      }
+    }
+  }
+
   LoadedModule *FindModuleForAddress(uptr address) {
     if (modules_ == 0) {
       modules_ = (LoadedModule*)(symbolizer_allocator.Allocate(
@@ -318,8 +332,8 @@ uptr SymbolizeCode(uptr address, AddressInfo *frames, uptr max_frames) {
   return symbolizer.SymbolizeCode(address, frames, max_frames);
 }
 
-bool SymbolizeData(uptr address, AddressInfo *frame) {
-  return symbolizer.SymbolizeData(address, frame);
+bool SymbolizeData(uptr address, DataInfo *info) {
+  return symbolizer.SymbolizeData(address, info);
 }
 
 bool InitializeExternalSymbolizer(const char *path_to_symbolizer) {
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
index abc84dfefc2..c26d621ea06 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
@@ -53,12 +53,21 @@ struct AddressInfo {
   }
 };
 
+struct DataInfo {
+  uptr address;
+  char *module;
+  uptr module_offset;
+  char *name;
+  uptr start;
+  uptr size;
+};
+
 // Fills at most "max_frames" elements of "frames" with descriptions
 // for a given address (in all inlined functions). Returns the number
 // of descriptions actually filled.
 // This function should NOT be called from two threads simultaneously.
 uptr SymbolizeCode(uptr address, AddressInfo *frames, uptr max_frames);
-bool SymbolizeData(uptr address, AddressInfo *frame);
+bool SymbolizeData(uptr address, DataInfo *info);
 
 // Attempts to demangle the provided C++ mangled name.
 const char *Demangle(const char *Name);
diff --git a/compiler-rt/lib/tsan/go/tsan_go.cc b/compiler-rt/lib/tsan/go/tsan_go.cc
index cfbe2573c57..360608a0cf1 100644
--- a/compiler-rt/lib/tsan/go/tsan_go.cc
+++ b/compiler-rt/lib/tsan/go/tsan_go.cc
@@ -35,7 +35,7 @@ bool IsExpectedReport(uptr addr, uptr size) {
 void internal_start_thread(void(*func)(void*), void *arg) {
 }
 
-ReportStack *SymbolizeData(uptr addr) {
+ReportLocation *SymbolizeData(uptr addr) {
   return 0;
 }
 
diff --git a/compiler-rt/lib/tsan/lit_tests/global_race.cc b/compiler-rt/lib/tsan/lit_tests/global_race.cc
new file mode 100644
index 00000000000..0892d07da2c
--- /dev/null
+++ b/compiler-rt/lib/tsan/lit_tests/global_race.cc
@@ -0,0 +1,25 @@
+// RUN: %clangxx_tsan -O1 %s -o %t && %t 2>&1 | FileCheck %s
+#include <pthread.h>
+#include <stdio.h>
+#include <stddef.h>
+
+int GlobalData[10];
+
+void *Thread(void *a) {
+  GlobalData[2] = 42;
+  return 0;
+}
+
+int main() {
+  fprintf(stderr, "addr=%p\n", GlobalData);
+  pthread_t t;
+  pthread_create(&t, 0, Thread, 0);
+  GlobalData[2] = 43;
+  pthread_join(t, 0);
+}
+
+// CHECK: addr=[[ADDR:0x[0-9,a-f]+]]
+// CHECK: WARNING: ThreadSanitizer: data race
+// Requires llvm-symbolizer, so disabled for now.
+// CHECK0: Location is global 'GlobalData' of size 40 at [[ADDR]]
+// CHECK0:                            (global_race.cc.exe+0x[0-9,a-f]+)
diff --git a/compiler-rt/lib/tsan/rtl/tsan_report.cc b/compiler-rt/lib/tsan/rtl/tsan_report.cc
index b79ab912554..0758cab59de 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_report.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_report.cc
@@ -104,9 +104,8 @@ static void PrintMop(const ReportMop *mop, bool first) {
 static void PrintLocation(const ReportLocation *loc) {
   char thrbuf[kThreadBufSize];
   if (loc->type == ReportLocationGlobal) {
-    Printf("  Location is global '%s' of size %zu at %zx %s:%d (%s+%p)\n\n",
-               loc->name, loc->size, loc->addr, loc->file, loc->line,
-               loc->module, loc->offset);
+    Printf("  Location is global '%s' of size %zu at %zx (%s+%p)\n\n",
+               loc->name, loc->size, loc->addr, loc->module, loc->offset);
   } else if (loc->type == ReportLocationHeap) {
     char thrbuf[kThreadBufSize];
     Printf("  Location is heap block of size %zu at %p allocated by %s:\n",
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
index 2d073be905c..65cf7868694 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc
@@ -277,22 +277,9 @@ void ScopedReport::AddLocation(uptr addr, uptr size) {
     return;
   }
 #endif
-  ReportStack *symb = SymbolizeData(addr);
-  if (symb) {
-    void *mem = internal_alloc(MBlockReportLoc, sizeof(ReportLocation));
-    ReportLocation *loc = new(mem) ReportLocation();
+  ReportLocation *loc = SymbolizeData(addr);
+  if (loc) {
     rep_->locs.PushBack(loc);
-    loc->type = ReportLocationGlobal;
-    loc->addr = addr;
-    loc->size = size;
-    loc->module = symb->module ? internal_strdup(symb->module) : 0;
-    loc->offset = symb->offset;
-    loc->tid = 0;
-    loc->name = symb->func ? internal_strdup(symb->func) : 0;
-    loc->file = symb->file ? internal_strdup(symb->file) : 0;
-    loc->line = symb->line;
-    loc->stack = 0;
-    internal_free(symb);
     return;
   }
 }
diff --git a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
index 48bee6774b4..29dfe237ffd 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
+++ b/compiler-rt/lib/tsan/rtl/tsan_symbolize.cc
@@ -29,21 +29,24 @@ ReportStack *NewReportStackEntry(uptr addr) {
   return ent;
 }
 
+// Strip module path to make output shorter.
+static char *StripModuleName(const char *module) {
+  if (module == 0)
+    return 0;
+  const char *short_module_name = internal_strrchr(module, '/');
+  if (short_module_name)
+    short_module_name += 1;
+  else
+    short_module_name = module;
+  return internal_strdup(short_module_name);
+}
+
 static ReportStack *NewReportStackEntry(const AddressInfo &info) {
   ReportStack *ent = NewReportStackEntry(info.address);
-  if (info.module) {
-    // Strip module path to make output shorter.
-    const char *short_module_name = internal_strrchr(info.module, '/');
-    if (short_module_name)
-      short_module_name += 1;
-    else
-      short_module_name = info.module;
-    ent->module = internal_strdup(short_module_name);
-  }
+  ent->module = StripModuleName(info.module);
   ent->offset = info.module_offset;
-  if (info.function) {
+  if (info.function)
     ent->func = internal_strdup(info.function);
-  }
   if (info.file)
     ent->file = internal_strdup(info.file);
   ent->line = info.line;
@@ -78,14 +81,23 @@ ReportStack *SymbolizeCode(uptr addr) {
   return SymbolizeCodeAddr2Line(addr);
 }
 
-ReportStack *SymbolizeData(uptr addr) {
-  if (flags()->external_symbolizer_path[0]) {
-    AddressInfo frame;
-    if (!__sanitizer::SymbolizeData(addr, &frame))
-      return 0;
-    return NewReportStackEntry(frame);
-  }
-  return SymbolizeDataAddr2Line(addr);
+ReportLocation *SymbolizeData(uptr addr) {
+  if (flags()->external_symbolizer_path[0] == 0)
+    return 0;
+  DataInfo info;
+  if (!__sanitizer::SymbolizeData(addr, &info))
+    return 0;
+  ReportLocation *ent = (ReportLocation*)internal_alloc(MBlockReportStack,
+                                                        sizeof(ReportLocation));
+  internal_memset(ent, 0, sizeof(*ent));
+  ent->type = ReportLocationGlobal;
+  ent->module = StripModuleName(info.module);
+  ent->offset = info.module_offset;
+  if (info.name)
+    ent->name = internal_strdup(info.name);
+  ent->addr = info.start;
+  ent->size = info.size;
+  return ent;
 }
 
 }  // namespace __tsan
diff --git a/compiler-rt/lib/tsan/rtl/tsan_symbolize.h b/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
index 115339be38a..29193043cd7 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
+++ b/compiler-rt/lib/tsan/rtl/tsan_symbolize.h
@@ -19,10 +19,9 @@
 namespace __tsan {
 
 ReportStack *SymbolizeCode(uptr addr);
-ReportStack *SymbolizeData(uptr addr);
+ReportLocation *SymbolizeData(uptr addr);
 
 ReportStack *SymbolizeCodeAddr2Line(uptr addr);
-ReportStack *SymbolizeDataAddr2Line(uptr addr);
 
 ReportStack *NewReportStackEntry(uptr addr);