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

[Richard Smith via cfe-commits]

On 1 May 2017 at 16:43, Vedant Kumar via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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.
>

I would expect this to be permitted to differ between an explicit module
build and its use. (Ideally we would apply the sanitization settings from
the module build to the code generated for its inline functions in that
case, but that can wait.)


> 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 --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170505/3c156d67/attachment.html>