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