From 6e613778717c7bfca8f9aabd5fc43ee0926e6098 Mon Sep 17 00:00:00 2001 From: Alex Lorenz Date: Tue, 18 Apr 2017 20:54:23 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@300591 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/AttrSubjectMatchRules.h | 16 +------------ lib/Sema/SemaAttr.cpp | 26 +++++++++++---------- 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/include/clang/Basic/AttrSubjectMatchRules.h b/include/clang/Basic/AttrSubjectMatchRules.h index 955f495ad6..4c88adf57f 100644 --- a/include/clang/Basic/AttrSubjectMatchRules.h +++ b/include/clang/Basic/AttrSubjectMatchRules.h @@ -24,23 +24,9 @@ enum SubjectMatchRule { const char *getSubjectMatchRuleSpelling(SubjectMatchRule Rule); -using ParsedSubjectMatchRuleSet = llvm::DenseMap; +using ParsedSubjectMatchRuleSet = llvm::DenseMap; } // end namespace attr } // end namespace clang -namespace llvm { - -template <> -struct DenseMapInfo : DenseMapInfo { - static inline clang::attr::SubjectMatchRule getEmptyKey() { - return (clang::attr::SubjectMatchRule)DenseMapInfo::getEmptyKey(); - } - static inline clang::attr::SubjectMatchRule getTombstoneKey() { - return (clang::attr::SubjectMatchRule)DenseMapInfo::getTombstoneKey(); - } -}; - -} // end namespace llvm - #endif diff --git a/lib/Sema/SemaAttr.cpp b/lib/Sema/SemaAttr.cpp index dfc5d6cd01..76ca65373d 100644 --- a/lib/Sema/SemaAttr.cpp +++ b/lib/Sema/SemaAttr.cpp @@ -440,12 +440,12 @@ void Sema::ActOnPragmaAttributePush(AttributeList &Attribute, // 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, 2> + llvm::SmallDenseMap, 2> RulesToFirstSpecifiedNegatedSubRule; for (const auto &Rule : Rules) { + attr::SubjectMatchRule MatchRule = attr::SubjectMatchRule(Rule.first); Optional ParentRule = - getParentAttrMatcherRule(Rule.first); + getParentAttrMatcherRule(MatchRule); if (!ParentRule) continue; auto It = Rules.find(*ParentRule); @@ -453,7 +453,7 @@ void Sema::ActOnPragmaAttributePush(AttributeList &Attribute, // 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(AttributeList &Attribute, // 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 ParentRule = - getParentAttrMatcherRule(Rule.first); + getParentAttrMatcherRule(MatchRule); if (!ParentRule) continue; auto It = RulesToFirstSpecifiedNegatedSubRule.find(*ParentRule); @@ -479,8 +480,9 @@ void Sema::ActOnPragmaAttributePush(AttributeList &Attribute, 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(AttributeList &Attribute, 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(AttributeList &Attribute, << Attribute.getName(); SmallVector 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)); } -- 2.40.0