Disable "UseOdrIndicator" ASan instrumentation mode by default.

Previously Swift enabled the "UseOdrIndicator" ASan instrumentation mode
and gave no option to disable this. This probably wasn't intentional but
happened due to the fact the
`createModuleAddressSanitizerLegacyPassPass()` function has a default
value for the `UseOdrIndicator` parameter of `true` and in Swift we
never specified this parameter explicitly.

Clang disables the "UseOdrIndicator" mode by default but allows it to be
enabled using the `-fsanitize-address-use-odr-indicator` flag.
Having "UseOdrIndicator" off by default is probably the right
default choice because it bloats the binary. So this patch changes the
Swift compiler to match Clang's behavior.

This patch disables the "UseOdrIndicator" mode by default but adds a
hidden driver and frontend flag (`-sanitize-address-use-odr-indicator`)
to enable it. The flag is hidden so that we can remove it in the future
if needed.

A side effect  of disabling "UseOdrIndicator" is that by we will no
longer use private aliases for poisoning globals. Private aliases were
introduced to avoid crashes
(google/sanitizers#398) due to ODR violations
with non-instrumented binaries. On Apple platforms the use of two-level
namespaces probably means that using private aliases wasn't ever really
necessary to avoid crashes. On platforms with a flat linking namespace
(e.g. Linux) using private aliases might matter more but should users
actually run into problems they can either:

* Fix their environment to remove the ODR, thus avoiding the crash.
* Instrument the previously non-instrumented code to avoid the crash.
* Use the new `-sanitize-address-use-odr-indicator` flag

rdar://problem/69335186

Author: danliew-apple

include/swift/AST/IRGenOptions.h

@@ -190,6 +190,9 @@ class IRGenOptions {
   /// Which sanitizer(s) have recovery instrumentation enabled.
   OptionSet<SanitizerKind> SanitizersWithRecoveryInstrumentation;
 
+  /// Whether to enable ODR indicators when building with ASan.
+  unsigned SanitizeAddressUseODRIndicator : 1;
+
   /// Path prefixes that should be rewritten in debug info.
   PathRemapper DebugPrefixMap;
 
@@ -345,6 +348,7 @@ class IRGenOptions {
         Verify(true), OptMode(OptimizationMode::NotSet),
         Sanitizers(OptionSet<SanitizerKind>()),
         SanitizersWithRecoveryInstrumentation(OptionSet<SanitizerKind>()),
+        SanitizeAddressUseODRIndicator(false),
         DebugInfoLevel(IRGenDebugInfoLevel::None),
         DebugInfoFormat(IRGenDebugInfoFormat::None),
         DisableClangModuleSkeletonCUs(false), UseJIT(false),

include/swift/Option/Options.td

@@ -1104,6 +1104,13 @@ def sanitize_recover_EQ
                "checks should be comma separated. Default behavior is to not "
                "allow error recovery.">;
 
+def sanitize_address_use_odr_indicator
+    : Flag<["-"], "sanitize-address-use-odr-indicator">,
+      Flags<[FrontendOption, NoInteractiveOption, HelpHidden]>,
+      HelpText<"When using AddressSanitizer enable ODR indicator globals to "
+               "avoid false ODR violation reports in partially sanitized "
+               "programs at the cost of an increase in binary size">;
+
 def sanitize_coverage_EQ : CommaJoined<["-"], "sanitize-coverage=">,
   Flags<[FrontendOption, NoInteractiveOption]>,
   MetaVarName<"<type>">,

include/swift/Option/SanitizerOptions.h

@@ -47,6 +47,11 @@ llvm::SanitizerCoverageOptions parseSanitizerCoverageArgValue(
         DiagnosticEngine &Diag,
         OptionSet<SanitizerKind> sanitizers);
 
+/// Parse -sanitize-address-use-odr-indicator's value.
+bool parseSanitizerAddressUseODRIndicator(
+    const llvm::opt::Arg *A, const OptionSet<SanitizerKind> &enabledSanitizers,
+    DiagnosticEngine &Diags);
+
 /// Returns the active sanitizers as a comma-separated list.
 std::string getSanitizerList(const OptionSet<SanitizerKind> &Set);
 }

lib/Driver/ToolChains.cpp

@@ -246,6 +246,8 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
                        options::OPT_no_warnings_as_errors);
   inputArgs.AddLastArg(arguments, options::OPT_sanitize_EQ);
   inputArgs.AddLastArg(arguments, options::OPT_sanitize_recover_EQ);
+  inputArgs.AddLastArg(arguments,
+                       options::OPT_sanitize_address_use_odr_indicator);
   inputArgs.AddLastArg(arguments, options::OPT_sanitize_coverage_EQ);
   inputArgs.AddLastArg(arguments, options::OPT_static);
   inputArgs.AddLastArg(arguments, options::OPT_swift_version);

lib/Frontend/CompilerInvocation.cpp

@@ -1228,6 +1228,12 @@ static bool ParseSILArgs(SILOptions &Opts, ArgList &Args,
                                        /*emitWarnings=*/true);
   }
 
+  if (const Arg *A =
+          Args.getLastArg(options::OPT_sanitize_address_use_odr_indicator)) {
+    IRGenOpts.SanitizeAddressUseODRIndicator =
+        parseSanitizerAddressUseODRIndicator(A, Opts.Sanitizers, Diags);
+  }
+
   if (auto A = Args.getLastArg(OPT_enable_verify_exclusivity,
                                OPT_disable_verify_exclusivity)) {
     Opts.VerifyExclusivity

lib/IRGen/IRGen.cpp

@@ -134,9 +134,11 @@ static void addAddressSanitizerPasses(const PassManagerBuilder &Builder,
   auto recover =
       bool(BuilderWrapper.IRGOpts.SanitizersWithRecoveryInstrumentation &
            SanitizerKind::Address);
+  auto useODRIndicator = BuilderWrapper.IRGOpts.SanitizeAddressUseODRIndicator;
   PM.add(createAddressSanitizerFunctionPass(/*CompileKernel=*/false, recover));
-  PM.add(createModuleAddressSanitizerLegacyPassPass(/*CompileKernel=*/false,
-                                                    recover));
+  PM.add(createModuleAddressSanitizerLegacyPassPass(
+      /*CompileKernel=*/false, recover, /*UseGlobalsGC=*/true,
+      useODRIndicator));
 }
 
 static void addThreadSanitizerPass(const PassManagerBuilder &Builder,

lib/Option/SanitizerOptions.cpp

@@ -258,6 +258,22 @@ OptionSet<SanitizerKind> swift::parseSanitizerRecoverArgValues(
   return sanitizerRecoverSet;
 }
 
+// Note this implementation cannot be inlined at its use site because it calls
+// `toStringRef(const SanitizerKind).`
+bool swift::parseSanitizerAddressUseODRIndicator(
+    const llvm::opt::Arg *A, const OptionSet<SanitizerKind> &enabledSanitizers,
+    DiagnosticEngine &Diags) {
+  // Warn if ASan isn't enabled.
+  if (!(enabledSanitizers & SanitizerKind::Address)) {
+    Diags.diagnose(
+        SourceLoc(), diag::warning_option_requires_specific_sanitizer,
+        A->getOption().getPrefixedName(), toStringRef(SanitizerKind::Address));
+    return false;
+  }
+
+  return true;
+}
+
 std::string swift::getSanitizerList(const OptionSet<SanitizerKind> &Set) {
   std::string list;
   #define SANITIZER(_, kind, name, file) \

test/Driver/sanitize_address_use_odr_indicator.swift

@@ -0,0 +1,11 @@
+// REQUIRES: asan_runtime
+// RUN: %swiftc_driver -driver-print-jobs -sanitize=address -sanitize-address-use-odr-indicator %s 2>&1 | %FileCheck %s
+
+// CHECK: swift
+// CHECK-DAG: -sanitize=address
+// CHECK-DAG: -sanitize-address-use-odr-indicator
+
+// Missing `-sanitize=address` causes warning to be emitted
+// RUN: %swiftc_driver  -sanitize-address-use-odr-indicator \
+// RUN:  %s -o %t 2>&1 | %FileCheck -check-prefix=CHECK-MISSING-ARG %s
+// CHECK-MISSING-ARG: warning: option '-sanitize-address-use-odr-indicator' has no effect when 'address' sanitizer is disabled. Use -sanitize=address to enable the sanitizer

test/IRGen/address_sanitizer_use_odr_indicator.swift

@@ -0,0 +1,18 @@
+// REQUIRES: asan_runtime
+
+// Default instrumentation that does not use ODR indicators
+// and private aliases.
+// RUN: %target-swift-frontend -emit-ir -sanitize=address %s | %FileCheck %s
+// CHECK: @"[[MANGLED_GLOBAL:.+aGlobal.+]]" =
+// CHECK-NOT: __odr_asan_gen_[[MANGLED_GLOBAL]]
+// CHECK-NOT: @{{.+}} = private alias {{.*}}@"[[MANGLED_GLOBAL]]"
+
+// Instrumentation with ODR indicators (implies private aliases)
+// RUN: %target-swift-frontend -emit-ir -sanitize=address \
+// RUN:   -sanitize-address-use-odr-indicator %s | \
+// RUN:   %FileCheck -check-prefix=CHECK-ODR %s
+// CHECK-ODR: @"[[MANGLED_GLOBAL:.+aGlobal.+]]" =
+// CHECK-ODR: __odr_asan_gen_[[MANGLED_GLOBAL]]
+// CHECK-ODR: @{{.+}} = private alias {{.*}}@"[[MANGLED_GLOBAL]]"
+
+let aGlobal:Int = 128;

test/Sanitizers/asan.swift

@@ -1,6 +1,14 @@
 // RUN: %target-swiftc_driver %s -target %sanitizers-target-triple -g -sanitize=address -o %t_asan-binary
 // RUN: %target-codesign %t_asan-binary
 // RUN: env %env-ASAN_OPTIONS=abort_on_error=0 not %target-run %t_asan-binary 2>&1 | %FileCheck %s
+
+// ODR Indicator variant
+// RUN: %target-swiftc_driver %s -target %sanitizers-target-triple -g -sanitize=address \
+// RUN:   -sanitize-address-use-odr-indicator -o %t_asan-binary-odr-indicator
+// RUN: %target-codesign %t_asan-binary-odr-indicator
+// RUN: env %env-ASAN_OPTIONS=abort_on_error=0 not %target-run \
+// RUN:   %t_asan-binary-odr-indicator 2>&1 | %FileCheck %s
+
 // REQUIRES: executable_test
 // REQUIRES: asan_runtime