Merged to release_90 in r371058.
On Fri, Aug 23, 2019 at 4:19 PM Kristof Umann via cfe-commits <cfe-commits@lists.llvm.org> wrote: > > Author: szelethus > Date: Fri Aug 23 07:21:13 2019 > New Revision: 369760 > > URL: http://llvm.org/viewvc/llvm-project?rev=369760&view=rev > Log: > [analyzer] Avoid unnecessary enum range check on LValueToRValue casts > > Summary: EnumCastOutOfRangeChecker should not perform enum range checks on > LValueToRValue casts, since this type of cast does not actually change the > underlying type. Performing the unnecessary check actually triggered an > assertion failure deeper in EnumCastOutOfRange for certain input (which is > captured in the accompanying test code). > > Reviewers: #clang, Szelethus, gamesh411, NoQ > > Reviewed By: Szelethus, gamesh411, NoQ > > Subscribers: NoQ, gamesh411, xazax.hun, baloghadamsoftware, szepet, > a.sidorin, mikhail.ramalho, donat.nagy, dkrupp, Charusso, bjope, cfe-commits > > Tags: #clang > > Differential Revision: https://reviews.llvm.org/D66014 > > Added: > cfe/trunk/test/Analysis/enum-cast-out-of-range.c > Modified: > cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp > cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp > > Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp?rev=369760&r1=369759&r2=369760&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp > (original) > +++ cfe/trunk/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp Fri > Aug 23 07:21:13 2019 > @@ -91,6 +91,22 @@ void EnumCastOutOfRangeChecker::reportWa > > void EnumCastOutOfRangeChecker::checkPreStmt(const CastExpr *CE, > CheckerContext &C) const { > + > + // Only perform enum range check on casts where such checks are valid. For > + // all other cast kinds (where enum range checks are unnecessary or > invalid), > + // just return immediately. TODO: The set of casts whitelisted for enum > + // range checking may be incomplete. Better to add a missing cast kind to > + // enable a missing check than to generate false negatives and have to > remove > + // those later. > + switch (CE->getCastKind()) { > + case CK_IntegralCast: > + break; > + > + default: > + return; > + break; > + } > + > // Get the value of the expression to cast. > const llvm::Optional<DefinedOrUnknownSVal> ValueToCast = > C.getSVal(CE->getSubExpr()).getAs<DefinedOrUnknownSVal>(); > > Added: cfe/trunk/test/Analysis/enum-cast-out-of-range.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/enum-cast-out-of-range.c?rev=369760&view=auto > ============================================================================== > --- cfe/trunk/test/Analysis/enum-cast-out-of-range.c (added) > +++ cfe/trunk/test/Analysis/enum-cast-out-of-range.c Fri Aug 23 07:21:13 2019 > @@ -0,0 +1,34 @@ > +// RUN: %clang_analyze_cc1 \ > +// RUN: -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \ > +// RUN: -verify %s > + > +enum En_t { > + En_0 = -4, > + En_1, > + En_2 = 1, > + En_3, > + En_4 = 4 > +}; > + > +void unscopedUnspecifiedCStyle() { > + enum En_t Below = (enum En_t)(-5); // expected-warning {{not in the > valid range}} > + 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 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 PosVal3 = (enum En_t)(4); // OK. > + enum En_t Above = (enum En_t)(5); // expected-warning {{not in the > valid range}} > +} > + > +enum En_t unused; > +void unusedExpr() { > + // Following line is not something that EnumCastOutOfRangeChecker should > + // evaluate. Checker should either ignore this line or process it without > + // producing any warnings. However, compilation will (and should) still > + // generate a warning having nothing to do with this checker. > + unused; // expected-warning {{expression result unused}} > +} > > Modified: cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp?rev=369760&r1=369759&r2=369760&view=diff > ============================================================================== > --- cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp (original) > +++ cfe/trunk/test/Analysis/enum-cast-out-of-range.cpp Fri Aug 23 07:21:13 > 2019 > @@ -150,7 +150,15 @@ void scopedSpecifiedCStyle() { > scoped_specified_t InvalidAfterRangeEnd = (scoped_specified_t)(5); // > expected-warning {{The value provided to the cast expression is not in the > valid range of values for the enum}} > } > > -void rangeContstrained1(int input) { > +unscoped_unspecified_t unused; > +void unusedExpr() { > + // following line is not something that EnumCastOutOfRangeChecker should > evaluate. checker should either ignore this line > + // or process it without producing any warnings. However, compilation > will (and should) still generate a warning having > + // nothing to do with this checker. > + unused; // expected-warning {{expression result unused}} > +} > + > +void rangeConstrained1(int input) { > if (input > -5 && input < 5) > auto value = static_cast<scoped_specified_t>(input); // OK. Being > conservative, this is a possibly good value. > } > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
