[PATCH] D32724: [Modules] Include the enabled sanitizers in the module hash

[Vedant Kumar via Phabricator via cfe-commits]

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();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D32724.97364.patch
Type: text/x-patch
Size: 3137 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170501/c9548eba/attachment.bin>