Index: include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -200,7 +200,7 @@ BugReport &R, const ExplodedNode *N); std::shared_ptr - VisitTrueTest(const Expr *Cond, const DeclRefExpr *DR, const bool tookTrue, + VisitTrueTest(const Expr *Cond, const DeclRefExpr *DRE, const bool tookTrue, BugReporterContext &BRC, BugReport &R, const ExplodedNode *N); std::shared_ptr @@ -208,6 +208,10 @@ const bool tookTrue, BugReporterContext &BRC, BugReport &R, const ExplodedNode *N); + std::shared_ptr + VisitTrueTest(const Expr *Cond, const MemberExpr *ME, const bool tookTrue, + BugReporterContext &BRC, BugReport &R, const ExplodedNode *N); + std::shared_ptr VisitConditionVariable(StringRef LhsString, const Expr *CondVarExpr, const bool tookTrue, BugReporterContext &BRC, @@ -231,7 +235,8 @@ BugReporterContext &BRC, BugReport &R, const ExplodedNode *N, - Optional &prunable); + Optional &prunable, + bool IsSameFieldName); static bool isPieceMessageGeneric(const PathDiagnosticPiece *Piece); }; Index: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -1968,6 +1968,11 @@ tookTrueTmp, BRC, R, N)) return P; break; + case Stmt::MemberExprClass: + if (auto P = VisitTrueTest(Cond, cast(CondTmp), + tookTrueTmp, BRC, R, N)) + return P; + break; case Stmt::UnaryOperatorClass: { const auto *UO = cast(CondTmp); if (UO->getOpcode() == UO_LNot) { @@ -1998,7 +2003,8 @@ BugReporterContext &BRC, BugReport &report, const ExplodedNode *N, - Optional &prunable) { + Optional &prunable, + bool IsSameFieldName) { const Expr *OriginalExpr = Ex; Ex = Ex->IgnoreParenCasts(); @@ -2085,6 +2091,17 @@ return false; } + if (const auto *ME = dyn_cast(Ex)) { + if (!IsSameFieldName) + Out << "field '" << ME->getMemberDecl()->getName() << '\''; + else + Out << '\'' + << Lexer::getSourceText( + CharSourceRange::getTokenRange(Ex->getSourceRange()), + BRC.getSourceManager(), BRC.getASTContext().getLangOpts(), 0) + << '\''; + } + return false; } @@ -2095,13 +2112,23 @@ bool shouldInvert = false; Optional shouldPrune; + // Check if the field name of the MemberExprs is ambiguous. Example: + // " 'a.d' is equal to 'h.d' " in 'test/Analysis/null-deref-path-notes.cpp'. + bool IsSameFieldName = false; + if (const auto *LhsME = + dyn_cast(BExpr->getLHS()->IgnoreParenCasts())) + if (const auto *RhsME = + dyn_cast(BExpr->getRHS()->IgnoreParenCasts())) + IsSameFieldName = LhsME->getMemberDecl()->getName() == + RhsME->getMemberDecl()->getName(); + SmallString<128> LhsString, RhsString; { llvm::raw_svector_ostream OutLHS(LhsString), OutRHS(RhsString); - const bool isVarLHS = patternMatch(BExpr->getLHS(), BExpr, OutLHS, - BRC, R, N, shouldPrune); - const bool isVarRHS = patternMatch(BExpr->getRHS(), BExpr, OutRHS, - BRC, R, N, shouldPrune); + const bool isVarLHS = patternMatch(BExpr->getLHS(), BExpr, OutLHS, BRC, R, + N, shouldPrune, IsSameFieldName); + const bool isVarRHS = patternMatch(BExpr->getRHS(), BExpr, OutRHS, BRC, R, + N, shouldPrune, IsSameFieldName); shouldInvert = !isVarLHS && isVarRHS; } @@ -2164,7 +2191,12 @@ Out << (shouldInvert ? LhsString : RhsString); const LocationContext *LCtx = N->getLocationContext(); PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); - auto event = std::make_shared(Loc, Out.str()); + + // Convert 'field ...' to 'Field ...' if it is a MemberExpr. + std::string Message = Out.str(); + Message[0] = toupper(Message[0]); + + auto event = std::make_shared(Loc, Message); if (shouldPrune.hasValue()) event->setPrunable(shouldPrune.getValue()); return event; @@ -2231,6 +2263,27 @@ return std::move(event); } +std::shared_ptr +ConditionBRVisitor::VisitTrueTest(const Expr *Cond, const MemberExpr *ME, + const bool tookTrue, BugReporterContext &BRC, + BugReport &report, const ExplodedNode *N) { + SmallString<256> Buf; + llvm::raw_svector_ostream Out(Buf); + + Out << (IsAssuming ? "Assuming field '" : "Field '") + << ME->getMemberDecl()->getName() << "' is "; + + if (!printValue(ME, tookTrue, Out, N)) + return nullptr; + + const LocationContext *LCtx = N->getLocationContext(); + PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); + if (!Loc.isValid() || !Loc.asLocation().isValid()) + return nullptr; + + return std::make_shared(Loc, Out.str()); +} + bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, const bool TookTrue, raw_ostream &Out, const ExplodedNode *N) { Index: test/Analysis/Inputs/expected-plists/edges-new.mm.plist =================================================================== --- test/Analysis/Inputs/expected-plists/edges-new.mm.plist +++ test/Analysis/Inputs/expected-plists/edges-new.mm.plist @@ -22284,9 +22284,9 @@ depth0 extended_message - The condition is true + Field 'b' is equal to 2 message - The condition is true + Field 'b' is equal to 2 kindcontrol Index: test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist =================================================================== --- test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist +++ test/Analysis/diagnostics/Inputs/expected-plists/deref-track-symbolic-region.c.plist @@ -165,9 +165,9 @@ depth0 extended_message - Assuming pointer value is null + Assuming field 'x' is null message - Assuming pointer value is null + Assuming field 'x' is null kindcontrol @@ -454,9 +454,9 @@ depth0 extended_message - Assuming pointer value is null + Assuming field 'x' is null message - Assuming pointer value is null + Assuming field 'x' is null kindcontrol Index: test/Analysis/diagnostics/deref-track-symbolic-region.c =================================================================== --- test/Analysis/diagnostics/deref-track-symbolic-region.c +++ test/Analysis/diagnostics/deref-track-symbolic-region.c @@ -15,8 +15,8 @@ struct S *ps = &syz; if (ps->x) - //expected-note@-1{{Taking false branch}} - //expected-note@-2{{Assuming pointer value is null}} + //expected-note@-1{{Assuming field 'x' is null}} + //expected-note@-2{{Taking false branch}} m++; @@ -30,8 +30,8 @@ struct S *ps = &syz; if (ps->x) - //expected-note@-1{{Taking false branch}} - //expected-note@-2{{Assuming pointer value is null}} + //expected-note@-1{{Assuming field 'x' is null}} + //expected-note@-2{{Taking false branch}} m++; int *p = syz.x; //expected-note {{'p' initialized to a null pointer value}} Index: test/Analysis/diagnostics/dtors.cpp =================================================================== --- test/Analysis/diagnostics/dtors.cpp +++ test/Analysis/diagnostics/dtors.cpp @@ -16,7 +16,7 @@ S *s; smart_ptr(S *); S *get() { - return (x || 0) ? nullptr : s; // expected-note{{The condition is false}} + return (x || 0) ? nullptr : s; // expected-note{{Field 'x' is 0}} // expected-note@-1{{Left side of '||' is false}} // expected-note@-2{{'?' condition is false}} // expected-warning@-3{{Use of memory after it is freed}} Index: test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist =================================================================== --- test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist +++ test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist @@ -4271,9 +4271,9 @@ depth0 extended_message - Assuming pointer value is null + Assuming field 'arr' is null message - Assuming pointer value is null + Assuming field 'arr' is null kindcontrol Index: test/Analysis/inlining/path-notes.cpp =================================================================== --- test/Analysis/inlining/path-notes.cpp +++ test/Analysis/inlining/path-notes.cpp @@ -231,7 +231,7 @@ }; void Owner::testGetDerefExprOnMemberExprWithADot() { - if (arr) // expected-note {{Assuming pointer value is null}} + if (arr) // expected-note {{Assuming field 'arr' is null}} // expected-note@-1 {{Taking false branch}} ; arr[1].x = 1; //expected-warning {{Dereference of null pointer}} Index: test/Analysis/null-deref-path-notes.cpp =================================================================== --- test/Analysis/null-deref-path-notes.cpp +++ test/Analysis/null-deref-path-notes.cpp @@ -19,7 +19,7 @@ // expected-note@-1{{Array access (via field 'd') results in a null pointer dereference}} B h, a; // expected-note{{Value assigned to 'h.d'}} a.d == __null; // expected-note{{Assuming the condition is true}} - a.d != h.d; // expected-note{{Assuming pointer value is null}} + a.d != h.d; // expected-note{{Assuming 'a.d' is equal to 'h.d'}} f(h, b); // expected-note{{Calling 'c::f'}} } } Index: test/Analysis/osobject-retain-release.cpp =================================================================== --- test/Analysis/osobject-retain-release.cpp +++ test/Analysis/osobject-retain-release.cpp @@ -601,13 +601,13 @@ { OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr'}} // expected-note@-1{{Returning from constructor for 'smart_ptr'}} - // expected-note@os_smart_ptr.h:13{{The condition is true}} + // expected-note@os_smart_ptr.h:13{{Field 'pointer' is non-null}} // expected-note@os_smart_ptr.h:13{{Taking true branch}} // expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}} // expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}} // expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}} } // expected-note{{Calling '~smart_ptr'}} - // expected-note@os_smart_ptr.h:35{{The condition is true}} + // expected-note@os_smart_ptr.h:35{{Field 'pointer' is non-null}} // expected-note@os_smart_ptr.h:35{{Taking true branch}} // expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}} // expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}} @@ -623,13 +623,13 @@ { OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr'}} // expected-note@-1{{Returning from constructor for 'smart_ptr'}} - // expected-note@os_smart_ptr.h:13{{The condition is true}} + // expected-note@os_smart_ptr.h:13{{Field 'pointer' is non-null}} // expected-note@os_smart_ptr.h:13{{Taking true branch}} // expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}} // expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}} // expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}} } // expected-note{{Calling '~smart_ptr'}} - // expected-note@os_smart_ptr.h:35{{The condition is true}} + // expected-note@os_smart_ptr.h:35{{Field 'pointer' is non-null}} // expected-note@os_smart_ptr.h:35{{Taking true branch}} // expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}} // expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}} Index: test/Analysis/uninit-vals.m =================================================================== --- test/Analysis/uninit-vals.m +++ test/Analysis/uninit-vals.m @@ -164,7 +164,7 @@ // expected-note@-1{{TRUE}} testObj->origin = makePoint(0.0, 0.0); - if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}} + if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}} // expected-note@-1{{Taking false branch}} // FIXME: Assigning to 'testObj->origin' kills the default binding for the @@ -219,13 +219,13 @@ // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint(1, 2); - if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}} + if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}} // expected-note@-1{{Taking false branch}} - // expected-note@-2{{Assuming the condition is false}} + // expected-note@-2{{Assuming field 'size' is <= 0}} // expected-note@-3{{Taking false branch}} - // expected-note@-4{{Assuming the condition is false}} + // expected-note@-4{{Assuming field 'size' is <= 0}} // expected-note@-5{{Taking false branch}} - // expected-note@-6{{Assuming the condition is false}} + // expected-note@-6{{Assuming field 'size' is <= 0}} // expected-note@-7{{Taking false branch}} // FIXME: Assigning to 'testObj->origin' kills the default binding for the @@ -321,11 +321,11 @@ // expected-note@-1{{TRUE}} testObj->origin = makeIntPoint2D(1, 2); - if (testObj->size > 0) { ; } // expected-note{{The condition is false}} + if (testObj->size > 0) { ; } // expected-note{{Field 'size' is <= 0}} // expected-note@-1{{Taking false branch}} - // expected-note@-2{{The condition is false}} + // expected-note@-2{{Field 'size' is <= 0}} // expected-note@-3{{Taking false branch}} - // expected-note@-4{{The condition is false}} + // expected-note@-4{{Field 'size' is <= 0}} // expected-note@-5{{Taking false branch}} clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}