<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 25, 2015 at 4:47 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"><div class="HOEnZb"><div class="h5">On Wed, Jun 24, 2015 at 8:21 PM, Alexey Samsonov <<a href="mailto:vonosmas@gmail.com">vonosmas@gmail.com</a>> wrote:<br>
><br>
> On Wed, Jun 24, 2015 at 6:04 AM, Dmitry Vyukov <<a href="mailto:dvyukov@google.com">dvyukov@google.com</a>> wrote:<br>
>><br>
>> Author: dvyukov<br>
>> Date: Wed Jun 24 08:04:12 2015<br>
>> New Revision: 240539<br>
>><br>
>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D240539-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=39E32Ph06H0-kO3KHKLIi2UG5lG161Yp_uwiqKmucOc&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=240539&view=rev</a><br>
>> Log:<br>
>> tsan: don't print external PCs in reports<br>
>><br>
>> They are meaningless.<br>
>><br>
>><br>
>> Added:<br>
>> compiler-rt/trunk/test/tsan/java_symbolization.cc<br>
>> Modified:<br>
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h<br>
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc<br>
>> compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc<br>
>> compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h<br>
>> compiler-rt/trunk/test/tsan/java.h<br>
>><br>
>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_lib_sanitizer-5Fcommon_sanitizer-5Fcommon.h-3Frev-3D240539-26r1-3D240538-26r2-3D240539-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=BbIYvg3FxiAxUk_gWA9wigo66bBrqr4r-0dAug0VKdw&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=240539&r1=240538&r2=240539&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)<br>
>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Wed Jun 24<br>
>> 08:04:12 2015<br>
>> @@ -44,6 +44,10 @@ static const uptr kMaxNumberOfModules =<br>
>><br>
>> const uptr kMaxThreadStackSize = 1 << 30; // 1Gb<br>
>><br>
>> +// Denotes fake PC values that come from JIT/JAVA/etc.<br>
>> +// For such PC values __tsan_symbolize_external() will be called.<br>
>> +const uptr kExternalPCBit = 1ULL << 60;<br>
>> +<br>
><br>
><br>
> I think you should instead introduce a function IsExternalPC(), and hide<br>
> this magic constant in .cc file as an implementation detail.<br>
<br>
</div></div>That will increase code size, introduce an additional indirection and<br>
don't allow grepping for constant value. What are the benefits?<br></blockquote><div><br></div><div>Well, at least add a comment about the expected use of this constant. Who is expected to set this bit, and who<br></div><div>should check it?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5">
<br>
>> extern const char *SanitizerToolName; // Can be changed by the tool.<br>
>><br>
>> extern atomic_uint32_t current_verbosity;<br>
>><br>
>> Modified:<br>
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_lib_sanitizer-5Fcommon_sanitizer-5Fstacktrace-5Fprinter.cc-3Frev-3D240539-26r1-3D240538-26r2-3D240539-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=8WaHUnsnvg0bzjt5Sz6UfwgVN10-6fp9MhXTWjYiGxE&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc?rev=240539&r1=240538&r2=240539&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc<br>
>> (original)<br>
>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_stacktrace_printer.cc<br>
>> Wed Jun 24 08:04:12 2015<br>
>> @@ -99,7 +99,9 @@ void RenderFrame(InternalScopedString *b<br>
>> break;<br>
>> case 'M':<br>
>> // Module basename and offset, or PC.<br>
>> - if (info.module)<br>
>> + if (info.address & kExternalPCBit)<br>
>> + {} // There PCs are not meaningful.<br>
>> + else if (info.module)<br>
>> buffer->append("(%s+%p)", StripModuleName(info.module),<br>
>> (void *)info.module_offset);<br>
>> else<br>
>><br>
>> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_lib_tsan_rtl_tsan-5Fsymbolize.cc-3Frev-3D240539-26r1-3D240538-26r2-3D240539-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=IRyEIMIRLjl8KD8WL6opXdSgz2P_mN1OTKRMrRIlBxU&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc?rev=240539&r1=240538&r2=240539&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc (original)<br>
>> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.cc Wed Jun 24 08:04:12<br>
>> 2015<br>
>> @@ -44,7 +44,7 @@ extern "C" bool __tsan_symbolize_externa<br>
>> int *line, int *col)<br>
>> SANITIZER_WEAK_ATTRIBUTE;<br>
>><br>
>> -bool __tsan_symbolize_external(uptr pc,<br>
>> +bool WEAK __tsan_symbolize_external(uptr pc,<br>
>> char *func_buf, uptr func_siz,<br>
>> char *file_buf, uptr file_siz,<br>
>> int *line, int *col) {<br>
><br>
><br>
> Can't you combine a declaration and definition here?<br>
> extern "C" bool __tsan_symbolize_external(...) SANITIZER_WEAK_ATTRIBUTE {<br>
> return false;<br>
> }<br>
<br>
<br>
</div></div>Done in r240633.<br>
<div class="HOEnZb"><div class="h5"><br>
>><br>
>><br>
>> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_lib_tsan_rtl_tsan-5Fsymbolize.h-3Frev-3D240539-26r1-3D240538-26r2-3D240539-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=NG1Axk2c-bLsMehkCkOhZ2ulF-MKkbv1ycS8lrOEshM&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h?rev=240539&r1=240538&r2=240539&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h (original)<br>
>> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_symbolize.h Wed Jun 24 08:04:12<br>
>> 2015<br>
>> @@ -18,10 +18,6 @@<br>
>><br>
>> namespace __tsan {<br>
>><br>
>> -// Denotes fake PC values that come from JIT/JAVA/etc.<br>
>> -// For such PC values __tsan_symbolize_external() will be called.<br>
>> -const uptr kExternalPCBit = 1ULL << 60;<br>
>> -<br>
>> void EnterSymbolizer();<br>
>> void ExitSymbolizer();<br>
>> SymbolizedStack *SymbolizeCode(uptr addr);<br>
>><br>
>> Modified: compiler-rt/trunk/test/tsan/java.h<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_test_tsan_java.h-3Frev-3D240539-26r1-3D240538-26r2-3D240539-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=Ras7eRqON8cEow1xbRw7Ws9vkkbJWmWR5qWqHjNZB5o&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/java.h?rev=240539&r1=240538&r2=240539&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/test/tsan/java.h (original)<br>
>> +++ compiler-rt/trunk/test/tsan/java.h Wed Jun 24 08:04:12 2015<br>
>> @@ -22,3 +22,5 @@ int __tsan_java_release_store(jptr addr<br>
>> void __tsan_read1_pc(jptr addr, jptr pc);<br>
>> void __tsan_write1_pc(jptr addr, jptr pc);<br>
>> }<br>
>> +<br>
>> +const jptr kExternalPCBit = 1ULL << 60;<br>
>><br>
>> Added: compiler-rt/trunk/test/tsan/java_symbolization.cc<br>
>> URL:<br>
>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_compiler-2Drt_trunk_test_tsan_java-5Fsymbolization.cc-3Frev-3D240539-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=SyZP-pfg4JVpY2H_MgDoOXBrtKdp09HUOy-zkkYn4G4&s=bFNdZg6c4vvcz9PT78tvMuYj0sZr34ldohZ_cvHVOCA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/tsan/java_symbolization.cc?rev=240539&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- compiler-rt/trunk/test/tsan/java_symbolization.cc (added)<br>
>> +++ compiler-rt/trunk/test/tsan/java_symbolization.cc Wed Jun 24 08:04:12<br>
>> 2015<br>
>> @@ -0,0 +1,44 @@<br>
>> +// RUN: %clangxx_tsan -O1 %s -o %t && %deflake %run %t | FileCheck %s<br>
>> +#include "java.h"<br>
>> +#include <memory.h><br>
>> +<br>
>> +extern "C" bool __tsan_symbolize_external(jptr pc,<br>
>> + char *func_buf, jptr func_siz,<br>
>> + char *file_buf, jptr file_siz,<br>
>> + int *line, int *col) {<br>
>> + if (pc == (1234 | kExternalPCBit)) {<br>
>> + memcpy(func_buf, "MyFunc", sizeof("MyFunc"));<br>
>> + memcpy(file_buf, "MyFile.java", sizeof("MyFile.java"));<br>
>> + *line = 1234;<br>
>> + *col = 56;<br>
>> + return true;<br>
>> + }<br>
>> + return false;<br>
>> +}<br>
>> +<br>
>> +void *Thread(void *p) {<br>
>> + barrier_wait(&barrier);<br>
>> + __tsan_write1_pc((jptr)p, 1234 | kExternalPCBit);<br>
>> + return 0;<br>
>> +}<br>
>> +<br>
>> +int main() {<br>
>> + barrier_init(&barrier, 2);<br>
>> + int const kHeapSize = 1024 * 1024;<br>
>> + jptr jheap = (jptr)malloc(kHeapSize + 8) + 8;<br>
>> + __tsan_java_init(jheap, kHeapSize);<br>
>> + const int kBlockSize = 16;<br>
>> + __tsan_java_alloc(jheap, kBlockSize);<br>
>> + pthread_t th;<br>
>> + pthread_create(&th, 0, Thread, (void*)jheap);<br>
>> + __tsan_write1_pc((jptr)jheap, 1234 | kExternalPCBit);<br>
>> + barrier_wait(&barrier);<br>
>> + pthread_join(th, 0);<br>
>> + __tsan_java_free(jheap, kBlockSize);<br>
>> + fprintf(stderr, "DONE\n");<br>
>> + return __tsan_java_fini();<br>
>> +}<br>
>> +<br>
>> +// CHECK: WARNING: ThreadSanitizer: data race<br>
>> +// CHECK: #0 MyFunc MyFile.java:1234:56<br>
>> +// CHECK: DONE<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Alexey Samsonov<br><a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a></div></div>
</div></div>