[compiler-rt] r224736 - [Sanitizer] Make CommonFlags immutable after initialization.
Alexey Samsonov
vonosmas at gmail.com
Fri Jan 2 13:30:57 PST 2015
No problem, I was aware of the problem and planned to work on a fix today
anyway. Sorry, I didn't realize that it would actually cause failures :(
Re-commited the change in r225088.
On Fri, Jan 2, 2015 at 1:47 AM, Chandler Carruth <chandlerc at google.com>
wrote:
> Hey Alexey,
>
> It looks like some of our internal users of TSan aren't yet ready for this
> change. Since this change, they've been failing consistently, and before
> this change they work fine.
>
> I'll follow up with you internally with the exact details of how to
> reproduce, and I'm happy to help sit down and get things working, but
> wanted to send you a heads up.
>
> I'm planning to revert this temporarily until we can sort everything out
> as nothing really depends on this and it's causing lots of trouble for our
> users.
>
> On Mon, Dec 22, 2014 at 1:46 PM, Alexey Samsonov <vonosmas at gmail.com>
> wrote:
>
>> Author: samsonov
>> Date: Mon Dec 22 15:46:10 2014
>> New Revision: 224736
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224736&view=rev
>> Log:
>> [Sanitizer] Make CommonFlags immutable after initialization.
>>
>> Summary:
>> Protect CommonFlags singleton by adding const qualifier to
>> common_flags() accessor. The only ways to modify the flags are
>> SetCommonFlagsDefaults(), ParseCommonFlagsFromString() and
>> OverrideCommonFlags() functions, which are only supposed to be
>> called during initialization.
>>
>> Test Plan: regression test suite
>>
>> Reviewers: kcc, eugenis, glider
>>
>> Subscribers: llvm-commits
>>
>> Differential Revision: http://reviews.llvm.org/D6741
>>
>> Modified:
>> compiler-rt/trunk/lib/asan/asan_flags.cc
>> compiler-rt/trunk/lib/lsan/lsan_common.cc
>> compiler-rt/trunk/lib/msan/msan.cc
>> compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h
>> compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc
>> compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc
>> compiler-rt/trunk/lib/ubsan/ubsan_flags.cc
>>
>> Modified: compiler-rt/trunk/lib/asan/asan_flags.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/asan/asan_flags.cc (original)
>> +++ compiler-rt/trunk/lib/asan/asan_flags.cc Mon Dec 22 15:46:10 2014
>> @@ -172,13 +172,15 @@ void ParseFlagsFromString(Flags *f, cons
>> }
>>
>> void InitializeFlags(Flags *f) {
>> - CommonFlags *cf = common_flags();
>> SetCommonFlagsDefaults();
>> - cf->detect_leaks = CAN_SANITIZE_LEAKS;
>> - cf->external_symbolizer_path = GetEnv("ASAN_SYMBOLIZER_PATH");
>> - cf->malloc_context_size = kDefaultMallocContextSize;
>> - cf->intercept_tls_get_addr = true;
>> - cf->coverage = false;
>> + {
>> + CommonFlags cf = *common_flags();
>> + cf.detect_leaks = CAN_SANITIZE_LEAKS;
>> + cf.external_symbolizer_path = GetEnv("ASAN_SYMBOLIZER_PATH");
>> + cf.malloc_context_size = kDefaultMallocContextSize;
>> + cf.intercept_tls_get_addr = true;
>> + OverrideCommonFlags(cf);
>> + }
>>
>> internal_memset(f, 0, sizeof(*f));
>> f->quarantine_size = (ASAN_LOW_MEMORY) ? 1UL << 26 : 1UL << 28;
>> @@ -258,17 +260,17 @@ void InitializeFlags(Flags *f) {
>> }
>>
>> // Flag validation:
>> - if (!CAN_SANITIZE_LEAKS && cf->detect_leaks) {
>> + if (!CAN_SANITIZE_LEAKS && common_flags()->detect_leaks) {
>> Report("%s: detect_leaks is not supported on this platform.\n",
>> SanitizerToolName);
>> - cf->detect_leaks = false;
>> + Die();
>> }
>> // Make "strict_init_order" imply "check_initialization_order".
>> // TODO(samsonov): Use a single runtime flag for an init-order checker.
>> if (f->strict_init_order) {
>> f->check_initialization_order = true;
>> }
>> - CHECK_LE((uptr)cf->malloc_context_size, kStackTraceMax);
>> + CHECK_LE((uptr)common_flags()->malloc_context_size, kStackTraceMax);
>> CHECK_LE(f->min_uar_stack_size_log, f->max_uar_stack_size_log);
>> CHECK_GE(f->redzone, 16);
>> CHECK_GE(f->max_redzone, f->redzone);
>>
>> Modified: compiler-rt/trunk/lib/lsan/lsan_common.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/lsan/lsan_common.cc (original)
>> +++ compiler-rt/trunk/lib/lsan/lsan_common.cc Mon Dec 22 15:46:10 2014
>> @@ -74,12 +74,13 @@ static void InitializeFlags(bool standal
>>
>> // Set defaults for common flags (only in standalone mode) and parse
>> // them from LSAN_OPTIONS.
>> - CommonFlags *cf = common_flags();
>> if (standalone) {
>> SetCommonFlagsDefaults();
>> - cf->external_symbolizer_path = GetEnv("LSAN_SYMBOLIZER_PATH");
>> - cf->malloc_context_size = 30;
>> - cf->detect_leaks = true;
>> + CommonFlags cf = *common_flags();
>> + cf.external_symbolizer_path = GetEnv("LSAN_SYMBOLIZER_PATH");
>> + cf.malloc_context_size = 30;
>> + cf.detect_leaks = true;
>> + OverrideCommonFlags(cf);
>> }
>> ParseCommonFlagsFromString(options);
>> }
>>
>> Modified: compiler-rt/trunk/lib/msan/msan.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/msan/msan.cc (original)
>> +++ compiler-rt/trunk/lib/msan/msan.cc Mon Dec 22 15:46:10 2014
>> @@ -144,14 +144,17 @@ static void ParseFlagsFromString(Flags *
>> }
>>
>> static void InitializeFlags(Flags *f, const char *options) {
>> - CommonFlags *cf = common_flags();
>> SetCommonFlagsDefaults();
>> - cf->external_symbolizer_path = GetEnv("MSAN_SYMBOLIZER_PATH");
>> - cf->malloc_context_size = 20;
>> - cf->handle_ioctl = true;
>> - // FIXME: test and enable.
>> - cf->check_printf = false;
>> - cf->intercept_tls_get_addr = true;
>> + {
>> + CommonFlags cf = *common_flags();
>> + cf.external_symbolizer_path = GetEnv("MSAN_SYMBOLIZER_PATH");
>> + cf.malloc_context_size = 20;
>> + cf.handle_ioctl = true;
>> + // FIXME: test and enable.
>> + cf.check_printf = false;
>> + cf.intercept_tls_get_addr = true;
>> + OverrideCommonFlags(cf);
>> + }
>>
>> internal_memset(f, 0, sizeof(*f));
>> f->poison_heap_with_zeroes = false;
>>
>> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h (original)
>> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h Mon Dec 22
>> 15:46:10 2014
>> @@ -71,7 +71,7 @@ struct CommonFlags {
>>
>> // Functions to get/set global CommonFlags shared by all sanitizer
>> runtimes:
>> extern CommonFlags common_flags_dont_use;
>> -inline CommonFlags *common_flags() {
>> +inline const CommonFlags *common_flags() {
>> return &common_flags_dont_use;
>> }
>>
>> @@ -82,6 +82,16 @@ inline void SetCommonFlagsDefaults() {
>> inline void ParseCommonFlagsFromString(const char *str) {
>> common_flags_dont_use.ParseFromString(str);
>> }
>> +
>> +// This function can only be used to setup tool-specific overrides for
>> +// CommonFlags defaults. Generally, it should only be used right after
>> +// SetCommonFlagsDefaults(), but before ParseCommonFlagsFromString(), and
>> +// only during the flags initialization (i.e. before they are used for
>> +// the first time).
>> +inline void OverrideCommonFlags(const CommonFlags &cf) {
>> + common_flags_dont_use = cf;
>> +}
>> +
>> void PrintFlagDescriptions();
>>
>> } // namespace __sanitizer
>>
>> Modified: compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc (original)
>> +++ compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc Mon Dec 22 15:46:10 2014
>> @@ -70,10 +70,13 @@ void InitializeFlags(Flags *f, const cha
>> // Default values.
>> f->second_deadlock_stack = false;
>>
>> - CommonFlags *cf = common_flags();
>> SetCommonFlagsDefaults();
>> - // Override some common flags defaults.
>> - cf->allow_addr2line = true;
>> + {
>> + // Override some common flags defaults.
>> + CommonFlags cf = *common_flags();
>> + cf.allow_addr2line = true;
>> + OverrideCommonFlags(cf);
>> + }
>>
>> // Override from command line.
>> ParseFlag(env, &f->second_deadlock_stack, "second_deadlock_stack", "");
>>
>> Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc (original)
>> +++ compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc Mon Dec 22 15:46:10 2014
>> @@ -93,13 +93,16 @@ void InitializeFlags(Flags *f, const cha
>> // DDFlags
>> f->second_deadlock_stack = false;
>>
>> - CommonFlags *cf = common_flags();
>> SetCommonFlagsDefaults();
>> - // Override some common flags defaults.
>> - cf->allow_addr2line = true;
>> - cf->detect_deadlocks = true;
>> - cf->print_suppressions = false;
>> - cf->stack_trace_format = " #%n %f %S %M";
>> + {
>> + // Override some common flags defaults.
>> + CommonFlags cf = *common_flags();
>> + cf.allow_addr2line = true;
>> + cf.detect_deadlocks = true;
>> + cf.print_suppressions = false;
>> + cf.stack_trace_format = " #%n %f %S %M";
>> + OverrideCommonFlags(cf);
>> + }
>>
>> // Let a frontend override.
>> ParseFlags(f, __tsan_default_options());
>> @@ -115,7 +118,8 @@ void InitializeFlags(Flags *f, const cha
>> f->report_signal_unsafe = false;
>> }
>>
>> - if (cf->help) PrintFlagDescriptions();
>> + if (common_flags()->help)
>> + PrintFlagDescriptions();
>>
>> if (f->history_size < 0 || f->history_size > 7) {
>> Printf("ThreadSanitizer: incorrect value for history_size"
>>
>> Modified: compiler-rt/trunk/lib/ubsan/ubsan_flags.cc
>> URL:
>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff
>>
>> ==============================================================================
>> --- compiler-rt/trunk/lib/ubsan/ubsan_flags.cc (original)
>> +++ compiler-rt/trunk/lib/ubsan/ubsan_flags.cc Mon Dec 22 15:46:10 2014
>> @@ -22,9 +22,10 @@ static const char *MaybeCallUbsanDefault
>> }
>>
>> void InitializeCommonFlags() {
>> - CommonFlags *cf = common_flags();
>> SetCommonFlagsDefaults();
>> - cf->print_summary = false;
>> + CommonFlags cf = *common_flags();
>> + cf.print_summary = false;
>> + OverrideCommonFlags(cf);
>> // Override from user-specified string.
>> ParseCommonFlagsFromString(MaybeCallUbsanDefaultOptions());
>> // Override from environment variable.
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
--
Alexey Samsonov
vonosmas at gmail.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150102/bd2f1d52/attachment.html>
More information about the llvm-commits
mailing list