llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Avi Kivity (avikivity) <details> <summary>Changes</summary> This reverts commit f5437c4d4f2e9920614d856158d9683d650acffa. That change moved population of Sema::FlagBitsCache out of IsValueInFlagEnum and into ActOnEnumBody, and turned the lookup into DenseMap::at(). ActOnEnumBody only runs for enum definitions that are parsed in the current translation unit, so an EnumDecl deserialized from a PCH never gets a cache entry and DenseMap::at() dereferences end(), crashing clang in builds without assertions. A switch over any enum with the flag_enum attribute that is defined in a PCH is enough to reproduce. The crash cannot be avoided by turning the switch diagnostics off: unlike DiagnoseAssignmentEnum, the switch path in ActOnFinishSwitchStmt has no Diags.isIgnored() guard, because it runs the analysis regardless in order to record the result in the AST. In practice this breaks any translation unit that instantiates std::basic_regex<char> while <regex> is in a PCH, since libstdc++ marks std::regex_constants::syntax_option_type as a flag enum and _Compiler<_TraitsT>::_S_validate switches over it. That became reachable with 44b33260a38c, which made clang honor the gnu:: spelling of the attribute, but the underlying defect is the one reverted here. Add a regression test for the PCH path. It checks the computed flag bits rather than just the absence of a crash, so that a silently empty cache entry fails the test instead of passing it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@<!-- -->anthropic.com> Fixes #<!-- -->223243 --- Full diff: https://github.com/llvm/llvm-project/pull/223244.diff 3 Files Affected: - (modified) clang/include/clang/Sema/Sema.h (+1-1) - (modified) clang/lib/Sema/SemaDecl.cpp (+11-15) - (added) clang/test/PCH/flag-enum.c (+34) ``````````diff diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 4ff4c669a6b70..d32eb3600ce54 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -3580,7 +3580,7 @@ class Sema final : public SemaBase { /// A cache of the flags available in enumerations with the flag_enum /// attribute. - llvm::DenseMap<const EnumDecl *, llvm::APInt> FlagBitsCache; + mutable llvm::DenseMap<const EnumDecl *, llvm::APInt> FlagBitsCache; /// A cache of enumerator values for enums checked by -Wassign-enum. llvm::DenseMap<const EnumDecl *, llvm::SmallVector<llvm::APSInt>> diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 5de5821fe263e..8f5a8e5133cc9 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -21116,7 +21116,17 @@ bool Sema::IsValueInFlagEnum(const EnumDecl *ED, const llvm::APInt &Val, assert(ED->isClosedFlag() && "looking for value in non-flag or open enum"); assert(ED->isCompleteDefinition() && "expected enum definition"); - llvm::APInt FlagBits = FlagBitsCache.at(ED); + auto R = FlagBitsCache.try_emplace(ED); + llvm::APInt &FlagBits = R.first->second; + + if (R.second) { + for (auto *E : ED->enumerators()) { + const auto &EVal = E->getInitVal(); + // Only single-bit enumerators introduce new flag values. + if (EVal.isPowerOf2()) + FlagBits = FlagBits.zext(EVal.getBitWidth()) | EVal; + } + } // 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 @@ -21327,20 +21337,6 @@ void Sema::ActOnEnumBody(SourceLocation EnumLoc, SourceRange BraceRange, CheckForDuplicateEnumValues(*this, Elements, Enum, EnumType); CheckForComparisonInEnumInitializer(*this, Enum); - if (Enum->hasAttr<FlagEnumAttr>()) { - auto R = FlagBitsCache.try_emplace(Enum); - llvm::APInt &FlagBits = R.first->second; - - if (R.second) { - for (auto *E : Enum->enumerators()) { - const auto &EVal = E->getInitVal(); - // Only single-bit enumerators introduce new flag values. - if (EVal.isPowerOf2()) - FlagBits = FlagBits.zext(EVal.getBitWidth()) | EVal; - } - } - } - if (Enum->isClosedFlag()) { for (Decl *D : Elements) { EnumConstantDecl *ECD = cast_or_null<EnumConstantDecl>(D); diff --git a/clang/test/PCH/flag-enum.c b/clang/test/PCH/flag-enum.c new file mode 100644 index 0000000000000..7454e0b90ed40 --- /dev/null +++ b/clang/test/PCH/flag-enum.c @@ -0,0 +1,34 @@ +// Check that an enum with the flag_enum attribute that is deserialized from a +// PCH does not crash Sema, and that its flag bits are still computed correctly. + +// RUN: %clang_cc1 -emit-pch -o %t %s +// RUN: %clang_cc1 -include-pch %t -fsyntax-only -verify %s + +#ifndef HEADER +#define HEADER + +enum __attribute__((flag_enum)) FlagEnum { + A = 0x1, + B = 0x2, + C = 0x4, +}; + +#else + +// The flag bits of FlagEnum are only known to Sema if they were recomputed +// after deserialization, so a wrong or missing cache entry shows up here as +// either a crash or a bogus diagnostic on the first two cases. +int f(enum FlagEnum e) { + switch (e) { + case A: + return 1; + case B | C: // no-warning + return 2; + case 0x8: // expected-warning {{case value not in enumerated type 'enum FlagEnum'}} + return 3; + default: + return 0; + } +} + +#endif `````````` </details> https://github.com/llvm/llvm-project/pull/223244 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
