r300591 - The SubjectMatchRule enum should not be used as a DenseMap key to avoid

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Tue Apr 18 15:54:23 2017
New Revision: 300591

URL: http://llvm.org/viewvc/llvm-project?rev=300591&view=rev
Log:
The SubjectMatchRule enum should not be used as a DenseMap key to avoid
UBSAN 'invalid value' failures

The commit r300556 introduced a UBSAN issue that was caught by
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap. The DenseMap
failed to create an empty/tombstone value as the empty/tombstone values for the
SubjectMatchRule enum were not valid enum constants.

Modified:
    cfe/trunk/include/clang/Basic/AttrSubjectMatchRules.h
    cfe/trunk/lib/Sema/SemaAttr.cpp

Modified: cfe/trunk/include/clang/Basic/AttrSubjectMatchRules.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/AttrSubjectMatchRules.h?rev=300591&r1=300590&r2=300591&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/AttrSubjectMatchRules.h (original)
+++ cfe/trunk/include/clang/Basic/AttrSubjectMatchRules.h Tue Apr 18 15:54:23 2017
@@ -24,23 +24,9 @@ enum SubjectMatchRule {
 
 const char *getSubjectMatchRuleSpelling(SubjectMatchRule Rule);
 
-using ParsedSubjectMatchRuleSet = llvm::DenseMap<SubjectMatchRule, SourceRange>;
+using ParsedSubjectMatchRuleSet = llvm::DenseMap<int, SourceRange>;
 
 } // end namespace attr
 } // end namespace clang
 
-namespace llvm {
-
-template <>
-struct DenseMapInfo<clang::attr::SubjectMatchRule> : DenseMapInfo<int> {
-  static inline clang::attr::SubjectMatchRule getEmptyKey() {
-    return (clang::attr::SubjectMatchRule)DenseMapInfo<int>::getEmptyKey();
-  }
-  static inline clang::attr::SubjectMatchRule getTombstoneKey() {
-    return (clang::attr::SubjectMatchRule)DenseMapInfo<int>::getTombstoneKey();
-  }
-};
-
-} // end namespace llvm
-
 #endif

Modified: cfe/trunk/lib/Sema/SemaAttr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAttr.cpp?rev=300591&r1=300590&r2=300591&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAttr.cpp Tue Apr 18 15:54:23 2017
@@ -440,12 +440,12 @@ void Sema::ActOnPragmaAttributePush(Attr
     //    variable(is_parameter).
     //  - a sub-rule and a sibling that's negated. E.g.
     //    variable(is_thread_local) and variable(unless(is_parameter))
-    llvm::SmallDenseMap<attr::SubjectMatchRule,
-                        std::pair<attr::SubjectMatchRule, SourceRange>, 2>
+    llvm::SmallDenseMap<int, std::pair<int, SourceRange>, 2>
         RulesToFirstSpecifiedNegatedSubRule;
     for (const auto &Rule : Rules) {
+      attr::SubjectMatchRule MatchRule = attr::SubjectMatchRule(Rule.first);
       Optional<attr::SubjectMatchRule> ParentRule =
-          getParentAttrMatcherRule(Rule.first);
+          getParentAttrMatcherRule(MatchRule);
       if (!ParentRule)
         continue;
       auto It = Rules.find(*ParentRule);
@@ -453,7 +453,7 @@ void Sema::ActOnPragmaAttributePush(Attr
         // A sub-rule contradicts a parent rule.
         Diag(Rule.second.getBegin(),
              diag::err_pragma_attribute_matcher_subrule_contradicts_rule)
-            << attr::getSubjectMatchRuleSpelling(Rule.first)
+            << attr::getSubjectMatchRuleSpelling(MatchRule)
             << attr::getSubjectMatchRuleSpelling(*ParentRule) << It->second
             << FixItHint::CreateRemoval(
                    replacementRangeForListElement(*this, Rule.second));
@@ -461,14 +461,15 @@ void Sema::ActOnPragmaAttributePush(Attr
         // declarations that receive the attribute.
         continue;
       }
-      if (isNegatedAttrMatcherSubRule(Rule.first))
+      if (isNegatedAttrMatcherSubRule(MatchRule))
         RulesToFirstSpecifiedNegatedSubRule.insert(
             std::make_pair(*ParentRule, Rule));
     }
     bool IgnoreNegatedSubRules = false;
     for (const auto &Rule : Rules) {
+      attr::SubjectMatchRule MatchRule = attr::SubjectMatchRule(Rule.first);
       Optional<attr::SubjectMatchRule> ParentRule =
-          getParentAttrMatcherRule(Rule.first);
+          getParentAttrMatcherRule(MatchRule);
       if (!ParentRule)
         continue;
       auto It = RulesToFirstSpecifiedNegatedSubRule.find(*ParentRule);
@@ -479,8 +480,9 @@ void Sema::ActOnPragmaAttributePush(Attr
             It->second.second.getBegin(),
             diag::
                 err_pragma_attribute_matcher_negated_subrule_contradicts_subrule)
-            << attr::getSubjectMatchRuleSpelling(It->second.first)
-            << attr::getSubjectMatchRuleSpelling(Rule.first) << Rule.second
+            << attr::getSubjectMatchRuleSpelling(
+                   attr::SubjectMatchRule(It->second.first))
+            << attr::getSubjectMatchRuleSpelling(MatchRule) << Rule.second
             << FixItHint::CreateRemoval(
                    replacementRangeForListElement(*this, It->second.second));
         // Keep going but ignore all of the negated sub-rules.
@@ -491,11 +493,11 @@ void Sema::ActOnPragmaAttributePush(Attr
 
     if (!IgnoreNegatedSubRules) {
       for (const auto &Rule : Rules)
-        SubjectMatchRules.push_back(Rule.first);
+        SubjectMatchRules.push_back(attr::SubjectMatchRule(Rule.first));
     } else {
       for (const auto &Rule : Rules) {
-        if (!isNegatedAttrMatcherSubRule(Rule.first))
-          SubjectMatchRules.push_back(Rule.first);
+        if (!isNegatedAttrMatcherSubRule(attr::SubjectMatchRule(Rule.first)))
+          SubjectMatchRules.push_back(attr::SubjectMatchRule(Rule.first));
       }
     }
     Rules.clear();
@@ -516,7 +518,7 @@ void Sema::ActOnPragmaAttributePush(Attr
         << Attribute.getName();
     SmallVector<attr::SubjectMatchRule, 2> ExtraRules;
     for (const auto &Rule : Rules) {
-      ExtraRules.push_back(Rule.first);
+      ExtraRules.push_back(attr::SubjectMatchRule(Rule.first));
       Diagnostic << FixItHint::CreateRemoval(
           replacementRangeForListElement(*this, Rule.second));
     }