[compiler-rt] r224736 - [Sanitizer] Make CommonFlags immutable after initialization.

Chandler Carruth chandlerc at google.com
Fri Jan 2 01:47:49 PST 2015


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150102/f93cb98e/attachment.html>


More information about the llvm-commits mailing list