[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

llvm-svn: 369589
This commit is contained in:
Kristof Umann 2019-08-21 21:59:22 +00:00
parent d98f975089
commit 49ac7ece16
3 changed files with 165 additions and 37 deletions

View File

@ -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 {

View File

@ -180,21 +180,44 @@ static bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal,
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();
// 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();
}
}
assert(CondVarExpr);
CondVarExpr = CondVarExpr->IgnoreImpCasts();
return {};
// The declaration of the value may rely on a pointer so take its l-value.
// 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 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 ConditionBRVisitor::VisitConditionVariable(
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 ConditionBRVisitor::VisitTrueTest(
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 ConditionBRVisitor::VisitTrueTest(
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(const Expr *CondVarExpr, raw_ostream &Out,
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) {

View File

@ -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@-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();
@ -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@-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,