r304463 - [Modules] Handle sanitizer feature mismatches when importing modules

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 1 13:01:02 PDT 2017


Author: vedantk
Date: Thu Jun  1 15:01:01 2017
New Revision: 304463

URL: http://llvm.org/viewvc/llvm-project?rev=304463&view=rev
Log:
[Modules] Handle sanitizer feature mismatches when importing modules

This patch makes it an error to have a mismatch between the enabled
sanitizers in a CU, and in any module being imported into the CU. Only
mismatches between non-modular sanitizers are treated as errors.

This patch also includes non-modular sanitizers in module hashes, in
order to ensure module rebuilds occur when -fsanitize=X is toggled on
and off for non-modular sanitizers, and to cut down on module rebuilds
when the option is toggled for modular sanitizers.

This fixes a longstanding issue with implicit modules and sanitizers,
which Duncan originally diagnosed.

When building with implicit modules it's possible to hit a scenario
where modules are built without -fsanitize=address, and are subsequently
imported into CUs with -fsanitize=address enabled. This causes strange
failures at runtime. The case Duncan found affects libcxx, since its
vector implementation behaves differently when ASan is enabled.

Implicit module builds should "just work" when -fsanitize=X is toggled
on and off across multiple compiler invocations, which is what this
patch does.

Differential Revision: https://reviews.llvm.org/D32724

Added:
    cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/
    cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/check.h
    cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/map
    cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
Modified:
    cfe/trunk/include/clang/Basic/Sanitizers.h
    cfe/trunk/lib/Basic/LangOptions.cpp
    cfe/trunk/lib/Frontend/CompilerInvocation.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Basic/Sanitizers.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.h?rev=304463&r1=304462&r2=304463&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Sanitizers.h (original)
+++ cfe/trunk/include/clang/Basic/Sanitizers.h Thu Jun  1 15:01:01 2017
@@ -61,8 +61,8 @@ struct SanitizerSet {
     Mask = Value ? (Mask | K) : (Mask & ~K);
   }
 
-  /// \brief Disable all sanitizers.
-  void clear() { Mask = 0; }
+  /// Disable the sanitizers specified in \p K.
+  void clear(SanitizerMask K = SanitizerKind::All) { Mask &= ~K; }
 
   /// \brief Returns true if at least one sanitizer is enabled.
   bool empty() const { return Mask == 0; }
@@ -79,6 +79,12 @@ SanitizerMask parseSanitizerValue(String
 /// this group enables.
 SanitizerMask expandSanitizerGroups(SanitizerMask Kinds);
 
+/// Return the sanitizers which do not affect preprocessing.
+static inline SanitizerMask getPPTransparentSanitizers() {
+  return SanitizerKind::CFI | SanitizerKind::Integer |
+         SanitizerKind::Nullability | SanitizerKind::Undefined;
+}
+
 }  // end namespace clang
 
 #endif

Modified: cfe/trunk/lib/Basic/LangOptions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/LangOptions.cpp?rev=304463&r1=304462&r2=304463&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/LangOptions.cpp (original)
+++ cfe/trunk/lib/Basic/LangOptions.cpp Thu Jun  1 15:01:01 2017
@@ -29,9 +29,7 @@ void LangOptions::resetNonModularOptions
   Name = Default;
 #include "clang/Basic/LangOptions.def"
 
-  // FIXME: This should not be reset; modules can be different with different
-  // sanitizer options (this affects __has_feature(address_sanitizer) etc).
-  Sanitize.clear();
+  // These options do not affect AST generation.
   SanitizerBlacklistFiles.clear();
   XRayAlwaysInstrumentFiles.clear();
   XRayNeverInstrumentFiles.clear();

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=304463&r1=304462&r2=304463&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Thu Jun  1 15:01:01 2017
@@ -2700,6 +2700,13 @@ std::string CompilerInvocation::getModul
     code = ext->hashExtension(code);
   }
 
+  // Extend the signature with the enabled sanitizers, if at least one is
+  // enabled. Sanitizers which cannot affect AST generation aren't hashed.
+  SanitizerSet SanHash = LangOpts->Sanitize;
+  SanHash.clear(getPPTransparentSanitizers());
+  if (!SanHash.empty())
+    code = hash_combine(code, SanHash.Mask);
+
   return llvm::APInt(64, code).toString(36, /*Signed=*/false);
 }
 

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=304463&r1=304462&r2=304463&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Jun  1 15:01:01 2017
@@ -292,6 +292,33 @@ static bool checkLanguageOptions(const L
     return true;
   }
 
+  // Sanitizer feature mismatches are treated as compatible differences. If
+  // compatible differences aren't allowed, we still only want to check for
+  // mismatches of non-modular sanitizers (the only ones which can affect AST
+  // generation).
+  if (!AllowCompatibleDifferences) {
+    SanitizerMask ModularSanitizers = getPPTransparentSanitizers();
+    SanitizerSet ExistingSanitizers = ExistingLangOpts.Sanitize;
+    SanitizerSet ImportedSanitizers = LangOpts.Sanitize;
+    ExistingSanitizers.clear(ModularSanitizers);
+    ImportedSanitizers.clear(ModularSanitizers);
+    if (ExistingSanitizers.Mask != ImportedSanitizers.Mask) {
+      const std::string Flag = "-fsanitize=";
+      if (Diags) {
+#define SANITIZER(NAME, ID)                                                    \
+  {                                                                            \
+    bool InExistingModule = ExistingSanitizers.has(SanitizerKind::ID);         \
+    bool InImportedModule = ImportedSanitizers.has(SanitizerKind::ID);         \
+    if (InExistingModule != InImportedModule)                                  \
+      Diags->Report(diag::err_pch_targetopt_feature_mismatch)                  \
+          << InExistingModule << (Flag + NAME);                                \
+  }
+#include "clang/Basic/Sanitizers.def"
+      }
+      return true;
+    }
+  }
+
   return false;
 }
 

Added: cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/check.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/check.h?rev=304463&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/check.h (added)
+++ cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/check.h Thu Jun  1 15:01:01 2017
@@ -0,0 +1,5 @@
+#if __has_feature(address_sanitizer)
+#define HAS_ASAN 1
+#else
+#define HAS_ASAN 0
+#endif

Added: cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/map?rev=304463&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/map (added)
+++ cfe/trunk/test/Modules/Inputs/check-for-sanitizer-feature/map Thu Jun  1 15:01:01 2017
@@ -0,0 +1,3 @@
+module check_feature {
+  header "check.h"
+}

Added: cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp?rev=304463&view=auto
==============================================================================
--- cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp (added)
+++ cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp Thu Jun  1 15:01:01 2017
@@ -0,0 +1,66 @@
+// RUN: rm -rf %t.1 %t.2
+// RUN: mkdir %t.1 %t.2
+
+// Build and use an ASan-enabled module.
+// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t.1 \
+// RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
+// RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
+// RUN: ls %t.1 | count 2
+
+// Force a module rebuild by disabling ASan.
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.1 \
+// RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
+// RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
+// RUN: ls %t.1 | count 3
+
+// Enable ASan again: check that there is no import failure, and no rebuild.
+// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t.1 \
+// RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
+// RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
+// RUN: ls %t.1 | count 3
+
+// Some sanitizers can not affect AST generation when enabled. Check that
+// module rebuilds don't occur when these sanitizers are enabled.
+//
+// First, build without any sanitization.
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.2 \
+// RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
+// RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
+// RUN: ls %t.2 | count 2
+//
+// Next, build with sanitization, and check that a new module isn't built.
+// RUN: %clang_cc1 -fsanitize=cfi-vcall,unsigned-integer-overflow,nullability-arg,null -fmodules \
+// RUN:   -fmodules-cache-path=%t.2 \
+// RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
+// RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
+// RUN: ls %t.2 | count 2
+
+// Finally, test that including enabled sanitizers in the module hash isn't
+// required to ensure correctness of module imports.
+//
+// Emit a PCH with ASan enabled.
+// RUN: %clang_cc1 -x c -fsanitize=address %S/Inputs/check-for-sanitizer-feature/check.h -emit-pch -o %t.asan_pch
+//
+// Import the PCH without ASan enabled (we expect an error).
+// RUN: not %clang_cc1 -x c -include-pch %t.asan_pch %s -verify 2>&1 | FileCheck %s --check-prefix=PCH_MISMATCH
+// PCH_MISMATCH: AST file was compiled with the target feature'-fsanitize=address' but the current translation unit is not
+//
+// Emit a PCH with UBSan enabled.
+// RUN: %clang_cc1 -x c -fsanitize=null %S/Inputs/check-for-sanitizer-feature/check.h -emit-pch -o %t.ubsan_pch
+//
+// Import the PCH without UBSan enabled (should work just fine).
+// RUN: %clang_cc1 -x c -include-pch %t.ubsan_pch %s -I %S/Inputs/check-for-sanitizer-feature -verify
+
+#include "check.h"
+
+#if __has_feature(address_sanitizer)
+#if HAS_ASAN != 1
+#error Module doesn't have the address_sanitizer feature, but main program does.
+#endif
+#else
+#if HAS_ASAN != 0
+#error Module has the address_sanitizer feature, but main program doesn't.
+#endif
+#endif
+
+// expected-no-diagnostics




More information about the cfe-commits mailing list