vsk updated this revision to Diff 98349.
vsk marked 7 inline comments as done.
vsk added a comment.
Add a comment explaining how and why ASTReader checks for sanitizer feature
mismatches.
https://reviews.llvm.org/D32724
Files:
include/clang/Basic/Sanitizers.h
lib/Basic/LangOptions.cpp
lib/CodeGen/CGDeclCXX.cpp
lib/CodeGen/CGObjCMac.cpp
lib/CodeGen/CodeGenFunction.h
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/Frontend/CompilerInvocation.cpp
lib/Serialization/ASTReader.cpp
test/Modules/Inputs/check-for-sanitizer-feature/check.h
test/Modules/Inputs/check-for-sanitizer-feature/map
test/Modules/check-for-sanitizer-feature.cpp
Index: test/Modules/check-for-sanitizer-feature.cpp
===================================================================
--- /dev/null
+++ test/Modules/check-for-sanitizer-feature.cpp
@@ -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
Index: test/Modules/Inputs/check-for-sanitizer-feature/map
===================================================================
--- /dev/null
+++ test/Modules/Inputs/check-for-sanitizer-feature/map
@@ -0,0 +1,3 @@
+module check_feature {
+ header "check.h"
+}
Index: test/Modules/Inputs/check-for-sanitizer-feature/check.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/check-for-sanitizer-feature/check.h
@@ -0,0 +1,5 @@
+#if __has_feature(address_sanitizer)
+#define HAS_ASAN 1
+#else
+#define HAS_ASAN 0
+#endif
Index: lib/Serialization/ASTReader.cpp
===================================================================
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -292,6 +292,33 @@
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 = getModularSanitizers();
+ 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;
}
Index: lib/Frontend/CompilerInvocation.cpp
===================================================================
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2693,6 +2693,13 @@
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(getModularSanitizers());
+ if (!SanHash.empty())
+ code = hash_combine(code, SanHash.Mask);
+
return llvm::APInt(64, code).toString(36, /*Signed=*/false);
}
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -344,8 +344,8 @@
/// List of global values which are required to be present in the object file;
/// bitcast to i8*. This is used for forcing visibility of symbols which may
/// otherwise be optimized out.
- std::vector<llvm::WeakVH> LLVMUsed;
- std::vector<llvm::WeakVH> LLVMCompilerUsed;
+ std::vector<llvm::WeakTrackingVH> LLVMUsed;
+ std::vector<llvm::WeakTrackingVH> LLVMCompilerUsed;
/// Store the list of global constructors and their respective priorities to
/// be emitted when the translation unit is complete.
@@ -416,7 +416,7 @@
SmallVector<GlobalInitData, 8> PrioritizedCXXGlobalInits;
/// Global destructor functions and arguments that need to run on termination.
- std::vector<std::pair<llvm::WeakVH,llvm::Constant*> > CXXGlobalDtors;
+ std::vector<std::pair<llvm::WeakTrackingVH, llvm::Constant *>> CXXGlobalDtors;
/// \brief The complete set of modules that has been imported.
llvm::SetVector<clang::Module *> ImportedModules;
@@ -433,7 +433,7 @@
/// Cached reference to the class for constant strings. This value has type
/// int * but is actually an Obj-C class pointer.
- llvm::WeakVH CFConstantStringClassRef;
+ llvm::WeakTrackingVH CFConstantStringClassRef;
/// \brief The type used to describe the state of a fast enumeration in
/// Objective-C's for..in loop.
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1154,7 +1154,7 @@
}
static void emitUsed(CodeGenModule &CGM, StringRef Name,
- std::vector<llvm::WeakVH> &List) {
+ std::vector<llvm::WeakTrackingVH> &List) {
// Don't create llvm.used if there is no need.
if (List.empty())
return;
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3471,9 +3471,10 @@
/// GenerateCXXGlobalDtorsFunc - Generates code for destroying global
/// variables.
- void GenerateCXXGlobalDtorsFunc(llvm::Function *Fn,
- const std::vector<std::pair<llvm::WeakVH,
- llvm::Constant*> > &DtorsAndObjects);
+ void GenerateCXXGlobalDtorsFunc(
+ llvm::Function *Fn,
+ const std::vector<std::pair<llvm::WeakTrackingVH, llvm::Constant *>>
+ &DtorsAndObjects);
void GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
const VarDecl *D,
Index: lib/CodeGen/CGObjCMac.cpp
===================================================================
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -886,7 +886,7 @@
/// Cached reference to the class for constant strings. This value has type
/// int * but is actually an Obj-C class pointer.
- llvm::WeakVH ConstantStringClassRef;
+ llvm::WeakTrackingVH ConstantStringClassRef;
/// \brief The LLVM type corresponding to NSConstantString.
llvm::StructType *NSConstantStringType = nullptr;
Index: lib/CodeGen/CGDeclCXX.cpp
===================================================================
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -571,9 +571,10 @@
FinishFunction();
}
-void CodeGenFunction::GenerateCXXGlobalDtorsFunc(llvm::Function *Fn,
- const std::vector<std::pair<llvm::WeakVH, llvm::Constant*> >
- &DtorsAndObjects) {
+void CodeGenFunction::GenerateCXXGlobalDtorsFunc(
+ llvm::Function *Fn,
+ const std::vector<std::pair<llvm::WeakTrackingVH, llvm::Constant *>>
+ &DtorsAndObjects) {
{
auto NL = ApplyDebugLocation::CreateEmpty(*this);
StartFunction(GlobalDecl(), getContext().VoidTy, Fn,
Index: lib/Basic/LangOptions.cpp
===================================================================
--- lib/Basic/LangOptions.cpp
+++ lib/Basic/LangOptions.cpp
@@ -29,9 +29,7 @@
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();
Index: include/clang/Basic/Sanitizers.h
===================================================================
--- include/clang/Basic/Sanitizers.h
+++ include/clang/Basic/Sanitizers.h
@@ -61,8 +61,9 @@
Mask = Value ? (Mask | K) : (Mask & ~K);
}
- /// \brief Disable all sanitizers.
- void clear() { Mask = 0; }
+ /// \brief Disable the sanitizers specified in \p K (by default, disable
+ /// all sanitizers).
+ 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 +80,13 @@
/// this group enables.
SanitizerMask expandSanitizerGroups(SanitizerMask Kinds);
+/// Return the sanitizers which, when enabled, cannot affect preprocessing or
+/// AST generation.
+static inline SanitizerMask getModularSanitizers() {
+ return SanitizerKind::CFI | SanitizerKind::Integer |
+ SanitizerKind::Nullability | SanitizerKind::Undefined;
+}
+
} // end namespace clang
#endif
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
