Tread TSan LLVM flags to driver: add TSan controlling flags to clang.

[alekseyshl]

Summary

New clang flags, all default to true:
-f[no-]sanitize-thread-data-races
-f[no-]sanitize-thread-stack-traces
-f[no-]sanitize-thread-atomics

[eugenis]

lib/Frontend/CompilerInvocation.cpp

735

It seems common to hardcode the default option value here.
The same in SanitizerArgs.cpp.

[eugenis]

Oh, and this needs a test. See test/Driver/fsanitize.c (search for -fsanitize-address-use-after-scope as an example).

[alekseyshl]

lib/Frontend/CompilerInvocation.cpp

735

True, but the default value for these flags is already mentioned in more than one place, why not to try to reduce the complexity?

[eugenis]

lib/Frontend/CompilerInvocation.cpp

735

I guess I don't mind either way.

[alekseyshl]

• Added ust test for the new command line args.

[eugenis]

test/Driver/fsanitize.c

288

did you mean CHECK-TSAN-DATA-RACES-BOTH-NOT?

[dvyukov]

include/clang/Driver/Options.td

733

These descriptions may be confusing for users.
This does not disable data race detection in tsan. Even if all files are compiled with this flag, tsan can still report races. Same for traces and atomics.
Please make it clear that it only enables/disables source _instrumentation_.

Does user see what's the default value? If not, then I guess some users will add flags just because they do want data races detection and stack traces. We need to make it clear that all flags are enabled by default.

[alekseyshl]

• Rename new options to better reflect their functionality.

[alekseyshl]

test/Driver/fsanitize.c

288

I missed -BOTH on the FileCheck line above and in couple of other places.

[alekseyshl]

• Translate new driver flags to legacy -llvm ones.

[alekseyshl]

PTAL

[pcc]

Seems like a reasonable enough first step. Please add a FIXME to pass these flags as attributes.

[eugenis]

lib/Frontend/CompilerInvocation.cpp

732

It looks like lib/Frontend changes are no longer necessary.

[alekseyshl]

• Remove unnecessary frontend options.

[alekseyshl]

Frontend options removed.

[eugenis]

LGTM

[pcc]

include/clang/Driver/Options.td

732

You can remove the Flags<[CC1Option]> part in each of these now.

[alekseyshl]

• Add FIXME to address the rest of the suggestions later.

[alekseyshl]

• Remove unnecessary flag qualifications.

[alekseyshl]

.

include/clang/Driver/Options.td

def fno_sanitize_cfi_cross_dso : Flag<["-"], "fno-sanitize-cfi-cross-dso">,
                                  Group<f_clang_Group>, Flags<[CC1Option]>,
                                  HelpText<"Disable control flow integrity (CFI) checks for cross-DSO calls.">;
 def fsanitize_stats : Flag<["-"], "fsanitize-stats">,
                               Group<f_clang_Group>, Flags<[CC1Option]>,
                               HelpText<"Enable sanitizer statistics gathering.">;
 def fno_sanitize_stats : Flag<["-"], "fno-sanitize-stats">,
                                  Group<f_clang_Group>, Flags<[CC1Option]>,
                                  HelpText<"Disable sanitizer statistics gathering.">;
+def fsanitize_thread_memory_access : Flag<["-"], "fsanitize-thread-memory-access">,
+                                     Group<f_clang_Group>, Flags<[CC1Option]>,
+                                     HelpText<"Enable memory access instrumentation in ThreadSanitizer (default)">;
+def fno_sanitize_thread_memory_access : Flag<["-"], "fno-sanitize-thread-memory-access">,
+                                        Group<f_clang_Group>, Flags<[CC1Option]>,
+                                        HelpText<"Disable memory access instrumentation in ThreadSanitizer">;
+def fsanitize_thread_func_entry_exit : Flag<["-"], "fsanitize-thread-func-entry-exit">,
+                                       Group<f_clang_Group>, Flags<[CC1Option]>,
+                                       HelpText<"Enable function entry/exit instrumentation in ThreadSanitizer (default)">;
+def fno_sanitize_thread_func_entry_exit : Flag<["-"], "fno-sanitize-thread-func-entry-exit">,
+                                          Group<f_clang_Group>, Flags<[CC1Option]>,
+                                          HelpText<"Disable function entry/exit instrumentation in ThreadSanitizer">;
+def fsanitize_thread_atomics : Flag<["-"], "fsanitize-thread-atomics">,
+                               Group<f_clang_Group>, Flags<[CC1Option]>,
+                               HelpText<"Enable atomic operations instrumentation in ThreadSanitizer (default)">;
+def fno_sanitize_thread_atomics : Flag<["-"], "fno-sanitize-thread-atomics">,
+                                  Group<f_clang_Group>, Flags<[CC1Option]>,
+                                  HelpText<"Disable atomic operations instrumentation in ThreadSanitizer">;
 def fsanitize_undefined_strip_path_components_EQ : Joined<["-"], "fsanitize-undefined-strip-path-components=">,
   Group<f_clang_Group>, Flags<[CC1Option]>, MetaVarName<"<number>">,
   HelpText<"Strip (or keep only, if negative) a given number of path components "
            "when emitting check metadata.">;
 def funsafe_math_optimizations : Flag<["-"], "funsafe-math-optimizations">,
   Group<f_Group>;
 def fno_unsafe_math_optimizations : Flag<["-"], "fno-unsafe-math-optimizations">,
   Group<f_Group>;

include/clang/Driver/SanitizerArgs.h

class SanitizerArgs {
   bool MsanUseAfterDtor = false;
   bool CfiCrossDso = false;
   int AsanFieldPadding = 0;
   bool AsanSharedRuntime = false;
   bool AsanUseAfterScope = false;
   bool LinkCXXRuntimes = false;
   bool NeedPIE = false;
   bool Stats = false;
+  bool TsanMemoryAccess = true;
+  bool TsanFuncEntryExit = true;
+  bool TsanAtomics = true;
 
  public:
   /// Parses the sanitizer arguments from an argument list.
   SanitizerArgs(const ToolChain &TC, const llvm::opt::ArgList &Args);
 
   bool needsAsanRt() const { return Sanitizers.has(SanitizerKind::Address); }
   bool needsSharedAsanRt() const { return AsanSharedRuntime; }
   bool needsTsanRt() const { return Sanitizers.has(SanitizerKind::Thread); }

include/clang/Frontend/CodeGenOptions.def

 CODEGENOPT(SanitizeCoverageTraceGep, 1, 0) ///< Enable GEP instruction tracing
                                            ///< in sanitizer coverage.
 CODEGENOPT(SanitizeCoverage8bitCounters, 1, 0) ///< Use 8-bit frequency counters
                                                ///< in sanitizer coverage.
 CODEGENOPT(SanitizeCoverageTracePC, 1, 0) ///< Enable PC tracing
                                           ///< in sanitizer coverage.
 CODEGENOPT(SanitizeCoverageTracePCGuard, 1, 0) ///< Enable PC tracing with guard
                                                ///< in sanitizer coverage.
+CODEGENOPT(SanitizeThreadMemoryAccess, 1, 1) ///< Enable memory access
+                                             ///< instrumentation in
+                                             ///< ThreadSanitizer
+CODEGENOPT(SanitizeThreadFuncEntryExit, 1, 1) ///< Enable function entry/exit
+                                              ///< instrumentation in
+                                              ///< ThreadSanitizer
+CODEGENOPT(SanitizeThreadAtomics, 1, 1) ///< Enable atomic operations
+                                        ///< instrumentation in ThreadSanitizer
 CODEGENOPT(SanitizeStats     , 1, 0) ///< Collect statistics for sanitizers.
 CODEGENOPT(SimplifyLibCalls  , 1, 1) ///< Set when -fbuiltin is enabled.
 CODEGENOPT(SoftFloat         , 1, 0) ///< -soft-float.
 CODEGENOPT(StrictEnums       , 1, 0) ///< Optimize based on strict enum definition.
 CODEGENOPT(StrictVTablePointers, 1, 0) ///< Optimize based on the strict vtable pointers
 CODEGENOPT(TimePasses        , 1, 0) ///< Set when -ftime-report is enabled.
 CODEGENOPT(UnrollLoops       , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops       , 1, 0) ///< Control whether loops are rerolled.

lib/CodeGen/BackendUtil.cpp

if (Builder.OptLevel > 0) {
     PM.add(createGVNPass());
     PM.add(createInstructionCombiningPass());
     PM.add(createDeadStoreEliminationPass());
   }
 }
 
 static void addThreadSanitizerPass(const PassManagerBuilder &Builder,
                                    legacy::PassManagerBase &PM) {
-  PM.add(createThreadSanitizerPass());
+  const PassManagerBuilderWrapper &BuilderWrapper =
+      static_cast<const PassManagerBuilderWrapper&>(Builder);
+  const CodeGenOptions &CGOpts = BuilderWrapper.getCGOpts();
+  PM.add(createThreadSanitizerPass(CGOpts.SanitizeThreadMemoryAccess,
+                                   CGOpts.SanitizeThreadFuncEntryExit,
+                                   CGOpts.SanitizeThreadAtomics));
 }
 
 static void addDataFlowSanitizerPass(const PassManagerBuilder &Builder,
                                      legacy::PassManagerBase &PM) {
   const PassManagerBuilderWrapper &BuilderWrapper =
       static_cast<const PassManagerBuilderWrapper&>(Builder);
   const LangOptions &LangOpts = BuilderWrapper.getLangOpts();
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));

lib/Driver/SanitizerArgs.cpp

if (Arg *A =
       }
     }
     MsanUseAfterDtor =
         Args.hasArg(options::OPT_fsanitize_memory_use_after_dtor);
     NeedPIE |= !(TC.getTriple().isOSLinux() &&
                  TC.getTriple().getArch() == llvm::Triple::x86_64);
   }
 
+  if (AllAddedKinds & Thread) {
+    TsanMemoryAccess = Args.hasFlag(options::OPT_fsanitize_thread_memory_access,
+                                    options::OPT_fno_sanitize_thread_memory_access,
+                                    TsanMemoryAccess);
+    TsanFuncEntryExit = Args.hasFlag(options::OPT_fsanitize_thread_func_entry_exit,
+                                     options::OPT_fno_sanitize_thread_func_entry_exit,
+                                     TsanFuncEntryExit);
+    TsanAtomics = Args.hasFlag(options::OPT_fsanitize_thread_atomics,
+                               options::OPT_fno_sanitize_thread_atomics,
+                               TsanAtomics);
+  }
+
   if (AllAddedKinds & CFI) {
     CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso,
                                options::OPT_fno_sanitize_cfi_cross_dso, false);
     // Without PIE, external function address may resolve to a PLT record, which
     // can not be verified by the target module.
     NeedPIE |= CfiCrossDso;
   }
 
void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
 
   if (MsanTrackOrigins)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" +
                                          llvm::utostr(MsanTrackOrigins)));
 
   if (MsanUseAfterDtor)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-use-after-dtor"));
 
+  if (!TsanMemoryAccess)
+    CmdArgs.push_back(Args.MakeArgString("-fno-sanitize-thread-memory-access"));
+  if (!TsanFuncEntryExit)
+    CmdArgs.push_back(Args.MakeArgString("-fno-sanitize-thread-func-entry-exit"));
+  if (!TsanAtomics)
+    CmdArgs.push_back(Args.MakeArgString("-fno-sanitize-thread-atomics"));
+
   if (CfiCrossDso)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-cfi-cross-dso"));
 
   if (Stats)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-stats"));
 
   if (AsanFieldPadding)
     CmdArgs.push_back(Args.MakeArgString("-fsanitize-address-field-padding=" +

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.SanitizeCoverageTraceCmp = Args.hasArg(OPT_fsanitize_coverage_trace_cmp);
   Opts.SanitizeCoverageTraceDiv = Args.hasArg(OPT_fsanitize_coverage_trace_div);
   Opts.SanitizeCoverageTraceGep = Args.hasArg(OPT_fsanitize_coverage_trace_gep);
   Opts.SanitizeCoverage8bitCounters =
       Args.hasArg(OPT_fsanitize_coverage_8bit_counters);
   Opts.SanitizeCoverageTracePC = Args.hasArg(OPT_fsanitize_coverage_trace_pc);
   Opts.SanitizeCoverageTracePCGuard =
       Args.hasArg(OPT_fsanitize_coverage_trace_pc_guard);
+  Opts.SanitizeThreadMemoryAccess =
+      Args.hasFlag(OPT_fsanitize_thread_memory_access,
+                   OPT_fno_sanitize_thread_memory_access,
+                   Opts.SanitizeThreadMemoryAccess);
+  Opts.SanitizeThreadFuncEntryExit =
+      Args.hasFlag(OPT_fsanitize_thread_func_entry_exit,
+                   OPT_fno_sanitize_thread_func_entry_exit,
+                   Opts.SanitizeThreadFuncEntryExit);
+  Opts.SanitizeThreadAtomics =
+      Args.hasFlag(OPT_fsanitize_thread_atomics,
+                   OPT_fno_sanitize_thread_atomics,
+                   Opts.SanitizeThreadAtomics);
   Opts.SanitizeMemoryTrackOrigins =
       getLastArgIntValue(Args, OPT_fsanitize_memory_track_origins_EQ, 0, Diags);
   Opts.SanitizeMemoryUseAfterDtor =
       Args.hasArg(OPT_fsanitize_memory_use_after_dtor);
   Opts.SanitizeCfiCrossDso = Args.hasArg(OPT_fsanitize_cfi_cross_dso);
   Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
   if (Arg *A = Args.getLastArg(OPT_fsanitize_address_use_after_scope,
                                OPT_fno_sanitize_address_use_after_scope)) {

test/Driver/fsanitize.c

 // CHECK-TSAN-ARM-IOS: unsupported option '-fsanitize=thread' for target 'arm-apple-ios'
 
 // RUN: %clang -target i386-apple-iossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-I386-IOSSIMULATOR
 // CHECK-TSAN-I386-IOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-iossimulator'
 
 // RUN: %clang -target i386-apple-tvossimulator -fsanitize=thread %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-I386-TVOSSIMULATOR
 // CHECK-TSAN-I386-TVOSSIMULATOR: unsupported option '-fsanitize=thread' for target 'i386-apple-tvossimulator'
 
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-memory-access %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-MEMORY-ACCESS
+// CHECK-TSAN-MEMORY-ACCESS-NOT: -cc1{{.*}}thread-data-races
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-memory-access %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-MEMORY-ACCESS-OFF
+// CHECK-TSAN-MEMORY-ACCESS-OFF: -cc1{{.*}}-fno-sanitize-thread-memory-access
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-memory-access -fsanitize-thread-memory-access %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-MEMORY-ACCESS-BOTH
+// CHECK-TSAN-MEMORY-ACCESS-BOTH-NOT: -cc1{{.*}}thread-data-races
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-memory-access -fno-sanitize-thread-memory-access %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-MEMORY-ACCESS-BOTH-OFF
+// CHECK-TSAN-MEMORY-ACCESS-BOTH-OFF: -cc1{{.*}}-fno-sanitize-thread-memory-access
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-func-entry-exit %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-FUNC-ENTRY-EXIT
+// CHECK-TSAN-FUNC-ENTRY-EXIT-NOT: -cc1{{.*}}thread-stack-traces
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-func-entry-exit %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-FUNC-ENTRY-EXIT-OFF
+// CHECK-TSAN-FUNC-ENTRY-EXIT-OFF: -cc1{{.*}}-fno-sanitize-thread-func-entry-exit
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-func-entry-exit -fsanitize-thread-func-entry-exit %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-FUNC-ENTRY-EXIT-BOTH
+// CHECK-TSAN-FUNC-ENTRY-EXIT-BOTH-NOT: -cc1{{.*}}thread-stack-traces
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-func-entry-exit -fno-sanitize-thread-func-entry-exit %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-FUNC-ENTRY-EXIT-BOTH-OFF
+// CHECK-TSAN-FUNC-ENTRY-EXIT-BOTH-OFF: -cc1{{.*}}-fno-sanitize-thread-func-entry-exit
+
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS
+// CHECK-TSAN-ATOMICS-NOT: -cc1{{.*}}thread-atomics
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS-OFF
+// CHECK-TSAN-ATOMICS-OFF: -cc1{{.*}}-fno-sanitize-thread-atomics
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fno-sanitize-thread-atomics -fsanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS-BOTH
+// CHECK-TSAN-ATOMICS-BOTH-NOT: -cc1{{.*}}thread-atomics
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=thread -fsanitize-thread-atomics -fno-sanitize-thread-atomics %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-TSAN-ATOMICS-BOTH-OFF
+// CHECK-TSAN-ATOMICS-BOTH-OFF: -cc1{{.*}}-fno-sanitize-thread-atomics
+
 // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=function %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-DARWIN
 // CHECK-FSAN-DARWIN: unsupported option '-fsanitize=function' for target 'x86_64-apple-darwin10'
 
 // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=function -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSAN-UBSAN-DARWIN
 // CHECK-FSAN-UBSAN-DARWIN: unsupported option '-fsanitize=function' for target 'x86_64-apple-darwin10'
 
 // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.8 -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-OLD
 // CHECK-VPTR-DARWIN-OLD: unsupported option '-fsanitize=vptr' for target 'x86_64-apple-darwin10'