diff --git a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h index 41c0a3a46b1c..e944585f983f 100644 --- a/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h +++ b/clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitor.h @@ -159,6 +159,12 @@ public: const bool tookTrue, BugReporterContext &BRC, const LocationContext *LC); + + PathDiagnosticPiece *VisitConditionVariable(StringRef LhsString, + const Expr *CondVarExpr, + const bool tookTrue, + BugReporterContext &BRC, + const LocationContext *LC); bool patternMatch(const Expr *Ex, llvm::raw_ostream &Out, diff --git a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp index 6828a9e15906..88f38a3e4b48 100644 --- a/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ b/clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -604,18 +604,26 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, shouldInvert = !isVarLHS && isVarRHS; } + BinaryOperator::Opcode Op = BExpr->getOpcode(); + + if (BinaryOperator::isAssignmentOp(Op)) { + // For assignment operators, all that we care about is that the LHS + // evaluates to "true" or "false". + return VisitConditionVariable(LhsString, BExpr->getLHS(), tookTrue, + BRC, LC); + } + + // For non-assignment operations, we require that we can understand + // both the LHS and RHS. if (LhsString.empty() || RhsString.empty()) return 0; - - // Should we invert the strings if the LHS is not a variable name? + // Should we invert the strings if the LHS is not a variable name? llvm::SmallString<256> buf; llvm::raw_svector_ostream Out(buf); Out << "Assuming " << (shouldInvert ? RhsString : LhsString) << " is "; // Do we need to invert the opcode? - BinaryOperator::Opcode Op = BExpr->getOpcode(); - if (shouldInvert) switch (Op) { default: break; @@ -654,6 +662,33 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LC); return new PathDiagnosticEventPiece(Loc, Out.str()); } + +PathDiagnosticPiece * +ConditionBRVisitor::VisitConditionVariable(StringRef LhsString, + const Expr *CondVarExpr, + const bool tookTrue, + BugReporterContext &BRC, + const LocationContext *LC) { + llvm::SmallString<256> buf; + llvm::raw_svector_ostream Out(buf); + Out << "Assuming " << LhsString << " is "; + + QualType Ty = CondVarExpr->getType(); + + if (Ty->isPointerType()) + Out << (tookTrue ? "not null" : "null"); + else if (Ty->isObjCObjectPointerType()) + Out << (tookTrue ? "not nil" : "nil"); + else if (Ty->isBooleanType()) + Out << (tookTrue ? "true" : "false"); + else if (Ty->isIntegerType()) + Out << (tookTrue ? "non-zero" : "zero"); + else + return 0; + + PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LC); + return new PathDiagnosticEventPiece(Loc, Out.str()); +} PathDiagnosticPiece * ConditionBRVisitor::VisitTrueTest(const Expr *Cond, diff --git a/clang/test/Analysis/plist-output.m b/clang/test/Analysis/plist-output.m index 2e209bbba0d2..bed4a85cbe17 100644 --- a/clang/test/Analysis/plist-output.m +++ b/clang/test/Analysis/plist-output.m @@ -50,6 +50,14 @@ void test_assumptions(int a, int b) *p = 0xDEADBEEF; } +int *bar_cond_assign(); +int test_cond_assign() { + int *p; + if (p = bar_cond_assign()) + return 1; + return *p; +} + // CHECK: // CHECK: // CHECK: @@ -975,6 +983,212 @@ void test_assumptions(int a, int b) // CHECK: file0 // CHECK: // CHECK: +// CHECK: +// CHECK: path +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line55 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line55 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: extended_message +// CHECK: Assuming 'p' is null +// CHECK: message +// CHECK: Assuming 'p' is null +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line56 +// CHECK: col7 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindcontrol +// CHECK: edges +// CHECK: +// CHECK: +// CHECK: start +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col3 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: end +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: +// CHECK: kindevent +// CHECK: location +// CHECK: +// CHECK: line58 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: ranges +// CHECK: +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: line58 +// CHECK: col11 +// CHECK: file0 +// CHECK: +// CHECK: +// CHECK: +// CHECK: extended_message +// CHECK: Dereference of null pointer (loaded from variable 'p') +// CHECK: message +// CHECK: Dereference of null pointer (loaded from variable 'p') +// CHECK: +// CHECK: +// CHECK: descriptionDereference of null pointer (loaded from variable 'p') +// CHECK: categoryLogic error +// CHECK: typeDereference of null pointer +// CHECK: location +// CHECK: +// CHECK: line58 +// CHECK: col10 +// CHECK: file0 +// CHECK: +// CHECK: // CHECK: // CHECK: // CHECK: