Szelethus updated this revision to Diff 216283.
Szelethus added a comment.
- Add the `FIXME`
- Cascade test changes from D65575 <https://reviews.llvm.org/D65575>
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65724/new/
https://reviews.llvm.org/D65724
Files:
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
clang/test/Analysis/track-control-dependency-conditions.cpp
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,
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<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.
@@ -2511,17 +2534,11 @@
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;
}
@@ -2551,16 +2568,10 @@
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);
}
@@ -2591,7 +2602,10 @@
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,
@@ -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/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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
