[Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pgousseau]

Summary

enum SanitizerOrdinal has reached maximum capacity, this change extends the capacity to 128 sanitizer checks.
This can eventually allow us to add gcc 8's options "-fsanitize=pointer-substract" and "-fsanitize=pointer-compare".

Fixes: https://llvm.org/PR39425

[pgousseau]

Move num bits constant inside class.

[pgousseau]

Updated patch to not use APInt for representing the mask as it feels like a hammer solution.

[riccibruno]

In D57914#1401367, @pgousseau wrote:

Updated patch to not use APInt for representing the mask as it feels like a hammer solution.

Ah! I was going to say exactly this.

[riccibruno]

include/clang/Basic/Sanitizers.h

44

Why not use a fixed size array of uint64_ts, and then compute the index without any branch with ArrayIndex = BitPosition / 64 (with an assertion that ArrayIndex < MaxArrayIndex ? This has the advantage that in the future all that is needed to do is bump up the size of the array.

[pgousseau]

include/clang/Basic/Sanitizers.h

44

Sounds good! I will give it a try.

[pgousseau]

Updated to use an array as suggested.

[riccibruno]

Looks mostly fine to me. I have added a few inline comments but they are all little nits.

include/clang/Basic/Sanitizers.h

40

struct -> class and get rid of the private ?

47

/// instead to have doxygen handle these comments (here and elsewhere).

52

You don't have to explicitly write the default ctor; you can just instead write uint64_t maskLoToHigh[kNumElem]{}; above to value-initialize each element in the array.

55

SanitizerOrdinal should probably be passed by value here and elsewhere.

62

Could you please add a relevant message to the assertion (here and elsewhere) ?

97

Is that vectorized by gcc/clang, or is that a sequence of branches ?

lib/Driver/SanitizerArgs.cpp

53

Shouldn't all of these masks be const to be sure that they are not modified by mistake (unless I a missing something and they are modified somewhere else) ?

[pgousseau]

Applied suggested changes.

[pgousseau]

include/clang/Basic/Sanitizers.h

97

clang 6 or gcc 7.3 does not vectorize but clang 8 does!

[riccibruno]

LGTM with an additional comment.

include/clang/Basic/Sanitizers.h

82

This operator should be const-qualified.

97

Nice, a round of applause for clang 8 I guess then. Thanks for looking!

[riccibruno]

Wait no, can you really define the SanitizerMasks in the header ? Isn't that an odr violation ?

[pgousseau]

In D57914#1404053, @riccibruno wrote:

Wait no, can you really define the SanitizerMasks in the header ? Isn't that an odr violation ?

A yes I better move the definitions, thanks!

[riccibruno]

I think what you would really want to do is mark the masks as inline constexpr, but sadly inline variables are C++17 only. I have seen some ways to emulate inline variables but I am not sure whether it is worth doing so in this case.

[pgousseau]

In D57914#1405615, @riccibruno wrote:

I think what you would really want to do is mark the masks as inline constexpr, but sadly inline variables are C++17 only. I have seen some ways to emulate inline variables but I am not sure whether it is worth doing so in this case.

Yes thanks for the advice.
I have tried moving the definitions to the cpp file but ran into initialization order fiasco issue because of SanitizerArgs.cpp global definitions.
If using a constexpr SanitizerMask ctor then that would work around the initialization fiasco issue hopefully?
Although I am not having much luck using constexpr ctor in VS2015 because of the array member, so might have to revert back to lo/hi mask members?

[riccibruno]

In D57914#1405665, @pgousseau wrote:

In D57914#1405615, @riccibruno wrote:

I think what you would really want to do is mark the masks as inline constexpr, but sadly inline variables are C++17 only. I have seen some ways to emulate inline variables but I am not sure whether it is worth doing so in this case.

Yes thanks for the advice.
I have tried moving the definitions to the cpp file but ran into initialization order fiasco issue because of SanitizerArgs.cpp global definitions.
If using a constexpr SanitizerMask ctor then that would work around the initialization fiasco issue hopefully?
Although I am not having much luck using constexpr ctor in VS2015 because of the array member, so might have to revert back to lo/hi mask members?

I would prefer to avoid doing this. What is the problem with constexpr ?

[pgousseau]

In D57914#1405781, @riccibruno wrote:

In D57914#1405665, @pgousseau wrote:

In D57914#1405615, @riccibruno wrote:

I think what you would really want to do is mark the masks as inline constexpr, but sadly inline variables are C++17 only. I have seen some ways to emulate inline variables but I am not sure whether it is worth doing so in this case.

Yes thanks for the advice.
I have tried moving the definitions to the cpp file but ran into initialization order fiasco issue because of SanitizerArgs.cpp global definitions.
If using a constexpr SanitizerMask ctor then that would work around the initialization fiasco issue hopefully?
Although I am not having much luck using constexpr ctor in VS2015 because of the array member, so might have to revert back to lo/hi mask members?

I would prefer to avoid doing this. What is the problem with constexpr ?

I thought of using constexpr SanitizerMask Foo = {lo, hi}; as this seems supported in VS2015 but I cant seem to find a way to get this to work with array members...
Now I also realised I would need something like constexpr SanitizerMask FooGroup = Foo + Bar;
And I havent found a way yet, arrays or no arrays, any ideas?

[riccibruno]

To define the masks as constexpr variables you will have to make at least bitPosToMask, operator|, operator& and operator~ constexpr. Unfortunately constexpr functions in c++11 are rather restricted. It seems to me however that you could do the following:

1. Wait until we can use C++14 in llvm (soon hopefully) to mark the masks contexpr (and leave a FIXME for now).
2. Solve the odr-violation issue by using one of the work-around in n4424. In particular I like the second work-around consisting of putting the static members in a class template.

[riccibruno]

More explicitly something like:

in Sanitizer.h:

template <typename T = void> struct SanitizerMasks {
  static const SanitizerMask SomeMask;
  /* and so on for each mask*/
};

template <typename T> const SanitizerMask SanitizerMasks<T>::SomeMask = the definition;

And then you can write SanitizerMasks<>::SomeMask when you want to use this mask.

[pgousseau]

In D57914#1405955, @riccibruno wrote:

More explicitly something like:

in Sanitizer.h:

template <typename T = void> struct SanitizerMasks {
  static const SanitizerMask SomeMask;
  /* and so on for each mask*/
};

template <typename T> const SanitizerMask SanitizerMasks<T>::SomeMask = the definition;

And then you can write SanitizerMasks<>::SomeMask when you want to use this mask.

Sounds good to me, I will try that thanks!

[pgousseau]

Fix odr violation issue using static data member of a class template as suggested.

[pgousseau]

Rework FIXME comment.

[riccibruno]

include/clang/Basic/Sanitizers.h

117

Not replaced. constexpr is nearly orthogonal to the odr issue. It can be replaced by inline variables in C++17. I think that you should leave two FIXME here: the first one about marking the masks`constexpr` in C++14, and the second one about replacing the workaround by marking the masks inline.

142

You are back to the same issue with the references.

[pgousseau]

include/clang/Basic/Sanitizers.h

117

Makes sense thanks!

142

A yes I should have read the odr definition more carefully! I am trying to find a way to not have to add SanitizerMasks<>:: all over the place, any ideas?

[riccibruno]

include/clang/Basic/Sanitizers.h

142

Nope, unfortunately.

(And I am wondering how many other similar issues exist in clang/llvm; I would bet it is > 0).

[pgousseau]

Fix bad use of reference as pointed out, aliased SanitizerKind to SanitizerMasks<> instead.

[riccibruno]

It looks better now as far as I can see. I like the idea of aliasing SanitizerKind to SanitizerMasks<>;.

include/clang/Basic/Sanitizers.h

106

SanitizerOrdinal used to be in SanitizerKind. Would it make sense to keep this by moving it to SanitizerMasks ?

126

"Force instantiate here [...]" -> "Explicit instantiation here [...]" ?

[pgousseau]

Keep enum SanitizerOrdinal as it was inside SanitizerKind namespace, fix comment.

[pgousseau]

include/clang/Basic/Sanitizers.h

55

Yes I meant to initially, done now thanks!

[riccibruno]

I think this looks good now. Thanks !

[pgousseau]

In D57914#1410387, @riccibruno wrote:

I think this looks good now. Thanks !

Pushed at r354873, thanks for the help!

[pgousseau]

reverted at r354875 at this break lldb build.

[pgousseau]

Reopening because of buildbot failure http://green.lab.llvm.org/green/job/lldb-cmake/20537/

I have not been able to reproduce the error, the order of includes must be different on the failing bot...
As suggested by the compiler warning, moving hash_value(const clang::SanitizerMask&) declaration into clang's namespace should fix it.

FAILED: tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o 
/Users/buildslave/jenkins/workspace/lldb-cmake/host-compiler/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/clang/lib/Frontend -I/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/lib/Frontend -I/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/include -Itools/clang/include -I/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk/usr/include/libxml2 -Iinclude -I/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -fmodules -fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/module.cache -fcxx-modules -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3  -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.14.sdk -mmacosx-version-min=10.9   -UNDEBUG  -fno-exceptions -fno-rtti -MD -MT tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -MF tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o.d -o tools/clang/lib/Frontend/CMakeFiles/clangFrontend.dir/CompilerInvocation.cpp.o -c /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp
In module 'LLVM_Utils' imported from /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/lib/Frontend/TestModuleFileExtension.h:13:
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/ADT/Hashing.h:374:10: error: call to function 'hash_value' that is neither visible in the template definition nor found by argument-dependent lookup
  return hash_value(value);
         ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/ADT/Hashing.h:555:63: note: in instantiation of function template specialization 'llvm::hashing::detail::get_hashable_data<clang::SanitizerMask>' requested here
    buffer_ptr = combine_data(length, buffer_ptr, buffer_end, get_hashable_data(arg));
                                                              ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/ADT/Hashing.h:558:12: note: in instantiation of function template specialization 'llvm::hashing::detail::hash_combine_recursive_helper::combine<clang::SanitizerMask>' requested here
    return combine(length, buffer_ptr, buffer_end, args...);
           ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/include/llvm/ADT/Hashing.h:603:17: note: in instantiation of function template specialization 'llvm::hashing::detail::hash_combine_recursive_helper::combine<llvm::hash_code, clang::SanitizerMask>' requested here
  return helper.combine(0, helper.buffer, helper.buffer + 64, args...);
                ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/lib/Frontend/CompilerInvocation.cpp:3430:12: note: in instantiation of function template specialization 'llvm::hash_combine<llvm::hash_code, clang::SanitizerMask>' requested here
    code = hash_combine(code, SanHash.Mask);
           ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/clang/include/clang/Basic/Sanitizers.h:29:11: note: 'hash_value' should be declared prior to the call site or in namespace 'clang'
hash_code hash_value(const clang::SanitizerMask &Arg);
          ^
1 error generated.

[pgousseau]

move hash_value declaration to clang's namespace to solve lldb cmake bot build error.

[riccibruno]

include/clang/Basic/Sanitizers.h

65

What ? You are forward-declaring hash_code here and using it as a value a few lines later. Just include llvm/ADT/Hashing.h.

[pgousseau]

include/clang/Basic/Sanitizers.h

65

Thanks for reviewing!
I am happy to include Hashing.h but I thought it was generally frown upon to add includes if avoidable?
hash_code is only used in the return type of the hash_value() declaration here no?
I saw StringRef.h or APInt.h also forward declare hash_code.
Thanks again for reviewing and sorry for the many iterations.

[riccibruno]

include/clang/Basic/Sanitizers.h

65

Never mind, I overlooked that it is only the declaration of llvm::hash_code hash_value(const clang::SanitizerMask &Arg); and not the definition. Sorry about that.

I believe that is it correct to have the overload of hash_value in the namespace clang, so that it can be found by ADL since SanitizerMask is in clang. However you can probably move the declaration of llvm::hash_code hash_value(const clang::SanitizerMask &Arg); just after SanitizerMask so that you don't have to forward-declare SanitizerMask (but keep the forward-declaration of hash_code in llvm).

[pgousseau]

Change hash_value() declaration's location as suggested.

[pgousseau]

include/clang/Basic/Sanitizers.h

65

Sounds good, done!

[riccibruno]

Looks good, hopefully this time it will stick. thanks!

[jyknight]

The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in rL355278.

[pgousseau]

In D57914#1416215, @jyknight wrote:

The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in rL355278.

Thanks for finding the issue! All constexpr sounds good!

[riccibruno]

In D57914#1416215, @jyknight wrote:

The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in rL355278.

Looking at what guarantees are given for dynamic initialization (in [basic.start.dynamic]), it seems that no guarantees are given if the variable is an explicit or implicit instantiation. So unless I am mistaken the original code was not guaranteed to work indeed. Apologies for missing that!

include/clang/Basic/Attr.td

 
 def NoSanitize : InheritableAttr {
   let Spellings = [Clang<"no_sanitize">];
   let Args = [VariadicStringArgument<"Sanitizers">];
   let Subjects = SubjectList<[Function, ObjCMethod, GlobalVar], ErrorDiag>;
   let Documentation = [NoSanitizeDocs];
   let AdditionalMembers = [{
     SanitizerMask getMask() const {
-      SanitizerMask Mask = 0;
+      SanitizerMask Mask(0);
       for (auto SanitizerName : sanitizers()) {
         SanitizerMask ParsedMask =
             parseSanitizerValue(SanitizerName, /*AllowGroups=*/true);
         Mask |= expandSanitizerGroups(ParsedMask);
       }
       return Mask;
     }
   }];

include/clang/Basic/Sanitizers.h

 /// Defines the clang::SanitizerKind enum.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_BASIC_SANITIZERS_H
 #define LLVM_CLANG_BASIC_SANITIZERS_H
 
 #include "clang/Basic/LLVM.h"
+#include "llvm/ADT/APInt.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MathExtras.h"
 #include <cassert>
 #include <cstdint>
 
 namespace clang {
+struct SanitizerMask;
+}
+
+namespace llvm {
+class hashcode;
+hash_code hash_value(const clang::SanitizerMask &Arg);
+} // namespace llvm
+
+namespace clang {
+
+namespace SanitizerKind {
+enum SanitizerOrdinal : uint64_t;
+}
+
+struct SanitizerMask {
+private:
+  llvm::APInt mask;
+
+public:
+  // Size of SanitizerMask's bit mask in bits.
+  static const int kNumBits = 128;
+
+  SanitizerMask() : mask(kNumBits, 0) {}
+
+  explicit SanitizerMask(uint64_t val) : mask(kNumBits, val) {}
+
+  explicit SanitizerMask(const llvm::APInt &v) : mask(v) {}
+
+  void flipAllBits() { mask.flipAllBits(); }
+
+  bool isPowerOf2() const { return mask.isPowerOf2(); }
+
+  unsigned countPopulation() const { return mask.countPopulation(); }
+
+  llvm::hash_code hash_value() const;
+
+  explicit operator bool() { return mask.getBoolValue(); };
 
-using SanitizerMask = uint64_t;
+  SanitizerMask operator<<(const SanitizerKind::SanitizerOrdinal &Bits) const {
+    assert(Bits < kNumBits && "Bit shift too big.");
+    return SanitizerMask(mask << Bits);
+  }
+
+  bool operator==(const SanitizerMask &V) const { return mask == V.mask; }
+
+  bool operator==(uint64_t Val) const { return mask == Val; }
+
+  SanitizerMask &operator&=(const SanitizerMask &RHS) {
+    mask &= RHS.mask;
+    return *this;
+  }
+
+  SanitizerMask &operator|=(const SanitizerMask &RHS) {
+    mask |= RHS.mask;
+    return *this;
+  }
+
+  bool operator!() const { return !mask; }
+
+  bool operator!=(const SanitizerMask &RHS) const { return !((*this) == RHS); }
+};
+
+inline SanitizerMask operator~(SanitizerMask v) {
+  v.flipAllBits();
+  return v;
+}
+
+inline SanitizerMask operator&(SanitizerMask a, const SanitizerMask &b) {
+  a &= b;
+  return a;
+}
+
+inline SanitizerMask operator|(SanitizerMask a, const SanitizerMask &b) {
+  a |= b;
+  return SanitizerMask(a);
+}
 
 namespace SanitizerKind {
 
 // Assign ordinals to possible values of -fsanitize= flag, which we will use as
 // bit positions.
 enum SanitizerOrdinal : uint64_t {
 #define SANITIZER(NAME, ID) SO_##ID,
 #define SANITIZER_GROUP(NAME, ID, ALIAS) SO_##ID##Group,
 #include "clang/Basic/Sanitizers.def"
   SO_Count
 };
 
 // Define the set of sanitizer kinds, as well as the set of sanitizers each
 // sanitizer group expands into.
-#define SANITIZER(NAME, ID) \
-  const SanitizerMask ID = 1ULL << SO_##ID;
-#define SANITIZER_GROUP(NAME, ID, ALIAS) \
-  const SanitizerMask ID = ALIAS; \
-  const SanitizerMask ID##Group = 1ULL << SO_##ID##Group;
+#define SANITIZER(NAME, ID)                                                    \
+  static_assert(SO_##ID < SanitizerMask::kNumBits,                             \
+                "Bit shift value too big.");                                   \
+  const SanitizerMask ID = SanitizerMask(1ULL) << SO_##ID;
+#define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
+  static_assert(SO_##ID##Group < SanitizerMask::kNumBits,                      \
+                "Bit shift value too big.");                                   \
+  const SanitizerMask ID = SanitizerMask(ALIAS);                               \
+  const SanitizerMask ID##Group = SanitizerMask(1ULL) << SO_##ID##Group;
 #include "clang/Basic/Sanitizers.def"
 
 } // namespace SanitizerKind
 
 struct SanitizerSet {
   /// Check if a certain (single) sanitizer is enabled.
   bool has(SanitizerMask K) const {
-    assert(llvm::isPowerOf2_64(K));
-    return Mask & K;
+    assert(K.isPowerOf2());
+    return static_cast<bool>(Mask & K);
   }
 
   /// Check if one or more sanitizers are enabled.
-  bool hasOneOf(SanitizerMask K) const { return Mask & K; }
+  bool hasOneOf(SanitizerMask K) const { return static_cast<bool>(Mask & K); }
 
   /// Enable or disable a certain (single) sanitizer.
   void set(SanitizerMask K, bool Value) {
-    assert(llvm::isPowerOf2_64(K));
+    assert(K.isPowerOf2());
     Mask = Value ? (Mask | K) : (Mask & ~K);
   }
 
   /// Disable the sanitizers specified in \p K.
   void clear(SanitizerMask K = SanitizerKind::All) { Mask &= ~K; }
 
   /// Returns true if no sanitizers are enabled.
   bool empty() const { return Mask == 0; }
 
   /// Bitmask of enabled sanitizers.
-  SanitizerMask Mask = 0;
+  SanitizerMask Mask = SanitizerMask(0);
 };
 
 /// Parse a single value from a -fsanitize= or -fno-sanitize= value list.
 /// Returns a non-zero SanitizerMask, or \c 0 if \p Value is not known.
 SanitizerMask parseSanitizerValue(StringRef Value, bool AllowGroups);
 
 /// For each sanitizer group bit set in \p Kinds, set the bits for sanitizers
 /// this group enables.

include/clang/Driver/ToolChain.h

public:
   /// On Windows, returns the MSVC compatibility version.
   virtual VersionTuple computeMSVCVersion(const Driver *D,
                                           const llvm::opt::ArgList &Args) const;
 
   /// Return sanitizers which are available in this toolchain.
   virtual SanitizerMask getSupportedSanitizers() const;
 
   /// Return sanitizers which are enabled by default.
-  virtual SanitizerMask getDefaultSanitizers() const { return 0; }
+  virtual SanitizerMask getDefaultSanitizers() const {
+    return SanitizerMask(0);
+  }
 };
 
 /// Set a ToolChain's effective triple. Reset it when the registration object
 /// is destroyed.
 class RegisterEffectiveTriple {
   const ToolChain &TC;
 
 public:

lib/Basic/SanitizerSpecialCaseList.cpp

SanitizerSpecialCaseList::createOrDie(const std::vector<std::string> &Paths) {
   std::string Error;
   if (auto SSCL = create(Paths, Error))
     return SSCL;
   llvm::report_fatal_error(Error);
 }
 
 void SanitizerSpecialCaseList::createSanitizerSections() {
   for (auto &S : Sections) {
-    SanitizerMask Mask = 0;
+    SanitizerMask Mask(0);
 
 #define SANITIZER(NAME, ID)                                                    \
   if (S.SectionMatcher->match(NAME))                                           \
     Mask |= SanitizerKind::ID;
 #define SANITIZER_GROUP(NAME, ID, ALIAS) SANITIZER(NAME, ID)
 
 #include "clang/Basic/Sanitizers.def"
 #undef SANITIZER

lib/Basic/Sanitizers.cpp

 //===- Sanitizers.cpp - C Language Family Language Options ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the classes from Sanitizers.h
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/Sanitizers.h"
+#include "llvm/ADT/Hashing.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace clang;
 
 SanitizerMask clang::parseSanitizerValue(StringRef Value, bool AllowGroups) {
   SanitizerMask ParsedKind = llvm::StringSwitch<SanitizerMask>(Value)
 #define SANITIZER(NAME, ID) .Case(NAME, SanitizerKind::ID)
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
-  .Case(NAME, AllowGroups ? SanitizerKind::ID##Group : 0)
+  .Case(NAME, AllowGroups ? SanitizerKind::ID##Group : SanitizerMask(0))
 #include "clang/Basic/Sanitizers.def"
-    .Default(0);
+    .Default(SanitizerMask(0));
   return ParsedKind;
 }
 
 SanitizerMask clang::expandSanitizerGroups(SanitizerMask Kinds) {
 #define SANITIZER(NAME, ID)
 #define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
   if (Kinds & SanitizerKind::ID##Group)                                        \
     Kinds |= SanitizerKind::ID;
 #include "clang/Basic/Sanitizers.def"
   return Kinds;
 }
+
+llvm::hash_code SanitizerMask::hash_value() const {
+  return llvm::hash_value(mask);
+}
+
+namespace llvm {
+hash_code hash_value(const clang::SanitizerMask &Arg) {
+  return Arg.hash_value();
+}
+} // namespace llvm

lib/CodeGen/CGExpr.cpp

enum class CheckRecoverableKind {
   /// non-fatal handlers for this check.
   Recoverable,
   /// Runtime conditionally aborts, always need to support recovery.
   AlwaysRecoverable
 };
 }
 
 static CheckRecoverableKind getRecoverableKind(SanitizerMask Kind) {
-  assert(llvm::countPopulation(Kind) == 1);
-  switch (Kind) {
-  case SanitizerKind::Vptr:
+  assert(Kind.countPopulation() == 1);
+  if (Kind == SanitizerKind::Vptr)
     return CheckRecoverableKind::AlwaysRecoverable;
-  case SanitizerKind::Return:
-  case SanitizerKind::Unreachable:
+  else if (Kind == SanitizerKind::Return || Kind == SanitizerKind::Unreachable)
     return CheckRecoverableKind::Unrecoverable;
-  default:
+  else
     return CheckRecoverableKind::Recoverable;
-  }
-}
 
 namespace {
 struct SanitizerHandlerInfo {
   char const *const Name;
   unsigned Version;
 };
 }
 

lib/Driver/SanitizerArgs.cpp

 #include "llvm/Support/TargetParser.h"
 #include <memory>
 
 using namespace clang;
 using namespace clang::SanitizerKind;
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
-                     Memory | KernelMemory | Leak | Undefined | Integer |
-                     ImplicitConversion | Nullability | DataFlow | Fuzzer |
-                     FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-                      ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-      CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+SanitizerMask NeedsUbsanRt =
+    Undefined | Integer | ImplicitConversion | Nullability | CFI;
+SanitizerMask NeedsUbsanCxxRt = Vptr | CFI;
+SanitizerMask NotAllowedWithTrap = Vptr;
+SanitizerMask NotAllowedWithMinimalRuntime = Vptr;
+SanitizerMask RequiresPIE = DataFlow | HWAddress | Scudo;
+SanitizerMask NeedsUnwindTables =
+    Address | HWAddress | Thread | Memory | DataFlow;
+SanitizerMask SupportsCoverage =
+    Address | HWAddress | KernelAddress | KernelHWAddress | Memory |
+    KernelMemory | Leak | Undefined | Integer | ImplicitConversion |
+    Nullability | DataFlow | Fuzzer | FuzzerNoLink;
+SanitizerMask RecoverableByDefault =
+    Undefined | Integer | ImplicitConversion | Nullability;
+SanitizerMask Unrecoverable = Unreachable | Return;
+SanitizerMask AlwaysRecoverable = KernelAddress | KernelHWAddress;
+SanitizerMask LegacyFsanitizeRecoverMask = Undefined | Integer;
+SanitizerMask NeedsLTO = CFI;
+SanitizerMask TrappingSupported = (Undefined & ~Vptr) |
+                                  UnsignedIntegerOverflow | ImplicitConversion |
+                                  Nullability | LocalBounds | CFI;
+SanitizerMask TrappingDefault = CFI;
+SanitizerMask CFIClasses =
+    CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast;
+SanitizerMask CompatibleWithMinimalRuntime =
+    TrappingSupported | Scudo | ShadowCallStack;
 
 enum CoverageFeature {
   CoverageFunc = 1 << 0,
   CoverageBB = 1 << 1,
   CoverageEdge = 1 << 2,
   CoverageIndirCall = 1 << 3,
   CoverageTraceBB = 1 << 4,  // Deprecated.
   CoverageTraceCmp = 1 << 5,
#define SANITIZER_GROUP(NAME, ID, ALIAS)                                       \
   if (Kinds & SanitizerKind::ID)                                               \
     Kinds |= SanitizerKind::ID##Group;
 #include "clang/Basic/Sanitizers.def"
   return Kinds;
 }
 
 static SanitizerMask parseSanitizeTrapArgs(const Driver &D,
                                            const llvm::opt::ArgList &Args) {
-  SanitizerMask TrapRemove = 0; // During the loop below, the accumulated set of
+  SanitizerMask TrapRemove(0);  // During the loop below, the accumulated set of
                                 // sanitizers disabled by the current sanitizer
                                 // argument or any argument after it.
-  SanitizerMask TrappingKinds = 0;
+  SanitizerMask TrappingKinds(0);
   SanitizerMask TrappingSupportedWithGroups = setGroupBits(TrappingSupported);
 
   for (ArgList::const_reverse_iterator I = Args.rbegin(), E = Args.rend();
        I != E; ++I) {
     const auto *Arg = *I;
     if (Arg->getOption().matches(options::OPT_fsanitize_trap_EQ)) {
       Arg->claim();
       SanitizerMask Add = parseArgValues(D, Arg, true);
return (Sanitizers.Mask & CFI & ~TrapSanitizers.Mask) && CfiCrossDso &&
          !ImplicitCfiRuntime;
 }
 
 bool SanitizerArgs::requiresPIE() const {
   return NeedPIE || (Sanitizers.Mask & RequiresPIE);
 }
 
 bool SanitizerArgs::needsUnwindTables() const {
-  return Sanitizers.Mask & NeedsUnwindTables;
+  return static_cast<bool>(Sanitizers.Mask & NeedsUnwindTables);
 }
 
-bool SanitizerArgs::needsLTO() const { return Sanitizers.Mask & NeedsLTO; }
+bool SanitizerArgs::needsLTO() const {
+  return static_cast<bool>(Sanitizers.Mask & NeedsLTO);
+}
 
 SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                              const llvm::opt::ArgList &Args) {
-  SanitizerMask AllRemove = 0;  // During the loop below, the accumulated set of
+  SanitizerMask AllRemove(0);   // During the loop below, the accumulated set of
                                 // sanitizers disabled by the current sanitizer
                                 // argument or any argument after it.
-  SanitizerMask AllAddedKinds = 0;  // Mask of all sanitizers ever enabled by
+  SanitizerMask AllAddedKinds(0);   // Mask of all sanitizers ever enabled by
                                     // -fsanitize= flags (directly or via group
                                     // expansion), some of which may be disabled
                                     // later. Used to carefully prune
                                     // unused-argument diagnostics.
-  SanitizerMask DiagnosedKinds = 0;  // All Kinds we have diagnosed up to now.
+  SanitizerMask DiagnosedKinds(0);   // All Kinds we have diagnosed up to now.
                                      // Used to deduplicate diagnostics.
-  SanitizerMask Kinds = 0;
+  SanitizerMask Kinds(0);
   const SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers());
 
   CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso,
                              options::OPT_fno_sanitize_cfi_cross_dso, false);
 
   ToolChain::RTTIMode RTTIMode = TC.getRTTIMode();
 
   const Driver &D = TC.getDriver();
SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   }
   // FIXME: Currently -fsanitize=leak is silently ignored in the presence of
   // -fsanitize=address. Perhaps it should print an error, or perhaps
   // -f(-no)sanitize=leak should change whether leak detection is enabled by
   // default in ASan?
 
   // Parse -f(no-)?sanitize-recover flags.
   SanitizerMask RecoverableKinds = RecoverableByDefault | AlwaysRecoverable;
-  SanitizerMask DiagnosedUnrecoverableKinds = 0;
-  SanitizerMask DiagnosedAlwaysRecoverableKinds = 0;
+  SanitizerMask DiagnosedUnrecoverableKinds(0);
+  SanitizerMask DiagnosedAlwaysRecoverableKinds(0);
   for (const auto *Arg : Args) {
     const char *DeprecatedReplacement = nullptr;
     if (Arg->getOption().matches(options::OPT_fsanitize_recover)) {
       DeprecatedReplacement =
           "-fsanitize-recover=undefined,integer' or '-fsanitize-recover=all";
       RecoverableKinds |= expandSanitizerGroups(LegacyFsanitizeRecoverMask);
       Arg->claim();
     } else if (Arg->getOption().matches(options::OPT_fno_sanitize_recover)) {
SanitizerMask parseArgValues(const Driver &D, const llvm::opt::Arg *A,
                              bool DiagnoseErrors) {
   assert((A->getOption().matches(options::OPT_fsanitize_EQ) ||
           A->getOption().matches(options::OPT_fno_sanitize_EQ) ||
           A->getOption().matches(options::OPT_fsanitize_recover_EQ) ||
           A->getOption().matches(options::OPT_fno_sanitize_recover_EQ) ||
           A->getOption().matches(options::OPT_fsanitize_trap_EQ) ||
           A->getOption().matches(options::OPT_fno_sanitize_trap_EQ)) &&
          "Invalid argument in parseArgValues!");
-  SanitizerMask Kinds = 0;
+  SanitizerMask Kinds(0);
   for (int i = 0, n = A->getNumValues(); i != n; ++i) {
     const char *Value = A->getValue(i);
     SanitizerMask Kind;
     // Special case: don't accept -fsanitize=all.
     if (A->getOption().matches(options::OPT_fsanitize_EQ) &&
         0 == strcmp("all", Value))
-      Kind = 0;
+      Kind = SanitizerMask(0);
     // Similarly, don't accept -fsanitize=efficiency-all.
     else if (A->getOption().matches(options::OPT_fsanitize_EQ) &&
         0 == strcmp("efficiency-all", Value))
-      Kind = 0;
+      Kind = SanitizerMask(0);
     else
       Kind = parseSanitizerValue(Value, /*AllowGroups=*/true);
 
     if (Kind)
       Kinds |= Kind;
     else if (DiagnoseErrors)
       D.Diag(clang::diag::err_drv_unsupported_option_argument)
           << A->getOption().getName() << Value;