[compiler-rt] r240539 - tsan: don't print external PCs in reports

[Dmitry Vyukov]

On Thu, Jun 25, 2015 at 9:00 PM, Alexey Samsonov <vonosmas at gmail.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?

The existing comment already says who sets this bit.
Enumeration of all users does not look useful to me. We don't do it
for any existing declaration in header files. kPageSize, who should
use it? I don't know. Whoever needs it.