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)); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
