[compiler-rt] r281969 - tsan: make CHECK more robust

Kostya Serebryany via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 22 12:26:47 PDT 2016


On Thu, Sep 22, 2016 at 12:09 AM, Dmitry Vyukov <dvyukov at google.com> wrote:

> Is this cost larger than the cost I outlined above?
>
IMHO -- yes. As long as we have the fake functions
with CHECK(0) the will be no possibility for them to harm


>
>
> On Wed, Sep 21, 2016 at 7:06 PM, Kostya Serebryany <kcc at google.com> wrote:
> > #ifdefs are much harder to read and they make code refactoring more
> > involved.
> > Essentially, every new #ifdef add to our technical debt.
> >
> > On Tue, Sep 20, 2016 at 11:33 PM, Dmitry Vyukov <dvyukov at google.com>
> wrote:
> >>
> >> On Tue, Sep 20, 2016 at 10:57 PM, Kostya Serebryany <kcc at google.com>
> >> wrote:
> >> > Then make it have a fake one.
> >> > These #ifs cost us very much in the long run.
> >>
> >> What is that cost? I can't remember any cases where they added cost.
> >>
> >> I remember cases when Go runtime become broken (don't built/link). But
> >> in all these cases the right answer is to exclude more code from Go
> >> build, rather than to pull in more code. Providing fake stubs would
> >> cause bloat with unnecessary functions, which now build successfully
> >> so nobody cares.
> >>
> >> If we introduce fake cur_thread() for Go, _it_ has good chances to add
> >> visible cost due to additional bugs. Looking at existing usages of
> >> cur_thread(), accidentally using a fake cur_thread() for Go would be
> >> very wrong. For example, calling HACKY_CALL(trace_switch) in Go would
> >> cause operating on a wrong ThreadState -- we don't need the fake one,
> >> we need the real one. For internal deadlock detector using a global
> >> fake ThreadState would cause bad races on the object.
> >>
> >> I don't see how it is better and what is the current cost of #if's.
> >>
> >>
> >>
> >> > On Tue, Sep 20, 2016 at 1:56 PM, Dmitry Vyukov <dvyukov at google.com>
> >> > wrote:
> >> >>
> >> >> No, because Go does not have cur_thread().
> >> >>
> >> >> On Tue, Sep 20, 2016 at 11:54 PM, Kostya Serebryany <kcc at google.com>
> >> >> wrote:
> >> >> > Please, can we have if() instead of #if?
> >> >> >
> >> >> > On Tue, Sep 20, 2016 at 6:28 AM, Dmitry Vyukov via llvm-commits
> >> >> > <llvm-commits at lists.llvm.org> wrote:
> >> >> >>
> >> >> >> Author: dvyukov
> >> >> >> Date: Tue Sep 20 08:28:20 2016
> >> >> >> New Revision: 281969
> >> >> >>
> >> >> >> URL: http://llvm.org/viewvc/llvm-project?rev=281969&view=rev
> >> >> >> Log:
> >> >> >> tsan: make CHECK more robust
> >> >> >>
> >> >> >> Enable more ignores when we start crashing.
> >> >> >> Unwind in CHECK SIGSEGVs if happens early:
> >> >> >>
> >> >> >> FATAL: ThreadSanitizer CHECK failed:
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cc:105
> >> >> >> "((beg)) <=
> >> >> >> ((end))" (0x8000000000, 0x4000000000)
> >> >> >> Program received signal SIGSEGV, Segmentation fault.
> >> >> >> __tsan::MetaMap::GetAndLock (this=0x1337c88
> >> >> >> <__tsan::ctx_placeholder+8>,
> >> >> >> thr=thr at entry=0x7ffff7f91800, pc=pc at entry=4639488,
> >> >> >> addr=addr at entry=140737339086992, write_lock=write_lock at entry=
> true,
> >> >> >>     create=create at entry=true) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_sync.cc:208
> >> >> >> 208       u32 idx0 = *meta;
> >> >> >> (gdb) bt
> >> >> >> #0  __tsan::MetaMap::GetAndLock (this=0x1337c88
> >> >> >> <__tsan::ctx_placeholder+8>, thr=thr at entry=0x7ffff7f91800,
> >> >> >> pc=pc at entry=4639488, addr=addr at entry=140737339086992,
> >> >> >> write_lock=write_lock at entry=true,
> >> >> >>     create=create at entry=true) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_sync.cc:208
> >> >> >> #1  0x00000000004a965f in __tsan::MetaMap::GetOrCreateAndLock
> >> >> >> (this=<optimized out>, thr=thr at entry=0x7ffff7f91800,
> >> >> >> pc=pc at entry=4639488,
> >> >> >> addr=addr at entry=140737339086992, write_lock=write_lock at entry=
> true)
> >> >> >>     at ../projects/compiler-rt/lib/tsan/rtl/tsan_sync.cc:198
> >> >> >> #2  0x00000000004a162a in __tsan::Release (thr=0x7ffff7f91800,
> >> >> >> pc=pc at entry=4639488, addr=addr at entry=140737339086992) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_rtl_mutex.cc:395
> >> >> >> #3  0x000000000046cc40 in __interceptor_pthread_once
> >> >> >> (o=0x7ffff71a5890
> >> >> >> <once_regsizes>, f=0x7ffff6f9d9c0 <init_dwarf_reg_size_table>) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1334
> >> >> >> #4  0x00007ffff6f9fe86 in __gthread_once (__func=0x7ffff6f9d9c0
> >> >> >> <init_dwarf_reg_size_table>, __once=0x7ffff71a5890
> <once_regsizes>)
> >> >> >> at
> >> >> >> ./gthr-default.h:699
> >> >> >> #5  uw_init_context_1 (context=context at entry=0x7fffffffd6d0,
> >> >> >> outer_cfa=outer_cfa at entry=0x7fffffffd980, outer_ra=0x437d13
> >> >> >> <__sanitizer::BufferedStackTrace::SlowUnwindStack(unsigned long,
> >> >> >> unsigned
> >> >> >> int)+67>)
> >> >> >>     at ../../../src/libgcc/unwind-dw2.c:1572
> >> >> >> #6  0x00007ffff6fa06a8 in _Unwind_Backtrace (trace=0x437c30
> >> >> >> <__sanitizer::Unwind_Trace(_Unwind_Context*, void*)>,
> >> >> >> trace_argument=0x7fffffffd980) at ../../../src/libgcc/unwind.
> inc:283
> >> >> >> #7  0x0000000000437d13 in
> >> >> >> __sanitizer::BufferedStackTrace::SlowUnwindStack
> >> >> >> (this=0x7ffff6103208, pc=pc at entry=4863574,
> >> >> >> max_depth=max_depth at entry=256)
> >> >> >>     at
> >> >> >>
> >> >> >>
> >> >> >> ../projects/compiler-rt/lib/sanitizer_common/sanitizer_
> unwind_linux_libcdep.cc:125
> >> >> >> #8  0x0000000000434f4a in __sanitizer::BufferedStackTrace::Unwind
> >> >> >> (this=this at entry=0x7ffff6103208, max_depth=max_depth at entry=256,
> >> >> >> pc=pc at entry=4863574, bp=bp at entry=0, context=context at entry=0x0,
> >> >> >>     stack_top=stack_top at entry=0, stack_bottom=stack_bottom@
> entry=0,
> >> >> >> request_fast_unwind=request_fast_unwind at entry=false) at
> >> >> >>
> >> >> >>
> >> >> >> ../projects/compiler-rt/lib/sanitizer_common/sanitizer_
> stacktrace_libcdep.cc:76
> >> >> >> #9  0x00000000004a36b3 in PrintCurrentStackSlow (pc=4863574) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc:696
> >> >> >> #10 __tsan::TsanCheckFailed (file=<optimized out>, line=<optimized
> >> >> >> out>,
> >> >> >> cond=<optimized out>, v1=<optimized out>, v2=<optimized out>) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_rtl_report.cc:44
> >> >> >> #11 0x000000000042dfd6 in __sanitizer::CheckFailed
> >> >> >> (file=file at entry=0x4b9fd0
> >> >> >> "../projects/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cc",
> >> >> >> line=line at entry=105,
> >> >> >>     cond=cond at entry=0x4ba049 "((beg)) <= ((end))",
> >> >> >> v1=v1 at entry=549755813888, v2=v2 at entry=274877906944) at
> >> >> >>
> >> >> >>
> >> >> >> ../projects/compiler-rt/lib/sanitizer_common/sanitizer_
> termination.cc:79
> >> >> >> #12 0x00000000004aa36c in ProtectRange (end=274877906944,
> >> >> >> beg=549755813888) at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cc:105
> >> >> >> #13 __tsan::CheckAndProtect () at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_platform_posix.cc:133
> >> >> >> #14 0x00000000004a9e95 in __tsan::InitializePlatform () at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cc:280
> >> >> >> #15 0x0000000000497e73 in __tsan::Initialize (thr=0x7ffff7f91800)
> at
> >> >> >> ../projects/compiler-rt/lib/tsan/rtl/tsan_rtl.cc:343
> >> >> >> #16 0x00007ffff7dea25a in _dl_init (main_map=0x7ffff7ffe1c8,
> argc=1,
> >> >> >> argv=0x7fffffffdb88, env=0x7fffffffdb98) at dl-init.c:111
> >> >> >> #17 0x00007ffff7ddb30a in _dl_start_user () at rtld.c:871
> >> >> >>
> >> >> >>
> >> >> >> Modified:
> >> >> >>     compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
> >> >> >>
> >> >> >> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc
> >> >> >> URL:
> >> >> >>
> >> >> >>
> >> >> >> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/
> tsan/rtl/tsan_rtl_report.cc?rev=281969&r1=281968&r2=281969&view=diff
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> ============================================================
> ==================
> >> >> >> --- compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc (original)
> >> >> >> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_rtl_report.cc Tue Sep 20
> >> >> >> 08:28:20
> >> >> >> 2016
> >> >> >> @@ -38,6 +38,10 @@ void TsanCheckFailed(const char *file, i
> >> >> >>    // on the other hand there is no sense in processing
> interceptors
> >> >> >>    // since we are going to die soon.
> >> >> >>    ScopedIgnoreInterceptors ignore;
> >> >> >> +#ifndef SANITIZER_GO
> >> >> >> +  cur_thread()->ignore_sync++;
> >> >> >> +  cur_thread()->ignore_reads_and_writes++;
> >> >> >> +#endif
> >> >> >>    Printf("FATAL: ThreadSanitizer CHECK failed: "
> >> >> >>           "%s:%d \"%s\" (0x%zx, 0x%zx)\n",
> >> >> >>           file, line, cond, (uptr)v1, (uptr)v2);
> >> >> >>
> >> >> >>
> >> >> >> _______________________________________________
> >> >> >> llvm-commits mailing list
> >> >> >> llvm-commits at lists.llvm.org
> >> >> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >> >> >
> >> >> >
> >> >
> >> >
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160922/3eaaefae/attachment.html>


More information about the llvm-commits mailing list