Merge pull request swiftlang#35074 from apple/dliew/rdar-69335186

Disable "UseOdrIndicator" ASan instrumentation mode by default.

Author: danliew-apple

include/swift/AST/IRGenOptions.h

@@ -193,6 +193,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;
 
@@ -348,6 +351,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

@@ -1238,6 +1238,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