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