Index: include/clang/StaticAnalyzer/Checkers/SValExplainer.h =================================================================== --- include/clang/StaticAnalyzer/Checkers/SValExplainer.h +++ include/clang/StaticAnalyzer/Checkers/SValExplainer.h @@ -125,8 +125,14 @@ return OS.str(); } - // TODO: IntSymExpr doesn't appear in practice. - // Add the relevant code once it does. + std::string VisitIntSymExpr(const IntSymExpr *S) { + std::string Str; + llvm::raw_string_ostream OS(Str); + OS << S->getLHS() + << std::string(BinaryOperator::getOpcodeStr(S->getOpcode())) << " " + << "(" << Visit(S->getRHS()) << ") "; + return OS.str(); + } std::string VisitSymSymExpr(const SymSymExpr *S) { return "(" + Visit(S->getLHS()) + ") " + @@ -134,8 +140,10 @@ " (" + Visit(S->getRHS()) + ")"; } - // TODO: SymbolCast doesn't appear in practice. - // Add the relevant code once it does. + std::string VisitSymbolCast(const SymbolCast *S) { + return "cast of type '" + S->getType().getAsString() + "' of " + + Visit(S->getOperand()); + } std::string VisitSymbolicRegion(const SymbolicRegion *R) { // Explain 'this' object here. Index: lib/StaticAnalyzer/Core/SValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SValBuilder.cpp +++ lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -100,7 +100,7 @@ if (T->isNullPtrType()) return makeZeroVal(T); - + if (!SymbolManager::canSymbolicate(T)) return UnknownVal(); @@ -354,9 +354,6 @@ BinaryOperator::Opcode Op, NonLoc LHS, NonLoc RHS, QualType ResultTy) { - if (!State->isTainted(RHS) && !State->isTainted(LHS)) - return UnknownVal(); - const SymExpr *symLHS = LHS.getAsSymExpr(); const SymExpr *symRHS = RHS.getAsSymExpr(); // TODO: When the Max Complexity is reached, we should conjure a symbol @@ -364,7 +361,7 @@ const unsigned MaxComp = 10000; // 100000 28X if (symLHS && symRHS && - (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp) + (symLHS->computeComplexity() + symRHS->computeComplexity()) < MaxComp) return makeNonLoc(symLHS, Op, symRHS, ResultTy); if (symLHS && symLHS->computeComplexity() < MaxComp) Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp =================================================================== --- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp +++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp @@ -672,12 +672,12 @@ // If one of the operands is a symbol and the other is a constant, // build an expression for use by the constraint manager. if (SymbolRef rSym = rhs.getAsLocSymbol()) { - // We can only build expressions with symbols on the left, - // so we need a reversible operator. + const llvm::APSInt &lVal = lhs.castAs().getValue(); + + // Prefer expressions with symbols on the left if (!BinaryOperator::isComparisonOp(op)) - return UnknownVal(); + return makeNonLoc(lVal, op, rSym, resultTy); - const llvm::APSInt &lVal = lhs.castAs().getValue(); op = BinaryOperator::reverseComparisonOp(op); return makeNonLoc(rSym, op, lVal, resultTy); } @@ -997,7 +997,8 @@ if (SymbolRef Sym = V.getAsSymbol()) return state->getConstraintManager().getSymVal(state, Sym); - // FIXME: Add support for SymExprs. + // FIXME: Add support for SymExprs in RangeConstraintManager. + return nullptr; } @@ -1022,8 +1023,11 @@ return nonloc::SymbolVal(S); } - // TODO: Support SymbolCast. Support IntSymExpr when/if we actually - // start producing them. + SVal VisitIntSymExpr(const IntSymExpr *S) { + SVal RHS = Visit(S->getRHS()); + SVal LHS = SVB.makeIntVal(S->getLHS()); + return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); + } SVal VisitSymIntExpr(const SymIntExpr *S) { SVal LHS = Visit(S->getLHS()); @@ -1048,6 +1052,11 @@ return SVB.evalBinOp(State, S->getOpcode(), LHS, RHS, S->getType()); } + SVal VisitSymbolCast(const SymbolCast *S) { + SVal V = Visit(S->getOperand()); + return SVB.evalCast(V, S->getType(), S->getOperand()->getType()); + } + SVal VisitSymSymExpr(const SymSymExpr *S) { SVal LHS = Visit(S->getLHS()); SVal RHS = Visit(S->getRHS()); @@ -1061,7 +1070,8 @@ SVal VisitNonLocSymbolVal(nonloc::SymbolVal V) { // Simplification is much more costly than computing complexity. // For high complexity, it may be not worth it. - if (V.getSymbol()->computeComplexity() > 100) + // Use a lower bound to avoid recursive blowup, e.g. on PR24184.cpp + if (V.getSymbol()->computeComplexity() > 10) return V; return Visit(V.getSymbol()); } Index: lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp =================================================================== --- lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp +++ lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp @@ -1034,16 +1034,19 @@ ProgramStateRef Z3ConstraintManager::assumeSymInclusiveRange( ProgramStateRef State, SymbolRef Sym, const llvm::APSInt &From, const llvm::APSInt &To, bool InRange) { + ASTContext &Ctx = getBasicVals().getContext(); + // FIXME: This should be a cast from a 1-bit integer type to a boolean type, + // but the former is not available in Clang. Instead, extend the APSInt + // directly. + bool isBoolTy = From.getBitWidth() == 1 && getAPSIntType(From).isNull(); + QualType RetTy; // The expression may be casted, so we cannot call getZ3DataExpr() directly Z3Expr Exp = getZ3Expr(Sym, &RetTy); - - assert((getAPSIntType(From) == getAPSIntType(To)) && - "Range values have different types!"); - QualType RTy = getAPSIntType(From); - bool isSignedTy = RetTy->isSignedIntegerOrEnumerationType(); - Z3Expr FromExp = Z3Expr::fromAPSInt(From); - Z3Expr ToExp = Z3Expr::fromAPSInt(To); + QualType RTy = isBoolTy ? Ctx.BoolTy : getAPSIntType(From); + Z3Expr FromExp = + isBoolTy ? Z3Expr::fromAPSInt(From.extend(Ctx.getTypeSize(Ctx.BoolTy))) + : Z3Expr::fromAPSInt(From); // Construct single (in)equality if (From == To) @@ -1051,6 +1054,10 @@ getZ3BinExpr(Exp, RetTy, InRange ? BO_EQ : BO_NE, FromExp, RTy, nullptr)); + assert((getAPSIntType(From) == getAPSIntType(To)) && + "Range values have different types!"); + + Z3Expr ToExp = Z3Expr::fromAPSInt(To); // Construct two (in)equalities, and a logical and/or Z3Expr LHS = getZ3BinExpr(Exp, RetTy, InRange ? BO_GE : BO_LT, FromExp, RTy, nullptr); @@ -1058,7 +1065,8 @@ getZ3BinExpr(Exp, RetTy, InRange ? BO_LE : BO_GT, ToExp, RTy, nullptr); return assumeZ3Expr( State, Sym, - Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, isSignedTy)); + Z3Expr::fromBinOp(LHS, InRange ? BO_LAnd : BO_LOr, RHS, + RetTy->isSignedIntegerOrEnumerationType())); } ProgramStateRef Z3ConstraintManager::assumeSymUnsupported(ProgramStateRef State, @@ -1406,6 +1414,7 @@ QualType <y, QualType &RTy) const { ASTContext &Ctx = getBasicVals().getContext(); + assert(!LTy.isNull() && !RTy.isNull() && "Input type is null!"); // Perform type conversion if (LTy->isIntegralOrEnumerationType() && RTy->isIntegralOrEnumerationType()) { @@ -1468,10 +1477,10 @@ void Z3ConstraintManager::doIntTypeConversion(T &LHS, QualType <y, T &RHS, QualType &RTy) const { ASTContext &Ctx = getBasicVals().getContext(); - uint64_t LBitWidth = Ctx.getTypeSize(LTy); uint64_t RBitWidth = Ctx.getTypeSize(RTy); + assert(!LTy.isNull() && !RTy.isNull() && "Input type is null!"); // Always perform integer promotion before checking type equality. // Otherwise, e.g. (bool) a + (bool) b could trigger a backend assertion if (LTy->isPromotableIntegerType()) { Index: test/Analysis/bitwise-ops.c =================================================================== --- test/Analysis/bitwise-ops.c +++ test/Analysis/bitwise-ops.c @@ -7,10 +7,9 @@ // Sanity check CHECK(x); // expected-warning{{TRUE}} CHECK(x & 1); // expected-warning{{TRUE}} - - // False positives due to SValBuilder giving up on certain kinds of exprs. - CHECK(1 - x); // expected-warning{{UNKNOWN}} - CHECK(x & y); // expected-warning{{UNKNOWN}} + + CHECK(1 - x); // expected-warning{{TRUE}} + CHECK(x & y); // expected-warning{{TRUE}} } int testConstantShifts_PR18073(int which) { @@ -29,4 +28,4 @@ default: return 0; } -} \ No newline at end of file +} Index: test/Analysis/bool-assignment.c =================================================================== --- test/Analysis/bool-assignment.c +++ test/Analysis/bool-assignment.c @@ -43,11 +43,7 @@ return; } if (y > 200 && y < 250) { -#ifdef ANALYZER_CM_Z3 BOOL x = y; // expected-warning {{Assignment of a non-Boolean value}} -#else - BOOL x = y; // no-warning -#endif return; } if (y >= 127 && y < 150) { Index: test/Analysis/conditional-path-notes.c =================================================================== --- test/Analysis/conditional-path-notes.c +++ test/Analysis/conditional-path-notes.c @@ -77,7 +77,8 @@ void testNonDiagnosableBranchArithmetic(int a, int b) { if (a - b) { - // expected-note@-1 {{Taking true branch}} + // expected-note@-1 {{Assuming the condition is true}} + // expected-note@-2 {{Taking true branch}} *(volatile int *)0 = 1; // expected-warning{{Dereference of null pointer}} // expected-note@-1 {{Dereference of null pointer}} } @@ -1573,12 +1574,75 @@ // CHECK-NEXT: end // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col11 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Assuming the condition is true +// CHECK-NEXT: message +// CHECK-NEXT: Assuming the condition is true +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line79 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line82 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -1594,12 +1658,12 @@ // CHECK-NEXT: start // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -1607,12 +1671,12 @@ // CHECK-NEXT: end // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col24 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col24 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -1624,7 +1688,7 @@ // CHECK-NEXT: kindevent // CHECK-NEXT: location // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col24 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -1632,12 +1696,12 @@ // CHECK-NEXT: // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col5 // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col26 // CHECK-NEXT: file0 // CHECK-NEXT: @@ -1658,10 +1722,10 @@ // CHECK-NEXT: issue_hash_content_of_line_in_contextf56671e5f67c73abef619b56f7c29fa4 // CHECK-NEXT: issue_context_kindfunction // CHECK-NEXT: issue_contexttestNonDiagnosableBranchArithmetic -// CHECK-NEXT: issue_hash_function_offset3 +// CHECK-NEXT: issue_hash_function_offset4 // CHECK-NEXT: location // CHECK-NEXT: -// CHECK-NEXT: line81 +// CHECK-NEXT: line82 // CHECK-NEXT: col24 // CHECK-NEXT: file0 // CHECK-NEXT: Index: test/Analysis/explain-svals.cpp =================================================================== --- test/Analysis/explain-svals.cpp +++ test/Analysis/explain-svals.cpp @@ -69,7 +69,7 @@ static int stat; clang_analyzer_explain(x + 1); // expected-warning-re{{{{^\(argument 'x'\) \+ 1$}}}} clang_analyzer_explain(1 + y); // expected-warning-re{{{{^\(argument 'y'\) \+ 1$}}}} - clang_analyzer_explain(x + y); // expected-warning-re{{{{^unknown value$}}}} + clang_analyzer_explain(x + y); // expected-warning-re{{{{^\(argument 'x'\) \+ \(argument 'y'\)$}}}} clang_analyzer_explain(z); // expected-warning-re{{{{^undefined value$}}}} clang_analyzer_explain(&z); // expected-warning-re{{{{^pointer to local variable 'z'$}}}} clang_analyzer_explain(stat); // expected-warning-re{{{{^signed 32-bit integer '0'$}}}} Index: test/Analysis/plist-macros.cpp =================================================================== --- test/Analysis/plist-macros.cpp +++ test/Analysis/plist-macros.cpp @@ -1,5 +1,5 @@ // RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -verify %s -// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=ture %s -o %t.plist +// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix -analyzer-eagerly-assume -analyzer-output=plist-multi-file -analyzer-config path-diagnostics-alternate=true %s -o %t.plist // RUN: FileCheck --input-file=%t.plist %s @@ -85,6 +85,7 @@ void test2(int *p) { CALL_FN(p); } + // CHECK: diagnostics // CHECK-NEXT: // CHECK-NEXT: @@ -636,6 +637,69 @@ // CHECK-NEXT: end // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col25 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Assuming the condition is true +// CHECK-NEXT: message +// CHECK-NEXT: Assuming the condition is true +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line36 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: line37 // CHECK-NEXT: col5 // CHECK-NEXT: file0 Index: test/Analysis/range_casts.c =================================================================== --- test/Analysis/range_casts.c +++ test/Analysis/range_casts.c @@ -67,8 +67,8 @@ { unsigned index = -1; if (index < foo) index = foo; - if (index - 1 == 0) // Was not reached prior fix. - clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + if (index - 1 == 0) + clang_analyzer_warnIfReached(); // no-warning else clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} } @@ -87,8 +87,8 @@ { unsigned index = -1; if (index < foo) index = foo; - if (index - 1L == 0L) // Was not reached prior fix. - clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + if (index - 1L == 0L) + clang_analyzer_warnIfReached(); // no-warning else clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} } @@ -117,8 +117,8 @@ { unsigned index = -1; if (index < foo) index = foo; - if (index - 1UL == 0L) // Was not reached prior fix. - clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} + if (index - 1UL == 0L) + clang_analyzer_warnIfReached(); // no-warning else clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}} } Index: test/Analysis/std-c-library-functions.c =================================================================== --- test/Analysis/std-c-library-functions.c +++ test/Analysis/std-c-library-functions.c @@ -57,8 +57,7 @@ size_t y = fread(buf, sizeof(int), 10, fp); clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}} size_t z = fwrite(buf, sizeof(int), y, fp); - // FIXME: should be TRUE once symbol-symbol constraint support is improved. - clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(z <= y); // expected-warning{{TRUE}} } ssize_t getline(char **, size_t *, FILE *);