r369589 - [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field
Kristof Umann via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 21 14:59:22 PDT 2019
Author: szelethus
Date: Wed Aug 21 14:59:22 2019
New Revision: 369589
URL: http://llvm.org/viewvc/llvm-project?rev=369589&view=rev
Log:
[analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field
Exactly what it says on the tin! Note that we're talking about interestingness
in general, hence this isn't a control-dependency-tracking specific patch.
Differential Revision: https://reviews.llvm.org/D65724
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h?rev=369589&r1=369588&r2=369589&view=diff
==============================================================================
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h Wed Aug 21 14:59:22 2019
@@ -211,8 +211,10 @@ public:
/// 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 {
Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=369589&r1=369588&r2=369589&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Aug 21 14:59:22 2019
@@ -180,21 +180,44 @@ static bool hasVisibleUpdate(const Explo
RLCV->getStore() == RightNode->getState()->getStore();
}
-static Optional<const llvm::APSInt *>
-getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
+static Optional<SVal> 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<DeclRefExpr>(CondVarExpr)) {
- if (const auto *VD = dyn_cast_or_null<VarDecl>(DRE->getDecl())) {
- SVal DeclSVal = State->getSVal(State->getLValue(VD, LCtx));
- if (auto DeclCI = DeclSVal.getAs<nonloc::ConcreteInt>())
- 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<DeclRefExpr>(CondVarExpr))
+ if (const auto *VD = dyn_cast<VarDecl>(DRE->getDecl()))
+ return State->getSVal(State->getLValue(VD, LCtx));
+
+ if (const auto *ME = dyn_cast<MemberExpr>(CondVarExpr))
+ if (const auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()))
+ if (auto FieldL = State->getSVal(ME, LCtx).getAs<Loc>())
+ return State->getRawSVal(*FieldL, FD->getType());
- return {};
+ return None;
+}
+
+static Optional<const llvm::APSInt *>
+getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
+
+ if (Optional<SVal> V = getSValForVar(CondVarExpr, N))
+ if (auto CI = V->getAs<nonloc::ConcreteInt>())
+ return &CI->getValue();
+ return None;
+}
+
+static bool isInterestingExpr(const Expr *E, const ExplodedNode *N,
+ const BugReport *B) {
+ if (Optional<SVal> V = getSValForVar(E, N))
+ return B->getInterestingnessKind(*V).hasValue();
+ return false;
}
/// \return name of the macro inside the location \p Loc.
@@ -2475,17 +2498,11 @@ PathDiagnosticPieceRef ConditionBRVisito
const LocationContext *LCtx = N->getLocationContext();
PathDiagnosticLocation Loc(CondVarExpr, BRC.getSourceManager(), LCtx);
+
auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
- if (const auto *DR = dyn_cast<DeclRefExpr>(CondVarExpr)) {
- if (const auto *VD = dyn_cast<VarDecl>(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;
}
@@ -2515,16 +2532,10 @@ PathDiagnosticPieceRef ConditionBRVisito
PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
auto event = std::make_shared<PathDiagnosticEventPiece>(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);
}
@@ -2555,7 +2566,10 @@ PathDiagnosticPieceRef ConditionBRVisito
if (!IsAssuming)
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
- return std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
+ auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
+ if (isInterestingExpr(ME, N, &report))
+ event->setPrunable(false);
+ return event;
}
bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, raw_ostream &Out,
@@ -2595,10 +2609,8 @@ bool ConditionBRVisitor::printValue(cons
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) {
Modified: cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp?rev=369589&r1=369588&r2=369589&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp (original)
+++ cfe/trunk/test/Analysis/track-control-dependency-conditions.cpp Wed Aug 21 14:59:22 2019
@@ -407,6 +407,91 @@ void f() {
}
} // 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 at -1{{{{^}}Returning from 'operator()'{{$}}}}
+
+ if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}}
+ // expected-note-re at -1{{{{^}}Taking true branch{{$}}}}
+ // debug-note-re at -2{{{{^}}Tracking condition 'flag'{{$}}}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -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 at -1{{{{^}}Taking false branch{{$}}}}
+ halt();
+ };
+
+ bar(flag); // tracking-note-re{{{{^}}Calling 'bar'{{$}}}}
+ // tracking-note-re at -1{{{{^}}Returning from 'bar'{{$}}}}
+ lambda(); // tracking-note-re{{{{^}}Calling 'operator()'{{$}}}}
+ // tracking-note-re at -1{{{{^}}Returning from 'operator()'{{$}}}}
+
+ if (flag) // expected-note-re{{{{^}}Assuming 'flag' is not equal to 0{{$}}}}
+ // expected-note-re at -1{{{{^}}Taking true branch{{$}}}}
+ // debug-note-re at -2{{{{^}}Tracking condition 'flag'{{$}}}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -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 at -1{{{{^}}Taking false branch{{$}}}}
+ halt();
+ };
+
+ bar(flag); // tracking-note-re{{{{^}}Calling 'bar'{{$}}}}
+ // tracking-note-re at -1{{{{^}}Returning from 'bar'{{$}}}}
+ lambda(); // tracking-note-re{{{{^}}Calling 'operator()'{{$}}}}
+ // tracking-note-re at -1{{{{^}}Returning from 'operator()'{{$}}}}
+
+ if (flag) // expected-note-re{{{{^}}'flag' is not equal to 0{{$}}}}
+ // expected-note-re at -1{{{{^}}Taking true branch{{$}}}}
+ // debug-note-re at -2{{{{^}}Tracking condition 'flag'}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -1{{Dereference of null pointer}}
+}
+} // end of namespace condition_lambda_capture_by_reference_assumption
+
namespace collapse_point_not_in_condition {
[[noreturn]] void halt();
@@ -471,6 +556,35 @@ void f6(int x) {
} // end of namespace dont_crash_on_nonlogical_binary_operator
+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 at -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 at -1{{{{^}}Returning from 'IntWrapper::check'{{$}}}}
+ if (i.b) // expected-note-re{{{{^}}Field 'b' is not equal to 0{{$}}}}
+ // expected-note-re at -1{{{{^}}Taking true branch{{$}}}}
+ // debug-note-re at -2{{{{^}}Tracking condition 'i.b'{{$}}}}
+ *x = 5; // expected-warning{{Dereference of null pointer}}
+ // expected-note at -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,
More information about the cfe-commits
mailing list