<div dir="ltr">Hey Alexey,<div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>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.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 22, 2014 at 1:46 PM, Alexey Samsonov <span dir="ltr"><<a href="mailto:vonosmas@gmail.com" target="_blank">vonosmas@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: samsonov<br>
Date: Mon Dec 22 15:46:10 2014<br>
New Revision: 224736<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224736&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224736&view=rev</a><br>
Log:<br>
[Sanitizer] Make CommonFlags immutable after initialization.<br>
<br>
Summary:<br>
Protect CommonFlags singleton by adding const qualifier to<br>
common_flags() accessor. The only ways to modify the flags are<br>
SetCommonFlagsDefaults(), ParseCommonFlagsFromString() and<br>
OverrideCommonFlags() functions, which are only supposed to be<br>
called during initialization.<br>
<br>
Test Plan: regression test suite<br>
<br>
Reviewers: kcc, eugenis, glider<br>
<br>
Subscribers: llvm-commits<br>
<br>
Differential Revision: <a href="http://reviews.llvm.org/D6741" target="_blank">http://reviews.llvm.org/D6741</a><br>
<br>
Modified:<br>
compiler-rt/trunk/lib/asan/asan_flags.cc<br>
compiler-rt/trunk/lib/lsan/lsan_common.cc<br>
compiler-rt/trunk/lib/msan/msan.cc<br>
compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h<br>
compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc<br>
compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc<br>
compiler-rt/trunk/lib/ubsan/ubsan_flags.cc<br>
<br>
Modified: compiler-rt/trunk/lib/asan/asan_flags.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/asan/asan_flags.cc (original)<br>
+++ compiler-rt/trunk/lib/asan/asan_flags.cc Mon Dec 22 15:46:10 2014<br>
@@ -172,13 +172,15 @@ void ParseFlagsFromString(Flags *f, cons<br>
}<br>
<br>
void InitializeFlags(Flags *f) {<br>
- CommonFlags *cf = common_flags();<br>
SetCommonFlagsDefaults();<br>
- cf->detect_leaks = CAN_SANITIZE_LEAKS;<br>
- cf->external_symbolizer_path = GetEnv("ASAN_SYMBOLIZER_PATH");<br>
- cf->malloc_context_size = kDefaultMallocContextSize;<br>
- cf->intercept_tls_get_addr = true;<br>
- cf->coverage = false;<br>
+ {<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.detect_leaks = CAN_SANITIZE_LEAKS;<br>
+ cf.external_symbolizer_path = GetEnv("ASAN_SYMBOLIZER_PATH");<br>
+ cf.malloc_context_size = kDefaultMallocContextSize;<br>
+ cf.intercept_tls_get_addr = true;<br>
+ OverrideCommonFlags(cf);<br>
+ }<br>
<br>
internal_memset(f, 0, sizeof(*f));<br>
f->quarantine_size = (ASAN_LOW_MEMORY) ? 1UL << 26 : 1UL << 28;<br>
@@ -258,17 +260,17 @@ void InitializeFlags(Flags *f) {<br>
}<br>
<br>
// Flag validation:<br>
- if (!CAN_SANITIZE_LEAKS && cf->detect_leaks) {<br>
+ if (!CAN_SANITIZE_LEAKS && common_flags()->detect_leaks) {<br>
Report("%s: detect_leaks is not supported on this platform.\n",<br>
SanitizerToolName);<br>
- cf->detect_leaks = false;<br>
+ Die();<br>
}<br>
// Make "strict_init_order" imply "check_initialization_order".<br>
// TODO(samsonov): Use a single runtime flag for an init-order checker.<br>
if (f->strict_init_order) {<br>
f->check_initialization_order = true;<br>
}<br>
- CHECK_LE((uptr)cf->malloc_context_size, kStackTraceMax);<br>
+ CHECK_LE((uptr)common_flags()->malloc_context_size, kStackTraceMax);<br>
CHECK_LE(f->min_uar_stack_size_log, f->max_uar_stack_size_log);<br>
CHECK_GE(f->redzone, 16);<br>
CHECK_GE(f->max_redzone, f->redzone);<br>
<br>
Modified: compiler-rt/trunk/lib/lsan/lsan_common.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/lsan/lsan_common.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/lsan/lsan_common.cc (original)<br>
+++ compiler-rt/trunk/lib/lsan/lsan_common.cc Mon Dec 22 15:46:10 2014<br>
@@ -74,12 +74,13 @@ static void InitializeFlags(bool standal<br>
<br>
// Set defaults for common flags (only in standalone mode) and parse<br>
// them from LSAN_OPTIONS.<br>
- CommonFlags *cf = common_flags();<br>
if (standalone) {<br>
SetCommonFlagsDefaults();<br>
- cf->external_symbolizer_path = GetEnv("LSAN_SYMBOLIZER_PATH");<br>
- cf->malloc_context_size = 30;<br>
- cf->detect_leaks = true;<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.external_symbolizer_path = GetEnv("LSAN_SYMBOLIZER_PATH");<br>
+ cf.malloc_context_size = 30;<br>
+ cf.detect_leaks = true;<br>
+ OverrideCommonFlags(cf);<br>
}<br>
ParseCommonFlagsFromString(options);<br>
}<br>
<br>
Modified: compiler-rt/trunk/lib/msan/msan.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/msan/msan.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/msan/msan.cc (original)<br>
+++ compiler-rt/trunk/lib/msan/msan.cc Mon Dec 22 15:46:10 2014<br>
@@ -144,14 +144,17 @@ static void ParseFlagsFromString(Flags *<br>
}<br>
<br>
static void InitializeFlags(Flags *f, const char *options) {<br>
- CommonFlags *cf = common_flags();<br>
SetCommonFlagsDefaults();<br>
- cf->external_symbolizer_path = GetEnv("MSAN_SYMBOLIZER_PATH");<br>
- cf->malloc_context_size = 20;<br>
- cf->handle_ioctl = true;<br>
- // FIXME: test and enable.<br>
- cf->check_printf = false;<br>
- cf->intercept_tls_get_addr = true;<br>
+ {<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.external_symbolizer_path = GetEnv("MSAN_SYMBOLIZER_PATH");<br>
+ cf.malloc_context_size = 20;<br>
+ cf.handle_ioctl = true;<br>
+ // FIXME: test and enable.<br>
+ cf.check_printf = false;<br>
+ cf.intercept_tls_get_addr = true;<br>
+ OverrideCommonFlags(cf);<br>
+ }<br>
<br>
internal_memset(f, 0, sizeof(*f));<br>
f->poison_heap_with_zeroes = false;<br>
<br>
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h (original)<br>
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h Mon Dec 22 15:46:10 2014<br>
@@ -71,7 +71,7 @@ struct CommonFlags {<br>
<br>
// Functions to get/set global CommonFlags shared by all sanitizer runtimes:<br>
extern CommonFlags common_flags_dont_use;<br>
-inline CommonFlags *common_flags() {<br>
+inline const CommonFlags *common_flags() {<br>
return &common_flags_dont_use;<br>
}<br>
<br>
@@ -82,6 +82,16 @@ inline void SetCommonFlagsDefaults() {<br>
inline void ParseCommonFlagsFromString(const char *str) {<br>
common_flags_dont_use.ParseFromString(str);<br>
}<br>
+<br>
+// This function can only be used to setup tool-specific overrides for<br>
+// CommonFlags defaults. Generally, it should only be used right after<br>
+// SetCommonFlagsDefaults(), but before ParseCommonFlagsFromString(), and<br>
+// only during the flags initialization (i.e. before they are used for<br>
+// the first time).<br>
+inline void OverrideCommonFlags(const CommonFlags &cf) {<br>
+ common_flags_dont_use = cf;<br>
+}<br>
+<br>
void PrintFlagDescriptions();<br>
<br>
} // namespace __sanitizer<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/dd/dd_rtl.cc Mon Dec 22 15:46:10 2014<br>
@@ -70,10 +70,13 @@ void InitializeFlags(Flags *f, const cha<br>
// Default values.<br>
f->second_deadlock_stack = false;<br>
<br>
- CommonFlags *cf = common_flags();<br>
SetCommonFlagsDefaults();<br>
- // Override some common flags defaults.<br>
- cf->allow_addr2line = true;<br>
+ {<br>
+ // Override some common flags defaults.<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.allow_addr2line = true;<br>
+ OverrideCommonFlags(cf);<br>
+ }<br>
<br>
// Override from command line.<br>
ParseFlag(env, &f->second_deadlock_stack, "second_deadlock_stack", "");<br>
<br>
Modified: compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc (original)<br>
+++ compiler-rt/trunk/lib/tsan/rtl/tsan_flags.cc Mon Dec 22 15:46:10 2014<br>
@@ -93,13 +93,16 @@ void InitializeFlags(Flags *f, const cha<br>
// DDFlags<br>
f->second_deadlock_stack = false;<br>
<br>
- CommonFlags *cf = common_flags();<br>
SetCommonFlagsDefaults();<br>
- // Override some common flags defaults.<br>
- cf->allow_addr2line = true;<br>
- cf->detect_deadlocks = true;<br>
- cf->print_suppressions = false;<br>
- cf->stack_trace_format = " #%n %f %S %M";<br>
+ {<br>
+ // Override some common flags defaults.<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.allow_addr2line = true;<br>
+ cf.detect_deadlocks = true;<br>
+ cf.print_suppressions = false;<br>
+ cf.stack_trace_format = " #%n %f %S %M";<br>
+ OverrideCommonFlags(cf);<br>
+ }<br>
<br>
// Let a frontend override.<br>
ParseFlags(f, __tsan_default_options());<br>
@@ -115,7 +118,8 @@ void InitializeFlags(Flags *f, const cha<br>
f->report_signal_unsafe = false;<br>
}<br>
<br>
- if (cf->help) PrintFlagDescriptions();<br>
+ if (common_flags()->help)<br>
+ PrintFlagDescriptions();<br>
<br>
if (f->history_size < 0 || f->history_size > 7) {<br>
Printf("ThreadSanitizer: incorrect value for history_size"<br>
<br>
Modified: compiler-rt/trunk/lib/ubsan/ubsan_flags.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/ubsan/ubsan_flags.cc?rev=224736&r1=224735&r2=224736&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/ubsan/ubsan_flags.cc (original)<br>
+++ compiler-rt/trunk/lib/ubsan/ubsan_flags.cc Mon Dec 22 15:46:10 2014<br>
@@ -22,9 +22,10 @@ static const char *MaybeCallUbsanDefault<br>
}<br>
<br>
void InitializeCommonFlags() {<br>
- CommonFlags *cf = common_flags();<br>
SetCommonFlagsDefaults();<br>
- cf->print_summary = false;<br>
+ CommonFlags cf = *common_flags();<br>
+ cf.print_summary = false;<br>
+ OverrideCommonFlags(cf);<br>
// Override from user-specified string.<br>
ParseCommonFlagsFromString(MaybeCallUbsanDefaultOptions());<br>
// Override from environment variable.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>