From 924ba5f40d93e00d880f7ab3f28be83fa1cefe01 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Mon, 19 Dec 2022 12:49:43 +0100 Subject: [PATCH] [analyzer] Fix crash inside RangeConstraintManager.cpp introduced by D112621 It seems like `LHS` and `RHS` could be empty range sets. This caused an assertion failure inside RangeConstraintManager. I'm hoisting out the check from the function into the call-site. This way we could assert that we only want to deal with non-empty range sets. The relevant part of the trace: ``` #6 0x00007fe6ff5f81a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6) #7 0x00007fe6ff5f8252 (/lib64/libc.so.6+0x2f252) #8 0x00000000049caed2 (anonymous namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet, clang::BinaryOperatorKind, clang::ento::RangeSet, clang::QualType) RangeConstraintManager.cpp:0:0 #9 0x00000000049c9867 (anonymous namespace)::SymbolicRangeInferrer::infer(clang::ento::SymExpr const*) RangeConstraintManager.cpp:0:0 #10 0x00000000049bebf5 (anonymous namespace)::RangeConstraintManager::assumeSymNE(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, llvm::APSInt const&, llvm::APSInt const&) RangeConstraintManager.cpp:0:0 #11 0x00000000049d368c clang::ento::RangedConstraintManager::assumeSymUnsupported(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, bool) (../../main-github/llvm/build-all/bin/clang+0x49d368c) #12 0x00000000049f0b09 clang::ento::SimpleConstraintManager::assumeAux(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) (../../main-github/llvm/build-all/bin/clang+0x49f0b09) #13 0x00000000049f096a clang::ento::SimpleConstraintManager::assume(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) (../../main-github/llvm/build-all/bin/clang+0x49f096a) #14 0x00000000049f086d clang::ento::SimpleConstraintManager::assumeInternal(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal, bool) (../../main-github/llvm/build-all/bin/clang+0x49f086d) #15 0x000000000492d3e3 clang::ento::ConstraintManager::assumeDual(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal) (../../main-github/llvm/build-all/bin/clang+0x492d3e3) #16 0x0000000004955b6d clang::ento::ExprEngine::evalEagerlyAssumeBinOpBifurcation(clang::ento::ExplodedNodeSet&, clang::ento::ExplodedNodeSet&, clang::Expr const*) (../../main-github/llvm/build-all/bin/clang+0x4955b6d) #17 0x00000000049514b6 clang::ento::ExprEngine::Visit(clang::Stmt const*, clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) (../../main-github/llvm/build-all/bin/clang+0x49514b6) #18 0x000000000494c73e clang::ento::ExprEngine::ProcessStmt(clang::Stmt const*, clang::ento::ExplodedNode*) (../../main-github/llvm/build-all/bin/clang+0x494c73e) #19 0x000000000494c459 clang::ento::ExprEngine::processCFGElement(clang::CFGElement, clang::ento::ExplodedNode*, unsigned int, clang::ento::NodeBuilderContext*) (../../main-github/llvm/build-all/bin/clang+0x494c459) #20 0x000000000492f3d0 clang::ento::CoreEngine::HandlePostStmt(clang::CFGBlock const*, unsigned int, clang::ento::ExplodedNode*) (../../main-github/llvm/build-all/bin/clang+0x492f3d0) #21 0x000000000492e1f6 clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, unsigned int, llvm::IntrusiveRefCntPtr) (../../main-github/llvm/build-all/bin/clang+0x492e1f6) ``` Differential Revision: https://reviews.llvm.org/D112621 (cherry picked from commit f61a08b67f5c8b0202dd30821766ee029e880b7c) --- .../Core/RangeConstraintManager.cpp | 15 ++++++++------- clang/test/Analysis/constant-folding-crash.cpp | 11 +++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) create mode 100644 clang/test/Analysis/constant-folding-crash.cpp diff --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp index 941d1c2f9d0e8c..1fae22f1581cc8 100644 --- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp @@ -1362,11 +1362,7 @@ class SymbolicRangeInferrer template RangeSet VisitBinaryOperator(RangeSet LHS, RangeSet RHS, QualType T) { - // We should propagate information about unfeasbility of one of the - // operands to the resulting range. - if (LHS.isEmpty() || RHS.isEmpty()) { - return RangeFactory.getEmptySet(); - } + assert(!LHS.isEmpty() && !RHS.isEmpty()); Range CoarseLHS = fillGaps(LHS); Range CoarseRHS = fillGaps(RHS); @@ -1617,8 +1613,7 @@ template <> RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS, RangeSet RHS, QualType T) { - - assert(!LHS.isEmpty() && !RHS.isEmpty() && "Both ranges should be non-empty"); + assert(!LHS.isEmpty() && !RHS.isEmpty()); if (LHS.getAPSIntType() == RHS.getAPSIntType()) { if (intersect(RangeFactory, LHS, RHS).isEmpty()) @@ -1802,6 +1797,12 @@ RangeSet SymbolicRangeInferrer::VisitBinaryOperator(Range LHS, RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS, BinaryOperator::Opcode Op, RangeSet RHS, QualType T) { + // We should propagate information about unfeasbility of one of the + // operands to the resulting range. + if (LHS.isEmpty() || RHS.isEmpty()) { + return RangeFactory.getEmptySet(); + } + switch (Op) { case BO_NE: return VisitBinaryOperator(LHS, RHS, T); diff --git a/clang/test/Analysis/constant-folding-crash.cpp b/clang/test/Analysis/constant-folding-crash.cpp new file mode 100644 index 00000000000000..156bfa5dda6a67 --- /dev/null +++ b/clang/test/Analysis/constant-folding-crash.cpp @@ -0,0 +1,11 @@ +// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s +// expected-no-diagnostics + +namespace bbi_77010 { +int crash_NE(int rhs, int lhs, int x) { + int band = lhs & rhs; + if (0 <= band) {} + if (rhs > 0) {} + return band != x; // no-crash D112621 +} +} // namespace bbi_77010