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 @@ -221,8 +221,10 @@ /// Visitor that tries to report interesting diagnostics from conditions. class ConditionBRVisitor final : public BugReporterVisitor { // FIXME: constexpr initialization isn't supported by MSVC2013. - static const char *const GenericTrueMessage; - static const char *const GenericFalseMessage; + constexpr static llvm::StringLiteral GenericTrueMessage = + "Assuming the condition is true"; + constexpr static llvm::StringLiteral GenericFalseMessage = + "Assuming the condition is false"; public: void Profile(llvm::FoldingSetNodeID &ID) const override { Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp =================================================================== --- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp +++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp @@ -180,21 +180,44 @@ RLCV->getStore() == RightNode->getState()->getStore(); } -static Optional -getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) { +static Optional getSValForVar(const Expr *CondVarExpr, + const ExplodedNode *N) { ProgramStateRef State = N->getState(); const LocationContext *LCtx = N->getLocationContext(); + assert(CondVarExpr); + CondVarExpr = CondVarExpr->IgnoreImpCasts(); + // The declaration of the value may rely on a pointer so take its l-value. - if (const auto *DRE = dyn_cast_or_null(CondVarExpr)) { - if (const auto *VD = dyn_cast_or_null(DRE->getDecl())) { - SVal DeclSVal = State->getSVal(State->getLValue(VD, LCtx)); - if (auto DeclCI = DeclSVal.getAs()) - return &DeclCI->getValue(); - } - } + // FIXME: As seen in VisitCommonDeclRefExpr, sometimes DeclRefExpr may + // evaluate to a FieldRegion when it refers to a declaration of a lambda + // capture variable. We most likely need to duplicate that logic here. + if (const auto *DRE = dyn_cast(CondVarExpr)) + if (const auto *VD = dyn_cast(DRE->getDecl())) + return State->getSVal(State->getLValue(VD, LCtx)); + + if (const auto *ME = dyn_cast(CondVarExpr)) + if (const auto *FD = dyn_cast(ME->getMemberDecl())) + if (auto FieldL = State->getSVal(ME, LCtx).getAs()) + return State->getRawSVal(*FieldL, FD->getType()); - return {}; + return None; +} + +static Optional +getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) { + + if (Optional V = getSValForVar(CondVarExpr, N)) + if (auto CI = V->getAs()) + return &CI->getValue(); + return None; +} + +static bool isInterestingExpr(const Expr *E, const ExplodedNode *N, + const BugReport *B) { + if (Optional V = getSValForVar(E, N)) + return B->getInterestingnessKind(*V).hasValue(); + return false; } /// \return name of the macro inside the location \p Loc. @@ -2511,17 +2534,11 @@ const LocationContext *LCtx = N->getLocationContext(); PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LCtx); + auto event = std::make_shared(Loc, Out.str()); - if (const auto *DR = dyn_cast(CondVarExpr)) { - if (const auto *VD = dyn_cast(DR->getDecl())) { - const ProgramState *state = N->getState().get(); - if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) { - if (report.isInteresting(R)) - event->setPrunable(false); - } - } - } + if (isInterestingExpr(CondVarExpr, N, &report)) + event->setPrunable(false); return event; } @@ -2551,16 +2568,10 @@ PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx); auto event = std::make_shared(Loc, Out.str()); - const ProgramState *state = N->getState().get(); - if (const MemRegion *R = state->getLValue(VD, LCtx).getAsRegion()) { - if (report.isInteresting(R)) - event->setPrunable(false); - else { - SVal V = state->getSVal(R); - if (report.isInteresting(V)) - event->setPrunable(false); - } - } + + if (isInterestingExpr(DRE, N, &report)) + event->setPrunable(false); + return std::move(event); } @@ -2591,7 +2602,10 @@ if (!IsAssuming) return std::make_shared(Loc, Out.str()); - return std::make_shared(Loc, Out.str()); + auto event = std::make_shared(Loc, Out.str()); + if (isInterestingExpr(ME, N, &report)) + event->setPrunable(false); + return event; } bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, raw_ostream &Out, @@ -2631,10 +2645,8 @@ return true; } -const char *const ConditionBRVisitor::GenericTrueMessage = - "Assuming the condition is true"; -const char *const ConditionBRVisitor::GenericFalseMessage = - "Assuming the condition is false"; +constexpr llvm::StringLiteral ConditionBRVisitor::GenericTrueMessage; +constexpr llvm::StringLiteral ConditionBRVisitor::GenericFalseMessage; bool ConditionBRVisitor::isPieceMessageGeneric( const PathDiagnosticPiece *Piece) { Index: clang/test/Analysis/track-control-dependency-conditions.cpp =================================================================== --- clang/test/Analysis/track-control-dependency-conditions.cpp +++ clang/test/Analysis/track-control-dependency-conditions.cpp @@ -407,6 +407,91 @@ } } // end of namespace condition_written_in_nested_stackframe_before_assignment +namespace condition_lambda_capture_by_reference_last_write { +int getInt(); + +[[noreturn]] void halt(); + +void f(int flag) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + auto lambda = [&flag]() { + flag = getInt(); // tracking-note-re{{{{^}}Value assigned to 'flag', which participates in a condition later{{$}}}} + }; + + lambda(); // tracking-note-re{{{{^}}Calling 'operator()'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'operator()'{{$}}}} + + if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace condition_lambda_capture_by_reference_last_write + +namespace condition_lambda_capture_by_value_assumption { +int getInt(); + +[[noreturn]] void halt(); + +void bar(int &flag) { + flag = getInt(); // tracking-note-re{{{{^}}Value assigned to 'flag', which participates in a condition later{{$}}}} +} + +void f(int flag) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + auto lambda = [flag]() { + if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} + halt(); + }; + + bar(flag); // tracking-note-re{{{{^}}Calling 'bar'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'bar'{{$}}}} + lambda(); // tracking-note-re{{{{^}}Calling 'operator()'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'operator()'{{$}}}} + + if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace condition_lambda_capture_by_value_assumption + +namespace condition_lambda_capture_by_reference_assumption { +int getInt(); + +[[noreturn]] void halt(); + +void bar(int &flag) { + flag = getInt(); // tracking-note-re{{{{^}}Value assigned to 'flag', which participates in a condition later{{$}}}} +} + +void f(int flag) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + auto lambda = [&flag]() { + if (!flag) // tracking-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}} + // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} + halt(); + }; + + bar(flag); // tracking-note-re{{{{^}}Calling 'bar'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'bar'{{$}}}} + lambda(); // tracking-note-re{{{{^}}Calling 'operator()'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'operator()'{{$}}}} + + if (flag) // expected-note-re{{{{^}}'flag' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'flag'}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} +} // end of namespace condition_lambda_capture_by_reference_assumption + namespace collapse_point_not_in_condition { [[noreturn]] void halt(); @@ -459,6 +544,35 @@ } // end of namespace unimportant_write_before_collapse_point +namespace collapse_point_not_in_condition_as_field { + +[[noreturn]] void halt(); +struct IntWrapper { + int b; + IntWrapper(); + + void check() { + if (!b) // tracking-note-re{{{{^}}Assuming field 'b' is not equal to 0{{$}}}} + // tracking-note-re@-1{{{{^}}Taking false branch{{$}}}} + halt(); + return; + } +}; + +void f(IntWrapper i) { + int *x = 0; // expected-note-re{{{{^}}'x' initialized to a null pointer value{{$}}}} + + i.check(); // tracking-note-re{{{{^}}Calling 'IntWrapper::check'{{$}}}} + // tracking-note-re@-1{{{{^}}Returning from 'IntWrapper::check'{{$}}}} + if (i.b) // expected-note-re{{{{^}}Field 'b' is not equal to 0{{$}}}} + // expected-note-re@-1{{{{^}}Taking true branch{{$}}}} + // debug-note-re@-2{{{{^}}Tracking condition 'i.b'{{$}}}} + *x = 5; // expected-warning{{Dereference of null pointer}} + // expected-note@-1{{Dereference of null pointer}} +} + +} // end of namespace collapse_point_not_in_condition_as_field + namespace dont_track_assertlike_conditions { extern void __assert_fail(__const char *__assertion, __const char *__file,