r246830 - Fix a potential APInt memory leak when using __attribute__((flag_enum)), and
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 3 18:03:03 PDT 2015
Author: rsmith
Date: Thu Sep 3 20:03:03 2015
New Revision: 246830
URL: http://llvm.org/viewvc/llvm-project?rev=246830&view=rev
Log:
Fix a potential APInt memory leak when using __attribute__((flag_enum)), and
simplify the implementation a bit.
Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=246830&r1=246829&r2=246830&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Sep 3 20:03:03 2015
@@ -747,18 +747,6 @@ def FlagEnum : InheritableAttr {
let Subjects = SubjectList<[Enum]>;
let Documentation = [FlagEnumDocs];
let LangOpts = [COnly];
- let AdditionalMembers = [{
-private:
- llvm::APInt FlagBits;
-public:
- llvm::APInt &getFlagBits() {
- return FlagBits;
- }
-
- const llvm::APInt &getFlagBits() const {
- return FlagBits;
- }
-}];
}
def Flatten : InheritableAttr {
Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=246830&r1=246829&r2=246830&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Sep 3 20:03:03 2015
@@ -903,6 +903,10 @@ public:
/// for C++ records.
llvm::FoldingSet<SpecialMemberOverloadResult> SpecialMemberCache;
+ /// \brief A cache of the flags available in enumerations with the flag_bits
+ /// attribute.
+ mutable llvm::DenseMap<const EnumDecl*, llvm::APInt> FlagBitsCache;
+
/// \brief The kind of translation unit we are processing.
///
/// When we're processing a complete translation unit, Sema will perform
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=246830&r1=246829&r2=246830&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Sep 3 20:03:03 2015
@@ -14017,14 +14017,21 @@ static void CheckForDuplicateEnumValues(
bool
Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val,
bool AllowMask) const {
- FlagEnumAttr *FEAttr = ED->getAttr<FlagEnumAttr>();
- assert(FEAttr && "looking for value in non-flag enum");
+ assert(ED->hasAttr<FlagEnumAttr>() && "looking for value in non-flag enum");
- llvm::APInt FlagMask = ~FEAttr->getFlagBits();
- unsigned Width = FlagMask.getBitWidth();
+ auto R = FlagBitsCache.insert(std::make_pair(ED, llvm::APInt()));
+ llvm::APInt &FlagBits = R.first->second;
- // We will try a zero-extended value for the regular check first.
- llvm::APInt ExtVal = Val.zextOrSelf(Width);
+ if (R.second) {
+ for (auto *E : ED->enumerators()) {
+ const auto &Val = E->getInitVal();
+ // Only single-bit enumerators introduce new flag values.
+ if (Val.isPowerOf2())
+ FlagBits = FlagBits.zextOrSelf(Val.getBitWidth()) | Val;
+ }
+ }
+
+ llvm::APInt FlagMask = ~FlagBits.zextOrTrunc(Val.getBitWidth());
// A value is in a flag enum if either its bits are a subset of the enum's
// flag bits (the first condition) or we are allowing masks and the same is
@@ -14034,26 +14041,10 @@ Sema::IsValueInFlagEnum(const EnumDecl *
// While it's true that any value could be used as a mask, the assumption is
// that a mask will have all of the insignificant bits set. Anything else is
// likely a logic error.
- if (!(FlagMask & ExtVal))
+ if (!(FlagMask & Val) ||
+ (AllowMask && !(FlagMask & ~Val)))
return true;
- if (AllowMask) {
- // Try a one-extended value instead. This can happen if the enum is wider
- // than the constant used, in C with extensions to allow for wider enums.
- // The mask will still have the correct behaviour, so we give the user the
- // benefit of the doubt.
- //
- // FIXME: This heuristic can cause weird results if the enum was extended
- // to a larger type and is signed, because then bit-masks of smaller types
- // that get extended will fall out of range (e.g. ~0x1u). We currently don't
- // detect that case and will get a false positive for it. In most cases,
- // though, it can be fixed by making it a signed type (e.g. ~0x1), so it may
- // be fine just to accept this as a warning.
- ExtVal |= llvm::APInt::getHighBitsSet(Width, Width - Val.getBitWidth());
- if (!(FlagMask & ~ExtVal))
- return true;
- }
-
return false;
}
@@ -14208,13 +14199,8 @@ void Sema::ActOnEnumBody(SourceLocation
}
}
- FlagEnumAttr *FEAttr = Enum->getAttr<FlagEnumAttr>();
- if (FEAttr)
- FEAttr->getFlagBits() = llvm::APInt(BestWidth, 0);
-
// Loop over all of the enumerator constants, changing their types to match
- // the type of the enum if needed. If we have a flag type, we also prepare the
- // FlagBits cache.
+ // the type of the enum if needed.
for (auto *D : Elements) {
auto *ECD = cast_or_null<EnumConstantDecl>(D);
if (!ECD) continue; // Already issued a diagnostic.
@@ -14246,7 +14232,7 @@ void Sema::ActOnEnumBody(SourceLocation
// enum-specifier, each enumerator has the type of its
// enumeration.
ECD->setType(EnumType);
- goto flagbits;
+ continue;
} else {
NewTy = BestType;
NewWidth = BestWidth;
@@ -14273,32 +14259,21 @@ void Sema::ActOnEnumBody(SourceLocation
ECD->setType(EnumType);
else
ECD->setType(NewTy);
-
-flagbits:
- // Check to see if we have a constant with exactly one bit set. Note that x
- // & (x - 1) will be nonzero if and only if x has more than one bit set.
- if (FEAttr) {
- llvm::APInt ExtVal = InitVal.zextOrSelf(BestWidth);
- if (ExtVal != 0 && !(ExtVal & (ExtVal - 1))) {
- FEAttr->getFlagBits() |= ExtVal;
- }
- }
}
- if (FEAttr) {
+ if (Enum->hasAttr<FlagEnumAttr>()) {
for (Decl *D : Elements) {
EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(D);
if (!ECD) continue; // Already issued a diagnostic.
llvm::APSInt InitVal = ECD->getInitVal();
- if (InitVal != 0 && !IsValueInFlagEnum(Enum, InitVal, true))
+ if (InitVal != 0 && !InitVal.isPowerOf2() &&
+ !IsValueInFlagEnum(Enum, InitVal, true))
Diag(ECD->getLocation(), diag::warn_flag_enum_constant_out_of_range)
<< ECD << Enum;
}
}
-
-
Enum->completeDefinition(BestType, BestPromotionType,
NumPositiveBits, NumNegativeBits);
Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=246830&r1=246829&r2=246830&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
+++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Sep 3 20:03:03 2015
@@ -698,8 +698,6 @@ static bool ShouldDiagnoseSwitchCaseNotI
EnumValsTy::iterator &EI,
EnumValsTy::iterator &EIEnd,
const llvm::APSInt &Val) {
- bool FlagType = ED->hasAttr<FlagEnumAttr>();
-
if (const DeclRefExpr *DRE =
dyn_cast<DeclRefExpr>(CaseExpr->IgnoreParenImpCasts())) {
if (const VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
@@ -711,7 +709,7 @@ static bool ShouldDiagnoseSwitchCaseNotI
}
}
- if (FlagType) {
+ if (ED->hasAttr<FlagEnumAttr>()) {
return !S.IsValueInFlagEnum(ED, Val, false);
} else {
while (EI != EIEnd && EI->first < Val)
More information about the cfe-commits
mailing list