In case you didn’t get an email from the failure because it was overshadowed by the previous error. This commit seems to break the green dragon bots: http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/12990/testReport/junit/Clang/Sema/attr_flag_enum_c/ <http://lab.llvm.org:8080/green/job/clang-stage1-cmake-RA-incremental_check/12990/testReport/junit/Clang/Sema/attr_flag_enum_c/>
Thanks Steven > On Sep 3, 2015, at 6:03 PM, Richard Smith via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > 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
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits