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.

llvm-svn: 300591
This commit is contained in:
Alex Lorenz 2017-04-18 20:54:23 +00:00
parent 7879598d75
commit 26b476502c
2 changed files with 15 additions and 27 deletions

View File

@ -24,23 +24,9 @@ enum SubjectMatchRule {
const char *getSubjectMatchRuleSpelling(SubjectMatchRule Rule); const char *getSubjectMatchRuleSpelling(SubjectMatchRule Rule);
using ParsedSubjectMatchRuleSet = llvm::DenseMap<SubjectMatchRule, SourceRange>; using ParsedSubjectMatchRuleSet = llvm::DenseMap<int, SourceRange>;
} // end namespace attr } // end namespace attr
} // end namespace clang } // 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 #endif

View File

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