Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h =================================================================== --- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h +++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h @@ -223,14 +223,25 @@ bool printValue(const Expr *CondVarExpr, raw_ostream &Out, const ExplodedNode *N, bool TookTrue, bool IsAssuming); - bool patternMatch(const Expr *Ex, - const Expr *ParentEx, - raw_ostream &Out, - BugReporterContext &BRC, - BugReport &R, - const ExplodedNode *N, - Optional &prunable, - bool IsSameFieldName); + /// Prints out the name or the value of the given expression. + /// We print out the name if we work with a variable or a macro, + /// otherwise it is a literal so we pretty-print its value. + /// + /// \param E The expression to print its name or value. + /// \param BOExpr The immediate ancestor of \c E. + /// \param Out The stream to print. + /// \param BRC The context. + /// \param R The report. + /// \param N The node where we encountered the condition. + /// \param Prunable True if the given expression is not interesting. + /// \param IsSameFieldName True if both the LHS and RHS are \c MemberExprs and + /// the name of the fields are equal. + /// + /// \return Whether the \c E is refers to a variable. + bool printNameOrValue(const Expr *E, const BinaryOperator *BOExpr, + raw_ostream &Out, BugReporterContext &BRC, BugReport &R, + const ExplodedNode *N, Optional &Prunable, + bool IsSameFieldName); static bool isPieceMessageGeneric(const PathDiagnosticPiece *Piece); }; Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -2020,38 +2020,34 @@ Loc, TookTrue ? GenericTrueMessage : GenericFalseMessage); } -bool ConditionBRVisitor::patternMatch(const Expr *Ex, - const Expr *ParentEx, - raw_ostream &Out, - BugReporterContext &BRC, - BugReport &report, - const ExplodedNode *N, - Optional &prunable, - bool IsSameFieldName) { - const Expr *OriginalExpr = Ex; - Ex = Ex->IgnoreParenCasts(); +bool ConditionBRVisitor::printNameOrValue( + const Expr *E, const BinaryOperator *BOExpr, raw_ostream &Out, + BugReporterContext &BRC, BugReport &R, const ExplodedNode *N, + Optional &Prunable, bool IsSameFieldName) { + SourceManager &SM = BRC.getSourceManager(); + const LangOptions &LO = BRC.getASTContext().getLangOpts(); + + QualType Ty = E->getType(); + E = E->IgnoreParenCasts(); - // Use heuristics to determine if Ex is a macro expending to a literal and + // Use heuristics to determine if 'E' is a macro expending to a literal and // if so, use the macro's name. - SourceLocation LocStart = Ex->getBeginLoc(); - SourceLocation LocEnd = Ex->getEndLoc(); + SourceLocation LocStart = E->getBeginLoc(); + SourceLocation LocEnd = E->getEndLoc(); if (LocStart.isMacroID() && LocEnd.isMacroID() && - (isa(Ex) || - isa(Ex) || - isa(Ex) || - isa(Ex) || - isa(Ex))) { - StringRef StartName = Lexer::getImmediateMacroNameForDiagnostics(LocStart, - BRC.getSourceManager(), BRC.getASTContext().getLangOpts()); - StringRef EndName = Lexer::getImmediateMacroNameForDiagnostics(LocEnd, - BRC.getSourceManager(), BRC.getASTContext().getLangOpts()); + (isa(E) || isa(E) || + isa(E) || isa(E) || + isa(E))) { + StringRef StartName = + Lexer::getImmediateMacroNameForDiagnostics(LocStart, SM, LO); + StringRef EndName = + Lexer::getImmediateMacroNameForDiagnostics(LocEnd, SM, LO); bool beginAndEndAreTheSameMacro = StartName.equals(EndName); bool partOfParentMacro = false; - if (ParentEx->getBeginLoc().isMacroID()) { + if (BOExpr->getBeginLoc().isMacroID()) { StringRef PName = Lexer::getImmediateMacroNameForDiagnostics( - ParentEx->getBeginLoc(), BRC.getSourceManager(), - BRC.getASTContext().getLangOpts()); + BOExpr->getBeginLoc(), SM, LO); partOfParentMacro = PName.equals(StartName); } @@ -2060,10 +2056,10 @@ SourceLocation Loc = LocStart; while (LocStart.isMacroID()) { Loc = LocStart; - LocStart = BRC.getSourceManager().getImmediateMacroCallerLoc(LocStart); + LocStart = SM.getImmediateMacroCallerLoc(LocStart); } - StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics( - Loc, BRC.getSourceManager(), BRC.getASTContext().getLangOpts()); + StringRef MacroName = + Lexer::getImmediateMacroNameForDiagnostics(Loc, SM, LO); // Return the macro name. Out << MacroName; @@ -2071,97 +2067,84 @@ } } - if (const auto *DR = dyn_cast(Ex)) { - const bool quotes = isa(DR->getDecl()); - if (quotes) { - Out << '\''; + if (const auto *DRE = dyn_cast(E)) { + if (const auto *VD = dyn_cast(DRE->getDecl())) { const LocationContext *LCtx = N->getLocationContext(); const ProgramState *state = N->getState().get(); - if (const MemRegion *R = state->getLValue(cast(DR->getDecl()), - LCtx).getAsRegion()) { - if (report.isInteresting(R)) - prunable = false; - else { - const ProgramState *state = N->getState().get(); - SVal V = state->getSVal(R); - if (report.isInteresting(V)) - prunable = false; + if (const MemRegion *MR = state->getLValue(VD, LCtx).getAsRegion()) { + if (R.isInteresting(MR)) { + Prunable = false; + } else { + SVal V = state->getSVal(MR); + if (R.isInteresting(V)) + Prunable = false; } } - } - Out << DR->getDecl()->getDeclName().getAsString(); - if (quotes) - Out << '\''; - return quotes; - } - if (const auto *IL = dyn_cast(Ex)) { - QualType OriginalTy = OriginalExpr->getType(); - if (OriginalTy->isPointerType()) { - if (IL->getValue() == 0) { - Out << "null"; - return false; - } - } - else if (OriginalTy->isObjCObjectPointerType()) { - if (IL->getValue() == 0) { - Out << "nil"; - return false; - } + Out << '\'' << VD->getName() << '\''; + return true; } - - Out << IL->getValue(); - return false; } - if (const auto *ME = dyn_cast(Ex)) { + if (const auto *ME = dyn_cast(E)) { if (!IsSameFieldName) Out << "field '" << ME->getMemberDecl()->getName() << '\''; else Out << '\'' << Lexer::getSourceText( - CharSourceRange::getTokenRange(Ex->getSourceRange()), - BRC.getSourceManager(), BRC.getASTContext().getLangOpts(), 0) + CharSourceRange::getTokenRange(E->getSourceRange()), SM, LO, 0) << '\''; + return true; + } + + // Value pretty-printing. + if (const auto *IL = dyn_cast(E)) { + if (Ty->isPointerType()) + Out << "null"; + else if (Ty->isObjCObjectPointerType()) + Out << "nil"; + else + Out << IL->getValue(); + + return false; } return false; } std::shared_ptr ConditionBRVisitor::VisitTrueTest( - const Expr *Cond, const BinaryOperator *BExpr, BugReporterContext &BRC, + const Expr *Cond, const BinaryOperator *BOExpr, BugReporterContext &BRC, BugReport &R, const ExplodedNode *N, bool TookTrue, bool IsAssuming) { 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'. + const Expr *LhsExpr = BOExpr->getLHS(); + const Expr *RhsExpr = BOExpr->getRHS(); bool IsSameFieldName = false; - if (const auto *LhsME = - dyn_cast(BExpr->getLHS()->IgnoreParenCasts())) - if (const auto *RhsME = - dyn_cast(BExpr->getRHS()->IgnoreParenCasts())) + if (const auto *LhsME = dyn_cast(LhsExpr->IgnoreImpCasts())) + if (const auto *RhsME = dyn_cast(RhsExpr->IgnoreImpCasts())) 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, IsSameFieldName); - const bool isVarRHS = patternMatch(BExpr->getRHS(), BExpr, OutRHS, BRC, R, - N, shouldPrune, IsSameFieldName); + const bool isVarLHS = printNameOrValue(LhsExpr, BOExpr, OutLHS, BRC, R, N, + shouldPrune, IsSameFieldName); + const bool isVarRHS = printNameOrValue(RhsExpr, BOExpr, OutRHS, BRC, R, N, + shouldPrune, IsSameFieldName); shouldInvert = !isVarLHS && isVarRHS; } - BinaryOperator::Opcode Op = BExpr->getOpcode(); + BinaryOperator::Opcode Op = BOExpr->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(), BRC, R, N, - TookTrue); + return VisitConditionVariable(LhsString, LhsExpr, BRC, R, N, TookTrue); } // For non-assignment operations, we require that we can understand Index: clang/test/Analysis/keychainAPI-diagnostic-visitor.m =================================================================== --- clang/test/Analysis/keychainAPI-diagnostic-visitor.m +++ clang/test/Analysis/keychainAPI-diagnostic-visitor.m @@ -27,7 +27,7 @@ char *x; st = SecKeychainItemCopyContent(2, ptr, ptr, &length, (void **)&bytes); // expected-note {{Data is allocated here}} x = bytes; - if (st == noErr) // expected-note {{Assuming 'st' is equal to noErr}} // expected-note{{Taking true branch}} + if (st == noErr) // expected-note {{Assuming the condition is true}} // expected-note{{Taking true branch}} x = bytes;; length++; // expected-warning {{Allocated data is not released}} // expected-note{{Allocated data is not released}}