[compiler-rt] r240539 - tsan: don't print external PCs in reports
Alexey Samsonov
vonosmas at gmail.com
Thu Jun 25 12:00:48 PDT 2015
On Thu, Jun 25, 2015 at 4:47 AM, Dmitry Vyukov <dvyukov at google.com> wrote:
> On Wed, Jun 24, 2015 at 8:21 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
> >
> > On Wed, Jun 24, 2015 at 6:04 AM, Dmitry Vyukov <dvyukov at google.com>
> wrote:
> >>
> >> Author: dvyukov
> >> Date: Wed Jun 24 08:04:12 2015
> >> New Revision: 240539
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=240539&view=rev
> >> Log:
> >> tsan: don't print external PCs in reports
> >>
> >> They are meaningless.
> >>
> >>
> >> Added:
> >> compiler-rt/trunk/test/tsan/java_symbolization.cc
> >> Modified:
> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
> >>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
> >> compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc
> >> compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h
> >> compiler-rt/trunk/test/tsan/java.h
> >>
> >> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=240539&r1=240538&r2=240539&view=diff
> >>
> >>
> ==============================================================================
> >> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
> >> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Wed Jun 24
> >> 08:04:12 2015
> >> @@ -44,6 +44,10 @@ static const uptr kMaxNumberOfModules =
> >>
> >> const uptr kMaxThreadStackSize = 1 << 30; // 1Gb
> >>
> >> +// Denotes fake PC values that come from JIT/JAVA/etc.
> >> +// For such PC values __tsan_symbolize_external() will be called.
> >> +const uptr kExternalPCBit = 1ULL << 60;
> >> +
> >
> >
> > I think you should instead introduce a function IsExternalPC(), and hide
> > this magic constant in .cc file as an implementation detail.
>
> That will increase code size, introduce an additional indirection and
> don't allow grepping for constant value. What are the benefits?
>
Well, at least add a comment about the expected use of this constant. Who
is expected to set this bit, and who
should check it?
> >> extern const char *SanitizerToolName; // Can be changed by the tool.
> >>
> >> extern atomic_uint32_t current_verbosity;
> >>
> >> Modified:
> >> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc?rev=240539&r1=240538&r2=240539&view=diff
> >>
> >>
> ==============================================================================
> >> ---
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
> >> (original)
> >> +++
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc
> >> Wed Jun 24 08:04:12 2015
> >> @@ -99,7 +99,9 @@ void RenderFrame(InternalScopedString *b
> >> break;
> >> case 'M':
> >> // Module basename and offset, or PC.
> >> - if (info.module)
> >> + if (info.address & kExternalPCBit)
> >> + {} // There PCs are not meaningful.
> >> + else if (info.module)
> >> buffer->append("(%s+%p)", StripModuleName(info.module),
> >> (void *)info.module_offset);
> >> else
> >>
> >> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc?rev=240539&r1=240538&r2=240539&view=diff
> >>
> >>
> ==============================================================================
> >> --- compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc (original)
> >> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc Wed Jun 24 08:04:12
> >> 2015
> >> @@ -44,7 +44,7 @@ extern "C" bool __tsan_symbolize_externa
> >> int *line, int *col)
> >> SANITIZER_WEAK_ATTRIBUTE;
> >>
> >> -bool __tsan_symbolize_external(uptr pc,
> >> +bool WEAK __tsan_symbolize_external(uptr pc,
> >> char *func_buf, uptr func_siz,
> >> char *file_buf, uptr file_siz,
> >> int *line, int *col) {
> >
> >
> > Can't you combine a declaration and definition here?
> > extern "C" bool __tsan_symbolize_external(...) SANITIZER_WEAK_ATTRIBUTE {
> > return false;
> > }
>
>
> Done in r240633.
>
> >>
> >>
> >> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h?rev=240539&r1=240538&r2=240539&view=diff
> >>
> >>
> ==============================================================================
> >> --- compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h (original)
> >> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h Wed Jun 24 08:04:12
> >> 2015
> >> @@ -18,10 +18,6 @@
> >>
> >> namespace __tsan {
> >>
> >> -// Denotes fake PC values that come from JIT/JAVA/etc.
> >> -// For such PC values __tsan_symbolize_external() will be called.
> >> -const uptr kExternalPCBit = 1ULL << 60;
> >> -
> >> void EnterSymbolizer();
> >> void ExitSymbolizer();
> >> SymbolizedStack *SymbolizeCode(uptr addr);
> >>
> >> Modified: compiler-rt/trunk/test/tsan/java.h
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/java.h?rev=240539&r1=240538&r2=240539&view=diff
> >>
> >>
> ==============================================================================
> >> --- compiler-rt/trunk/test/tsan/java.h (original)
> >> +++ compiler-rt/trunk/test/tsan/java.h Wed Jun 24 08:04:12 2015
> >> @@ -22,3 +22,5 @@ int __tsan_java_release_store(jptr addr
> >> void __tsan_read1_pc(jptr addr, jptr pc);
> >> void __tsan_write1_pc(jptr addr, jptr pc);
> >> }
> >> +
> >> +const jptr kExternalPCBit = 1ULL << 60;
> >>
> >> Added: compiler-rt/trunk/test/tsan/java_symbolization.cc
> >> URL:
> >>
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/java_symbolization.cc?rev=240539&view=auto
> >>
> >>
> ==============================================================================
> >> --- compiler-rt/trunk/test/tsan/java_symbolization.cc (added)
> >> +++ compiler-rt/trunk/test/tsan/java_symbolization.cc Wed Jun 24
> 08:04:12
> >> 2015
> >> @@ -0,0 +1,44 @@
> >> +// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s
> >> +#include "java.h"
> >> +#include <memory.h>
> >> +
> >> +extern "C" bool __tsan_symbolize_external(jptr pc,
> >> + char *func_buf, jptr
> func_siz,
> >> + char *file_buf, jptr
> file_siz,
> >> + int *line, int *col) {
> >> + if (pc == (1234 | kExternalPCBit)) {
> >> + memcpy(func_buf, "MyFunc", sizeof("MyFunc"));
> >> + memcpy(file_buf, "MyFile.java", sizeof("MyFile.java"));
> >> + *line = 1234;
> >> + *col = 56;
> >> + return true;
> >> + }
> >> + return false;
> >> +}
> >> +
> >> +void *Thread(void *p) {
> >> + barrier_wait(&barrier);
> >> + __tsan_write1_pc((jptr)p, 1234 | kExternalPCBit);
> >> + return 0;
> >> +}
> >> +
> >> +int main() {
> >> + barrier_init(&barrier, 2);
> >> + int const kHeapSize = 1024 * 1024;
> >> + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8;
> >> + __tsan_java_init(jheap, kHeapSize);
> >> + const int kBlockSize = 16;
> >> + __tsan_java_alloc(jheap, kBlockSize);
> >> + pthread_t th;
> >> + pthread_create(&th, 0, Thread, (void*)jheap);
> >> + __tsan_write1_pc((jptr)jheap, 1234 | kExternalPCBit);
> >> + barrier_wait(&barrier);
> >> + pthread_join(th, 0);
> >> + __tsan_java_free(jheap, kBlockSize);
> >> + fprintf(stderr, "DONE\n");
> >> + return __tsan_java_fini();
> >> +}
> >> +
> >> +// CHECK: WARNING: ThreadSanitizer: data race
> >> +// CHECK: #0 MyFunc MyFile.java:1234:56
> >> +// CHECK: DONE
>
--
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150625/68a9d5f2/attachment.html>
More information about the llvm-commits
mailing list