Skip to content

Conversation

@gamesh411
Copy link
Contributor

EnumCastOutOfRange checker now reports the name of the enum in the warning message. Additionally, a note-tag is placed to highlight the location of the declaration.

@gamesh411 gamesh411 requested a review from NagyDonat October 4, 2023 08:41
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html labels Oct 4, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 4, 2023

@llvm/pr-subscribers-clang-static-analyzer-1

@llvm/pr-subscribers-clang

Changes

EnumCastOutOfRange checker now reports the name of the enum in the warning message. Additionally, a note-tag is placed to highlight the location of the declaration.


Patch is 30.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68191.diff

3 Files Affected:

  • (modified) clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp (+24-9)
  • (modified) clang/test/Analysis/enum-cast-out-of-range.c (+7-6)
  • (modified) clang/test/Analysis/enum-cast-out-of-range.cpp (+56-52)
diff --git a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp index 89be6a47250a245..6163f7a23804091 100644 --- a/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp +++ b/clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp @@ -59,7 +59,7 @@ class ConstraintBasedEQEvaluator { // value can be matching. class EnumCastOutOfRangeChecker : public Checker<check::PreStmt<CastExpr>> { mutable std::unique_ptr<BugType> EnumValueCastOutOfRange; - void reportWarning(CheckerContext &C) const; + void reportWarning(CheckerContext &C, const EnumDecl *E) const; public: void checkPreStmt(const CastExpr *CE, CheckerContext &C) const; @@ -72,21 +72,36 @@ EnumValueVector getDeclValuesForEnum(const EnumDecl *ED) { EnumValueVector DeclValues( std::distance(ED->enumerator_begin(), ED->enumerator_end())); llvm::transform(ED->enumerators(), DeclValues.begin(), - [](const EnumConstantDecl *D) { return D->getInitVal(); }); + [](const EnumConstantDecl *D) { return D->getInitVal(); }); return DeclValues; } } // namespace -void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C) const { +void EnumCastOutOfRangeChecker::reportWarning(CheckerContext &C, + const EnumDecl *E) const { + assert(E && "valid EnumDecl* is expected"); if (const ExplodedNode *N = C.generateNonFatalErrorNode()) { if (!EnumValueCastOutOfRange) EnumValueCastOutOfRange.reset( new BugType(this, "Enum cast out of range")); - constexpr llvm::StringLiteral Msg = - "The value provided to the cast expression is not in the valid range" - " of values for the enum"; - C.emitReport(std::make_unique<PathSensitiveBugReport>( - *EnumValueCastOutOfRange, Msg, N)); + + llvm::SmallString<128> Msg{"The value provided to the cast expression is " + "not in the valid range of values for "}; + StringRef EnumName{E->getName()}; + if (EnumName.empty()) { + Msg += "the enum"; + } else { + Msg += '\''; + Msg += EnumName; + Msg += '\''; + } + + auto BR = std::make_unique<PathSensitiveBugReport>(*EnumValueCastOutOfRange, + Msg, N); + BR->addNote("enum declared here", + PathDiagnosticLocation::create(E, C.getSourceManager()), + {E->getSourceRange()}); + C.emitReport(std::move(BR)); } } @@ -144,7 +159,7 @@ void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE, // If there is no value that can possibly match any of the enum values, then // warn. if (!PossibleValueMatch) - reportWarning(C); + reportWarning(C, ED); } void ento::registerEnumCastOutOfRangeChecker(CheckerManager &mgr) { diff --git a/clang/test/Analysis/enum-cast-out-of-range.c b/clang/test/Analysis/enum-cast-out-of-range.c index 3282cba653d7125..6d3afa3fcf9885f 100644 --- a/clang/test/Analysis/enum-cast-out-of-range.c +++ b/clang/test/Analysis/enum-cast-out-of-range.c @@ -2,6 +2,7 @@ // RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \ // RUN: -verify %s +// expected-note@+1 6 {{enum declared here}} enum En_t { En_0 = -4, En_1, @@ -11,17 +12,17 @@ enum En_t { }; void unscopedUnspecifiedCStyle(void) { - enum En_t Below = (enum En_t)(-5); // expected-warning {{not in the valid range}} + enum En_t Below = (enum En_t)(-5); // expected-warning {{not in the valid range of values for 'En_t'}} enum En_t NegVal1 = (enum En_t)(-4); // OK. enum En_t NegVal2 = (enum En_t)(-3); // OK. - enum En_t InRange1 = (enum En_t)(-2); // expected-warning {{not in the valid range}} - enum En_t InRange2 = (enum En_t)(-1); // expected-warning {{not in the valid range}} - enum En_t InRange3 = (enum En_t)(0); // expected-warning {{not in the valid range}} + enum En_t InRange1 = (enum En_t)(-2); // expected-warning {{not in the valid range of values for 'En_t'}} + enum En_t InRange2 = (enum En_t)(-1); // expected-warning {{not in the valid range of values for 'En_t'}} + enum En_t InRange3 = (enum En_t)(0); // expected-warning {{not in the valid range of values for 'En_t'}} enum En_t PosVal1 = (enum En_t)(1); // OK. enum En_t PosVal2 = (enum En_t)(2); // OK. - enum En_t InRange4 = (enum En_t)(3); // expected-warning {{not in the valid range}} + enum En_t InRange4 = (enum En_t)(3); // expected-warning {{not in the valid range of values for 'En_t'}} enum En_t PosVal3 = (enum En_t)(4); // OK. - enum En_t Above = (enum En_t)(5); // expected-warning {{not in the valid range}} + enum En_t Above = (enum En_t)(5); // expected-warning {{not in the valid range of values for 'En_t'}} } enum En_t unused; diff --git a/clang/test/Analysis/enum-cast-out-of-range.cpp b/clang/test/Analysis/enum-cast-out-of-range.cpp index abc1431e5be140f..0eb740664ecdc7c 100644 --- a/clang/test/Analysis/enum-cast-out-of-range.cpp +++ b/clang/test/Analysis/enum-cast-out-of-range.cpp @@ -2,6 +2,7 @@ // RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \ // RUN: -std=c++11 -verify %s +// expected-note@+1 + {{enum declared here}} enum unscoped_unspecified_t { unscoped_unspecified_0 = -4, unscoped_unspecified_1, @@ -10,6 +11,7 @@ enum unscoped_unspecified_t { unscoped_unspecified_4 = 4 }; +// expected-note@+1 + {{enum declared here}} enum unscoped_specified_t : int { unscoped_specified_0 = -4, unscoped_specified_1, @@ -18,6 +20,7 @@ enum unscoped_specified_t : int { unscoped_specified_4 = 4 }; +// expected-note@+1 + {{enum declared here}} enum class scoped_unspecified_t { scoped_unspecified_0 = -4, scoped_unspecified_1, @@ -26,6 +29,7 @@ enum class scoped_unspecified_t { scoped_unspecified_4 = 4 }; +// expected-note@+1 + {{enum declared here}} enum class scoped_specified_t : int { scoped_specified_0 = -4, scoped_specified_1, @@ -39,115 +43,115 @@ struct S { }; void unscopedUnspecified() { - unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast<unscoped_unspecified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidBeforeRangeBegin = static_cast<unscoped_unspecified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidNegativeValue1 = static_cast<unscoped_unspecified_t>(-4); // OK. unscoped_unspecified_t ValidNegativeValue2 = static_cast<unscoped_unspecified_t>(-3); // OK. - unscoped_unspecified_t InvalidInsideRange1 = static_cast<unscoped_unspecified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_unspecified_t InvalidInsideRange2 = static_cast<unscoped_unspecified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_unspecified_t InvalidInsideRange3 = static_cast<unscoped_unspecified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidInsideRange1 = static_cast<unscoped_unspecified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} + unscoped_unspecified_t InvalidInsideRange2 = static_cast<unscoped_unspecified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} + unscoped_unspecified_t InvalidInsideRange3 = static_cast<unscoped_unspecified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidPositiveValue1 = static_cast<unscoped_unspecified_t>(1); // OK. unscoped_unspecified_t ValidPositiveValue2 = static_cast<unscoped_unspecified_t>(2); // OK. - unscoped_unspecified_t InvalidInsideRange4 = static_cast<unscoped_unspecified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidInsideRange4 = static_cast<unscoped_unspecified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidPositiveValue3 = static_cast<unscoped_unspecified_t>(4); // OK. - unscoped_unspecified_t InvalidAfterRangeEnd = static_cast<unscoped_unspecified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidAfterRangeEnd = static_cast<unscoped_unspecified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} } void unscopedSpecified() { - unscoped_specified_t InvalidBeforeRangeBegin = static_cast<unscoped_specified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_specified_t InvalidBeforeRangeBegin = static_cast<unscoped_specified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} unscoped_specified_t ValidNegativeValue1 = static_cast<unscoped_specified_t>(-4); // OK. unscoped_specified_t ValidNegativeValue2 = static_cast<unscoped_specified_t>(-3); // OK. - unscoped_specified_t InvalidInsideRange1 = static_cast<unscoped_specified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_specified_t InvalidInsideRange2 = static_cast<unscoped_specified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_specified_t InvalidInsideRange3 = static_cast<unscoped_specified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_specified_t InvalidInsideRange1 = static_cast<unscoped_specified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} + unscoped_specified_t InvalidInsideRange2 = static_cast<unscoped_specified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} + unscoped_specified_t InvalidInsideRange3 = static_cast<unscoped_specified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} unscoped_specified_t ValidPositiveValue1 = static_cast<unscoped_specified_t>(1); // OK. unscoped_specified_t ValidPositiveValue2 = static_cast<unscoped_specified_t>(2); // OK. - unscoped_specified_t InvalidInsideRange4 = static_cast<unscoped_specified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_specified_t InvalidInsideRange4 = static_cast<unscoped_specified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} unscoped_specified_t ValidPositiveValue3 = static_cast<unscoped_specified_t>(4); // OK. - unscoped_specified_t InvalidAfterRangeEnd = static_cast<unscoped_specified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_specified_t InvalidAfterRangeEnd = static_cast<unscoped_specified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_specified_t'}} } void scopedUnspecified() { - scoped_unspecified_t InvalidBeforeRangeBegin = static_cast<scoped_unspecified_t>(-5); // expected-warning{{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_unspecified_t InvalidBeforeRangeBegin = static_cast<scoped_unspecified_t>(-5); // expected-warning{{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} scoped_unspecified_t ValidNegativeValue1 = static_cast<scoped_unspecified_t>(-4); // OK. scoped_unspecified_t ValidNegativeValue2 = static_cast<scoped_unspecified_t>(-3); // OK. - scoped_unspecified_t InvalidInsideRange1 = static_cast<scoped_unspecified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - scoped_unspecified_t InvalidInsideRange2 = static_cast<scoped_unspecified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - scoped_unspecified_t InvalidInsideRange3 = static_cast<scoped_unspecified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_unspecified_t InvalidInsideRange1 = static_cast<scoped_unspecified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} + scoped_unspecified_t InvalidInsideRange2 = static_cast<scoped_unspecified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} + scoped_unspecified_t InvalidInsideRange3 = static_cast<scoped_unspecified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} scoped_unspecified_t ValidPositiveValue1 = static_cast<scoped_unspecified_t>(1); // OK. scoped_unspecified_t ValidPositiveValue2 = static_cast<scoped_unspecified_t>(2); // OK. - scoped_unspecified_t InvalidInsideRange4 = static_cast<scoped_unspecified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_unspecified_t InvalidInsideRange4 = static_cast<scoped_unspecified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} scoped_unspecified_t ValidPositiveValue3 = static_cast<scoped_unspecified_t>(4); // OK. - scoped_unspecified_t InvalidAfterRangeEnd = static_cast<scoped_unspecified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_unspecified_t InvalidAfterRangeEnd = static_cast<scoped_unspecified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_unspecified_t'}} } void scopedSpecified() { - scoped_specified_t InvalidBeforeRangeBegin = static_cast<scoped_specified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_specified_t InvalidBeforeRangeBegin = static_cast<scoped_specified_t>(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} scoped_specified_t ValidNegativeValue1 = static_cast<scoped_specified_t>(-4); // OK. scoped_specified_t ValidNegativeValue2 = static_cast<scoped_specified_t>(-3); // OK. - scoped_specified_t InvalidInsideRange1 = static_cast<scoped_specified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - scoped_specified_t InvalidInsideRange2 = static_cast<scoped_specified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - scoped_specified_t InvalidInsideRange3 = static_cast<scoped_specified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_specified_t InvalidInsideRange1 = static_cast<scoped_specified_t>(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} + scoped_specified_t InvalidInsideRange2 = static_cast<scoped_specified_t>(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} + scoped_specified_t InvalidInsideRange3 = static_cast<scoped_specified_t>(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} scoped_specified_t ValidPositiveValue1 = static_cast<scoped_specified_t>(1); // OK. scoped_specified_t ValidPositiveValue2 = static_cast<scoped_specified_t>(2); // OK. - scoped_specified_t InvalidInsideRange4 = static_cast<scoped_specified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_specified_t InvalidInsideRange4 = static_cast<scoped_specified_t>(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} scoped_specified_t ValidPositiveValue3 = static_cast<scoped_specified_t>(4); // OK. - scoped_specified_t InvalidAfterRangeEnd = static_cast<scoped_specified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + scoped_specified_t InvalidAfterRangeEnd = static_cast<scoped_specified_t>(5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'scoped_specified_t'}} } void unscopedUnspecifiedCStyle() { - unscoped_unspecified_t InvalidBeforeRangeBegin = (unscoped_unspecified_t)(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidBeforeRangeBegin = (unscoped_unspecified_t)(-5); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidNegativeValue1 = (unscoped_unspecified_t)(-4); // OK. unscoped_unspecified_t ValidNegativeValue2 = (unscoped_unspecified_t)(-3); // OK. - unscoped_unspecified_t InvalidInsideRange1 = (unscoped_unspecified_t)(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_unspecified_t InvalidInsideRange2 = (unscoped_unspecified_t)(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} - unscoped_unspecified_t InvalidInsideRange3 = (unscoped_unspecified_t)(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidInsideRange1 = (unscoped_unspecified_t)(-2); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} + unscoped_unspecified_t InvalidInsideRange2 = (unscoped_unspecified_t)(-1); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} + unscoped_unspecified_t InvalidInsideRange3 = (unscoped_unspecified_t)(0); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidPositiveValue1 = (unscoped_unspecified_t)(1); // OK. unscoped_unspecified_t ValidPositiveValue2 = (unscoped_unspecified_t)(2); // OK. - unscoped_unspecified_t InvalidInsideRange4 = (unscoped_unspecified_t)(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for the enum}} + unscoped_unspecified_t InvalidInsideRange4 = (unscoped_unspecified_t)(3); // expected-warning {{The value provided to the cast expression is not in the valid range of values for 'unscoped_unspecified_t'}} unscoped_unspecified_t ValidPositiveValue3 = (unscoped_unspecified_t)(4); // OK. -... [truncated] 
@Xazax-hun
Copy link
Collaborator

Overall looks good, but I think there are no tests that would showcase what trackexpression is actually doing.

Endre Fülöp added 3 commits October 26, 2023 10:33
EnumCastOutOfRange checker now reports the name of the enum in the warning message. Additionally, a note-tag is placed to highlight the location of the declaration.
track subexpression instead of the whole cast
@gamesh411 gamesh411 force-pushed the extend-enumcast-diagnostics branch from afe843b to 4d12939 Compare October 26, 2023 08:44
@gamesh411
Copy link
Contributor Author

@Xazax-hun Thanks for the review!
I have modified the tracking a bit, because seeing the other usecases, and also just thinking through, the subexpression of the cast is what we are interested in upstream in the bugpath.
I have added a very minimal test as well, and rebased.

"not in the valid range of values for "};
StringRef EnumName{E->getName()};
if (EnumName.empty()) {
Msg += "the enum";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a testcase that shows an example where this branch is used? (Is this for the C-style typedef enum {...} foo_t; declarations?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:dataflow Clang Dataflow Analysis framework - https://clang.llvm.org/docs/DataFlowAnalysisIntro.html clang:static analyzer clang Clang issues not falling into any other category

4 participants