Author: rtrieu Date: Fri Oct 18 17:57:23 2019 New Revision: 375318 URL: http://llvm.org/viewvc/llvm-project?rev=375318&view=rev Log: New tautological warning for bitwise-or with non-zero constant always true.
Taking a value and the bitwise-or it with a non-zero constant will always result in a non-zero value. In a boolean context, this is always true. if (x | 0x4) {} // always true, intended '&' This patch creates a new warning group -Wtautological-bitwise-compare for this warning. It also moves in the existing tautological bitwise comparisons into this group. A few other changes were needed to the CFGBuilder so that all bool contexts would be checked. The warnings in -Wtautological-bitwise-compare will be off by default due to using the CFG. Fixes: https://bugs.llvm.org/show_bug.cgi?id=42666 Differential Revision: https://reviews.llvm.org/D66046 Added: cfe/trunk/test/SemaCXX/warn-bitwise-compare.cpp Modified: cfe/trunk/docs/ReleaseNotes.rst cfe/trunk/include/clang/Analysis/CFG.h cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp cfe/trunk/test/Sema/warn-bitwise-compare.c Modified: cfe/trunk/docs/ReleaseNotes.rst URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/docs/ReleaseNotes.rst (original) +++ cfe/trunk/docs/ReleaseNotes.rst Fri Oct 18 17:57:23 2019 @@ -56,6 +56,11 @@ Improvements to Clang's diagnostics - -Wtautological-compare for self comparisons and -Wtautological-overlap-compare will now look through member and array access to determine if two operand expressions are the same. +- -Wtautological-bitwise-compare is a new warning group. This group has the + current warning which diagnoses the tautological comparison of a bitwise + operation and a constant. The group also has the new warning which diagnoses + when a bitwise-or with a non-negative value is converted to a bool, since + that bool will always be true. Non-comprehensive list of changes in this release ------------------------------------------------- Modified: cfe/trunk/include/clang/Analysis/CFG.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/include/clang/Analysis/CFG.h (original) +++ cfe/trunk/include/clang/Analysis/CFG.h Fri Oct 18 17:57:23 2019 @@ -1213,6 +1213,7 @@ public: virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} virtual void compareBitwiseEquality(const BinaryOperator *B, bool isAlwaysTrue) {} + virtual void compareBitwiseOr(const BinaryOperator *B) {} }; /// Represents a source-level, intra-procedural CFG that represents the Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Oct 18 17:57:23 2019 @@ -516,12 +516,14 @@ def TautologicalConstantCompare : DiagGr [TautologicalOutOfRangeCompare]>; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; +def TautologicalBitwiseCompare : DiagGroup<"tautological-bitwise-compare">; def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">; def TautologicalObjCBoolCompare : DiagGroup<"tautological-objc-bool-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalConstantCompare, TautologicalPointerCompare, TautologicalOverlapCompare, + TautologicalBitwiseCompare, TautologicalUndefinedCompare, TautologicalObjCBoolCompare]>; def HeaderHygiene : DiagGroup<"header-hygiene">; Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Oct 18 17:57:23 2019 @@ -8332,7 +8332,10 @@ def warn_comparison_always : Warning< InGroup<TautologicalCompare>; def warn_comparison_bitwise_always : Warning< "bitwise comparison always evaluates to %select{false|true}0">, - InGroup<TautologicalCompare>; + InGroup<TautologicalBitwiseCompare>, DefaultIgnore; +def warn_comparison_bitwise_or : Warning< + "bitwise or with non-zero value always evaluates to true">, + InGroup<TautologicalBitwiseCompare>, DefaultIgnore; def warn_tautological_overlap_comparison : Warning< "overlapping comparisons always evaluate to %select{false|true}0">, InGroup<TautologicalOverlapCompare>, DefaultIgnore; Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/lib/Analysis/CFG.cpp (original) +++ cfe/trunk/lib/Analysis/CFG.cpp Fri Oct 18 17:57:23 2019 @@ -1139,6 +1139,31 @@ private: return {}; } + /// A bitwise-or with a non-zero constant always evaluates to true. + TryResult checkIncorrectBitwiseOrOperator(const BinaryOperator *B) { + const Expr *LHSConstant = + tryTransformToIntOrEnumConstant(B->getLHS()->IgnoreParenImpCasts()); + const Expr *RHSConstant = + tryTransformToIntOrEnumConstant(B->getRHS()->IgnoreParenImpCasts()); + + if ((LHSConstant && RHSConstant) || (!LHSConstant && !RHSConstant)) + return {}; + + const Expr *Constant = LHSConstant ? LHSConstant : RHSConstant; + + Expr::EvalResult Result; + if (!Constant->EvaluateAsInt(Result, *Context)) + return {}; + + if (Result.Val.getInt() == 0) + return {}; + + if (BuildOpts.Observer) + BuildOpts.Observer->compareBitwiseOr(B); + + return TryResult(true); + } + /// Try and evaluate an expression to an integer constant. bool tryEvaluate(Expr *S, Expr::EvalResult &outResult) { if (!BuildOpts.PruneTriviallyFalseEdges) @@ -1156,7 +1181,7 @@ private: return {}; if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(S)) { - if (Bop->isLogicalOp()) { + if (Bop->isLogicalOp() || Bop->isEqualityOp()) { // Check the cache first. CachedBoolEvalsTy::iterator I = CachedBoolEvals.find(S); if (I != CachedBoolEvals.end()) @@ -1240,6 +1265,10 @@ private: TryResult BopRes = checkIncorrectRelationalOperator(Bop); if (BopRes.isKnown()) return BopRes.isTrue(); + } else if (Bop->getOpcode() == BO_Or) { + TryResult BopRes = checkIncorrectBitwiseOrOperator(Bop); + if (BopRes.isKnown()) + return BopRes.isTrue(); } } @@ -2340,6 +2369,9 @@ CFGBlock *CFGBuilder::VisitUnaryOperator appendStmt(Block, U); } + if (U->getOpcode() == UO_LNot) + tryEvaluateBool(U->getSubExpr()->IgnoreParens()); + return Visit(U->getSubExpr(), AddStmtChoice()); } @@ -2474,6 +2506,9 @@ CFGBlock *CFGBuilder::VisitBinaryOperato appendStmt(Block, B); } + if (B->isEqualityOp() || B->isRelationalOp()) + tryEvaluateBool(B); + CFGBlock *RBlock = Visit(B->getRHS()); CFGBlock *LBlock = Visit(B->getLHS()); // If visiting RHS causes us to finish 'Block', e.g. the RHS is a StmtExpr @@ -4527,6 +4562,10 @@ CFGBlock *CFGBuilder::VisitImplicitCastE autoCreateBlock(); appendStmt(Block, E); } + + if (E->getCastKind() == CK_IntegralToBoolean) + tryEvaluateBool(E->getSubExpr()->IgnoreParens()); + return Visit(E->getSubExpr(), AddStmtChoice()); } Modified: cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp (original) +++ cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp Fri Oct 18 17:57:23 2019 @@ -159,6 +159,20 @@ public: S.Diag(B->getExprLoc(), diag::warn_comparison_bitwise_always) << DiagRange << isAlwaysTrue; } + + void compareBitwiseOr(const BinaryOperator *B) override { + if (HasMacroID(B)) + return; + + SourceRange DiagRange = B->getSourceRange(); + S.Diag(B->getExprLoc(), diag::warn_comparison_bitwise_or) << DiagRange; + } + + static bool hasActiveDiagnostics(DiagnosticsEngine &Diags, + SourceLocation Loc) { + return !Diags.isIgnored(diag::warn_tautological_overlap_comparison, Loc) || + !Diags.isIgnored(diag::warn_comparison_bitwise_or, Loc); + } }; } // anonymous namespace @@ -2070,10 +2084,9 @@ AnalysisBasedWarnings::IssueWarnings(sem .setAlwaysAdd(Stmt::AttributedStmtClass); } - // Install the logical handler for -Wtautological-overlap-compare + // Install the logical handler. llvm::Optional<LogicalErrorHandler> LEH; - if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison, - D->getBeginLoc())) { + if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { LEH.emplace(S); AC.getCFGBuildOptions().Observer = &*LEH; } @@ -2222,9 +2235,8 @@ AnalysisBasedWarnings::IssueWarnings(sem checkThrowInNonThrowingFunc(S, FD, AC); // If none of the previous checks caused a CFG build, trigger one here - // for -Wtautological-overlap-compare - if (!Diags.isIgnored(diag::warn_tautological_overlap_comparison, - D->getBeginLoc())) { + // for the logical error handler. + if (LogicalErrorHandler::hasActiveDiagnostics(Diags, D->getBeginLoc())) { AC.getCFG(); } Modified: cfe/trunk/test/Sema/warn-bitwise-compare.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-bitwise-compare.c?rev=375318&r1=375317&r2=375318&view=diff ============================================================================== --- cfe/trunk/test/Sema/warn-bitwise-compare.c (original) +++ cfe/trunk/test/Sema/warn-bitwise-compare.c Fri Oct 18 17:57:23 2019 @@ -1,7 +1,12 @@ -// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-compare %s +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s #define mydefine 2 +enum { + ZERO, + ONE, +}; + void f(int x) { if ((8 & x) == 3) {} // expected-warning {{bitwise comparison always evaluates to false}} if ((x & 8) == 4) {} // expected-warning {{bitwise comparison always evaluates to false}} @@ -13,6 +18,9 @@ void f(int x) { if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}} if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}} + if (!!((8 & x) == 3)) {} // expected-warning {{bitwise comparison always evaluates to false}} + int y = ((8 & x) == 3) ? 1 : 2; // expected-warning {{bitwise comparison always evaluates to false}} + if ((x & 8) == 8) {} if ((x & 8) != 8) {} if ((x | 4) == 4) {} @@ -26,3 +34,14 @@ void f(int x) { if ((x & mydefine) == 8) {} if ((x | mydefine) == 4) {} } + +void g(int x) { + if (x | 5) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + if (5 | x) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + if (!((x | 5))) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + + if (x | -1) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + if (x | ONE) {} // expected-warning {{bitwise or with non-zero value always evaluates to true}} + + if (x | ZERO) {} +} Added: cfe/trunk/test/SemaCXX/warn-bitwise-compare.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-bitwise-compare.cpp?rev=375318&view=auto ============================================================================== --- cfe/trunk/test/SemaCXX/warn-bitwise-compare.cpp (added) +++ cfe/trunk/test/SemaCXX/warn-bitwise-compare.cpp Fri Oct 18 17:57:23 2019 @@ -0,0 +1,12 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wtautological-bitwise-compare %s + +void test(int x) { + bool b1 = (8 & x) == 3; + // expected-warning@-1 {{bitwise comparison always evaluates to false}} + bool b2 = x | 5; + // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}} + bool b3 = (x | 5); + // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}} + bool b4 = !!(x | 5); + // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}} +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits