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