vsk created this revision. 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 can cause strange failures at runtime. One case I've seen affects libcxx, since its vector implementation behaves differently when ASan is enabled.
Implicit module builds should work even when -fsanitize=X is toggled on and off across multiple compiler invocations. This patch implements a fix by including the set of enabled sanitizers in the module hash. I am not sure if the module hash should change when building with explicit modules, and would appreciate any input on this. https://reviews.llvm.org/D32724 Files: lib/Basic/LangOptions.cpp lib/Frontend/CompilerInvocation.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,32 @@ +// RUN: rm -rf %t +// RUN: mkdir %t + +// Build and use an ASan-enabled module. +// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +// Force a module rebuild by disabling ASan. +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +// Force another module rebuild by enabling ASan again. +// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +#include "check.h" + +#if __has_feature(address_sanitizer) +#if HAS_ASAN != 1 +#error Does not have the address_sanitizer feature, but should. +#endif +#else +#if HAS_ASAN != 0 +#error Has the address_sanitizer feature, but shouldn'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/Frontend/CompilerInvocation.cpp =================================================================== --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2693,6 +2693,11 @@ code = ext->hashExtension(code); } + // Extend the signature with the enabled sanitizers, if at least one is + // enabled. + if (!LangOpts->Sanitize.empty()) + code = hash_combine(code, LangOpts->Sanitize.Mask); + return llvm::APInt(64, code).toString(36, /*Signed=*/false); } Index: lib/Basic/LangOptions.cpp =================================================================== --- lib/Basic/LangOptions.cpp +++ lib/Basic/LangOptions.cpp @@ -29,9 +29,11 @@ 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(); + // FIXME: Some sanitizers do not expose any information via the preprocessor + // (e.g UBSan). We may be able to avoid some module rebuilds by disabling + // those sanitizers. + + // These options do not affect AST generation. SanitizerBlacklistFiles.clear(); XRayAlwaysInstrumentFiles.clear(); XRayNeverInstrumentFiles.clear();
Index: test/Modules/check-for-sanitizer-feature.cpp =================================================================== --- /dev/null +++ test/Modules/check-for-sanitizer-feature.cpp @@ -0,0 +1,32 @@ +// RUN: rm -rf %t +// RUN: mkdir %t + +// Build and use an ASan-enabled module. +// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +// Force a module rebuild by disabling ASan. +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +// Force another module rebuild by enabling ASan again. +// RUN: %clang_cc1 -fsanitize=address -fmodules -fmodules-cache-path=%t \ +// RUN: -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \ +// RUN: -I %S/Inputs/check-for-sanitizer-feature -verify %s + +#include "check.h" + +#if __has_feature(address_sanitizer) +#if HAS_ASAN != 1 +#error Does not have the address_sanitizer feature, but should. +#endif +#else +#if HAS_ASAN != 0 +#error Has the address_sanitizer feature, but shouldn'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/Frontend/CompilerInvocation.cpp =================================================================== --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -2693,6 +2693,11 @@ code = ext->hashExtension(code); } + // Extend the signature with the enabled sanitizers, if at least one is + // enabled. + if (!LangOpts->Sanitize.empty()) + code = hash_combine(code, LangOpts->Sanitize.Mask); + return llvm::APInt(64, code).toString(36, /*Signed=*/false); } Index: lib/Basic/LangOptions.cpp =================================================================== --- lib/Basic/LangOptions.cpp +++ lib/Basic/LangOptions.cpp @@ -29,9 +29,11 @@ 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(); + // FIXME: Some sanitizers do not expose any information via the preprocessor + // (e.g UBSan). We may be able to avoid some module rebuilds by disabling + // those sanitizers. + + // These options do not affect AST generation. SanitizerBlacklistFiles.clear(); XRayAlwaysInstrumentFiles.clear(); XRayNeverInstrumentFiles.clear();
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits