[analyzer] ConditionBRVisitor: MemberExpr support
Summary: -
Reviewers: NoQ, george.karpenkov
Reviewed By: NoQ
Subscribers: cfe-commits, xazax.hun, baloghadamsoftware, szepet, a.sidorin,
mikhail.ramalho, Szelethus, donat.nagy, dkrupp
Tags: #clang
Differential Revision: https://reviews.llvm.org/D58206
llvm-svn: 362026
This commit is contained in:
Csaba Dabis
parent
9942a996d9
commit
d1f0ec3f64
|
|
@ -202,6 +202,11 @@ public:
|
|||
BugReporterContext &BRC, BugReport &R, const ExplodedNode *N,
|
||||
bool TookTrue, bool IsAssuming);
|
||||
|
||||
std::shared_ptr<PathDiagnosticPiece>
|
||||
VisitTrueTest(const Expr *Cond, const MemberExpr *ME, BugReporterContext &BRC,
|
||||
BugReport &R, const ExplodedNode *N, bool TookTrue,
|
||||
bool IsAssuming);
|
||||
|
||||
std::shared_ptr<PathDiagnosticPiece>
|
||||
VisitConditionVariable(StringRef LhsString, const Expr *CondVarExpr,
|
||||
BugReporterContext &BRC, BugReport &R,
|
||||
|
|
@ -225,7 +230,8 @@ public:
|
|||
BugReporterContext &BRC,
|
||||
BugReport &R,
|
||||
const ExplodedNode *N,
|
||||
Optional<bool> &prunable);
|
||||
Optional<bool> &prunable,
|
||||
bool IsSameFieldName);
|
||||
|
||||
static bool isPieceMessageGeneric(const PathDiagnosticPiece *Piece);
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1991,6 +1991,11 @@ ConditionBRVisitor::VisitTrueTest(const Expr *Cond, BugReporterContext &BRC,
|
|||
BRC, R, N, TookTrueTmp, IsAssuming))
|
||||
return P;
|
||||
break;
|
||||
case Stmt::MemberExprClass:
|
||||
if (auto P = VisitTrueTest(Cond, cast<MemberExpr>(CondTmp),
|
||||
BRC, R, N, TookTrueTmp, IsAssuming))
|
||||
return P;
|
||||
break;
|
||||
case Stmt::UnaryOperatorClass: {
|
||||
const auto *UO = cast<UnaryOperator>(CondTmp);
|
||||
if (UO->getOpcode() == UO_LNot) {
|
||||
|
|
@ -2025,7 +2030,8 @@ bool ConditionBRVisitor::patternMatch(const Expr *Ex,
|
|||
BugReporterContext &BRC,
|
||||
BugReport &report,
|
||||
const ExplodedNode *N,
|
||||
Optional<bool> &prunable) {
|
||||
Optional<bool> &prunable,
|
||||
bool IsSameFieldName) {
|
||||
const Expr *OriginalExpr = Ex;
|
||||
Ex = Ex->IgnoreParenCasts();
|
||||
|
||||
|
|
@ -2091,6 +2097,17 @@ bool ConditionBRVisitor::patternMatch(const Expr *Ex,
|
|||
return false;
|
||||
}
|
||||
|
||||
if (const auto *ME = dyn_cast<MemberExpr>(Ex)) {
|
||||
if (!IsSameFieldName)
|
||||
Out << "field '" << ME->getMemberDecl()->getName() << '\'';
|
||||
else
|
||||
Out << '\''
|
||||
<< Lexer::getSourceText(
|
||||
CharSourceRange::getTokenRange(Ex->getSourceRange()),
|
||||
BRC.getSourceManager(), BRC.getASTContext().getLangOpts(), 0)
|
||||
<< '\'';
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -2100,13 +2117,23 @@ std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTrueTest(
|
|||
bool shouldInvert = false;
|
||||
Optional<bool> 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'.
|
||||
bool IsSameFieldName = false;
|
||||
if (const auto *LhsME =
|
||||
dyn_cast<MemberExpr>(BExpr->getLHS()->IgnoreParenCasts()))
|
||||
if (const auto *RhsME =
|
||||
dyn_cast<MemberExpr>(BExpr->getRHS()->IgnoreParenCasts()))
|
||||
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);
|
||||
const bool isVarRHS = patternMatch(BExpr->getRHS(), BExpr, OutRHS,
|
||||
BRC, R, N, shouldPrune);
|
||||
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);
|
||||
|
||||
shouldInvert = !isVarLHS && isVarRHS;
|
||||
}
|
||||
|
|
@ -2170,11 +2197,15 @@ std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTrueTest(
|
|||
const LocationContext *LCtx = N->getLocationContext();
|
||||
PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
|
||||
|
||||
// Convert 'field ...' to 'Field ...' if it is a MemberExpr.
|
||||
std::string Message = Out.str();
|
||||
Message[0] = toupper(Message[0]);
|
||||
|
||||
// If we know the value create a pop-up note.
|
||||
if (!IsAssuming)
|
||||
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
|
||||
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Message);
|
||||
|
||||
auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
|
||||
auto event = std::make_shared<PathDiagnosticEventPiece>(Loc, Message);
|
||||
if (shouldPrune.hasValue())
|
||||
event->setPrunable(shouldPrune.getValue());
|
||||
return event;
|
||||
|
|
@ -2246,6 +2277,30 @@ std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTrueTest(
|
|||
return std::move(event);
|
||||
}
|
||||
|
||||
std::shared_ptr<PathDiagnosticPiece> ConditionBRVisitor::VisitTrueTest(
|
||||
const Expr *Cond, const MemberExpr *ME, BugReporterContext &BRC,
|
||||
BugReport &report, const ExplodedNode *N, bool TookTrue, bool IsAssuming) {
|
||||
SmallString<256> Buf;
|
||||
llvm::raw_svector_ostream Out(Buf);
|
||||
|
||||
Out << (IsAssuming ? "Assuming field '" : "Field '")
|
||||
<< ME->getMemberDecl()->getName() << "' is ";
|
||||
|
||||
if (!printValue(ME, Out, N, TookTrue, IsAssuming))
|
||||
return nullptr;
|
||||
|
||||
const LocationContext *LCtx = N->getLocationContext();
|
||||
PathDiagnosticLocation Loc(Cond, BRC.getSourceManager(), LCtx);
|
||||
if (!Loc.isValid() || !Loc.asLocation().isValid())
|
||||
return nullptr;
|
||||
|
||||
// If we know the value create a pop-up note.
|
||||
if (!IsAssuming)
|
||||
return std::make_shared<PathDiagnosticPopUpPiece>(Loc, Out.str());
|
||||
|
||||
return std::make_shared<PathDiagnosticEventPiece>(Loc, Out.str());
|
||||
}
|
||||
|
||||
bool ConditionBRVisitor::printValue(const Expr *CondVarExpr, raw_ostream &Out,
|
||||
const ExplodedNode *N, bool TookTrue,
|
||||
bool IsAssuming) {
|
||||
|
|
|
|||
|
|
@ -22227,6 +22227,68 @@
|
|||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
</array>
|
||||
<key>end</key>
|
||||
<array>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
</array>
|
||||
</dict>
|
||||
</array>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>kind</key><string>pop-up</string>
|
||||
<key>location</key>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
<key>ranges</key>
|
||||
<array>
|
||||
<array>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>16</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
</array>
|
||||
</array>
|
||||
<key>extended_message</key>
|
||||
<string>Field 'b' is equal to 2</string>
|
||||
<key>message</key>
|
||||
<string>Field 'b' is equal to 2</string>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>kind</key><string>control</string>
|
||||
<key>edges</key>
|
||||
<array>
|
||||
<dict>
|
||||
<key>start</key>
|
||||
<array>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>line</key><integer>587</integer>
|
||||
<key>col</key><integer>11</integer>
|
||||
<key>file</key><integer>0</integer>
|
||||
</dict>
|
||||
</array>
|
||||
<key>end</key>
|
||||
<array>
|
||||
<dict>
|
||||
|
|
|
|||
|
|
@ -165,9 +165,9 @@
|
|||
</array>
|
||||
<key>depth</key><integer>0</integer>
|
||||
<key>extended_message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'x' is null</string>
|
||||
<key>message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'x' is null</string>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>kind</key><string>control</string>
|
||||
|
|
@ -454,9 +454,9 @@
|
|||
</array>
|
||||
<key>depth</key><integer>0</integer>
|
||||
<key>extended_message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'x' is null</string>
|
||||
<key>message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'x' is null</string>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>kind</key><string>control</string>
|
||||
|
|
|
|||
|
|
@ -15,8 +15,8 @@ void test(struct S syz, int *pp) {
|
|||
|
||||
struct S *ps = &syz;
|
||||
if (ps->x)
|
||||
//expected-note@-1{{Taking false branch}}
|
||||
//expected-note@-2{{Assuming pointer value is null}}
|
||||
//expected-note@-1{{Assuming field 'x' is null}}
|
||||
//expected-note@-2{{Taking false branch}}
|
||||
|
||||
m++;
|
||||
|
||||
|
|
@ -30,8 +30,8 @@ void testTrackConstraintBRVisitorIsTrackingTurnedOn(struct S syz, int *pp) {
|
|||
|
||||
struct S *ps = &syz;
|
||||
if (ps->x)
|
||||
//expected-note@-1{{Taking false branch}}
|
||||
//expected-note@-2{{Assuming pointer value is null}}
|
||||
//expected-note@-1{{Assuming field 'x' is null}}
|
||||
//expected-note@-2{{Taking false branch}}
|
||||
|
||||
m++;
|
||||
int *p = syz.x; //expected-note {{'p' initialized to a null pointer value}}
|
||||
|
|
|
|||
|
|
@ -16,10 +16,11 @@ struct smart_ptr {
|
|||
S *s;
|
||||
smart_ptr(S *);
|
||||
S *get() {
|
||||
return (x || 0) ? nullptr : s; // expected-note{{Left side of '||' is false}}
|
||||
// expected-note@-1{{'?' condition is false}}
|
||||
// expected-warning@-2{{Use of memory after it is freed}}
|
||||
// expected-note@-3{{Use of memory after it is freed}}
|
||||
return (x || 0) ? nullptr : s; // expected-note{{Field 'x' is 0}}
|
||||
// expected-note@-1{{Left side of '||' is false}}
|
||||
// expected-note@-2{{'?' condition is false}}
|
||||
// expected-warning@-3{{Use of memory after it is freed}}
|
||||
// expected-note@-4{{Use of memory after it is freed}}
|
||||
}
|
||||
};
|
||||
|
||||
|
|
|
|||
|
|
@ -4271,9 +4271,9 @@
|
|||
</array>
|
||||
<key>depth</key><integer>0</integer>
|
||||
<key>extended_message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'arr' is null</string>
|
||||
<key>message</key>
|
||||
<string>Assuming pointer value is null</string>
|
||||
<string>Assuming field 'arr' is null</string>
|
||||
</dict>
|
||||
<dict>
|
||||
<key>kind</key><string>control</string>
|
||||
|
|
|
|||
|
|
@ -231,7 +231,7 @@ struct Owner {
|
|||
};
|
||||
|
||||
void Owner::testGetDerefExprOnMemberExprWithADot() {
|
||||
if (arr) // expected-note {{Assuming pointer value is null}}
|
||||
if (arr) // expected-note {{Assuming field 'arr' is null}}
|
||||
// expected-note@-1 {{Taking false branch}}
|
||||
;
|
||||
arr[1].x = 1; //expected-warning {{Dereference of null pointer}}
|
||||
|
|
|
|||
|
|
@ -19,7 +19,7 @@ void c::f(B &g, int &i) {
|
|||
// expected-note@-1{{Array access (via field 'd') results in a null pointer dereference}}
|
||||
B h, a; // expected-note{{Value assigned to 'h.d'}}
|
||||
a.d == __null; // expected-note{{Assuming the condition is true}}
|
||||
a.d != h.d; // expected-note{{Assuming pointer value is null}}
|
||||
a.d != h.d; // expected-note{{Assuming 'a.d' is equal to 'h.d'}}
|
||||
f(h, b); // expected-note{{Calling 'c::f'}}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -601,16 +601,18 @@ void test_smart_ptr_uaf() {
|
|||
{
|
||||
OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr<OSObject>'}}
|
||||
// expected-note@-1{{Returning from constructor for 'smart_ptr<OSObject>'}}
|
||||
// expected-note@os_smart_ptr.h:13{{Field 'pointer' is non-null}}
|
||||
// expected-note@os_smart_ptr.h:13{{Taking true branch}}
|
||||
// expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}}
|
||||
// expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}}
|
||||
// expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}}
|
||||
} // expected-note{{Calling '~smart_ptr'}}
|
||||
// expected-note@os_smart_ptr.h:35{{Field 'pointer' is non-null}}
|
||||
// expected-note@os_smart_ptr.h:35{{Taking true branch}}
|
||||
// expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}}
|
||||
// expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}}
|
||||
// expected-note@os_smart_ptr.h:36{{Returning from 'smart_ptr::_release'}}
|
||||
// expected-note@-5{{Returning from '~smart_ptr'}}
|
||||
// expected-note@-6{{Returning from '~smart_ptr'}}
|
||||
obj->release(); // expected-note{{Object released}}
|
||||
obj->release(); // expected-warning{{Reference-counted object is used after it is released}}
|
||||
// expected-note@-1{{Reference-counted object is used after it is released}}
|
||||
|
|
@ -621,16 +623,18 @@ void test_smart_ptr_leak() {
|
|||
{
|
||||
OSObjectPtr p(obj); // expected-note{{Calling constructor for 'smart_ptr<OSObject>'}}
|
||||
// expected-note@-1{{Returning from constructor for 'smart_ptr<OSObject>'}}
|
||||
// expected-note@os_smart_ptr.h:13{{Field 'pointer' is non-null}}
|
||||
// expected-note@os_smart_ptr.h:13{{Taking true branch}}
|
||||
// expected-note@os_smart_ptr.h:14{{Calling 'smart_ptr::_retain'}}
|
||||
// expected-note@os_smart_ptr.h:71{{Reference count incremented. The object now has a +2 retain count}}
|
||||
// expected-note@os_smart_ptr.h:14{{Returning from 'smart_ptr::_retain'}}
|
||||
} // expected-note{{Calling '~smart_ptr'}}
|
||||
// expected-note@os_smart_ptr.h:35{{Field 'pointer' is non-null}}
|
||||
// expected-note@os_smart_ptr.h:35{{Taking true branch}}
|
||||
// expected-note@os_smart_ptr.h:36{{Calling 'smart_ptr::_release'}}
|
||||
// expected-note@os_smart_ptr.h:76{{Reference count decremented. The object now has a +1 retain count}}
|
||||
// expected-note@os_smart_ptr.h:36{{Returning from 'smart_ptr::_release'}}
|
||||
// expected-note@-5{{Returning from '~smart_ptr'}}
|
||||
// expected-note@-6{{Returning from '~smart_ptr'}}
|
||||
} // expected-warning{{Potential leak of an object stored into 'obj'}}
|
||||
// expected-note@-1{{Object leaked: object allocated and stored into 'obj' is not referenced later in this execution path and has a retain count of +1}}
|
||||
|
||||
|
|
|
|||
|
|
@ -164,7 +164,7 @@ void PR14765_test() {
|
|||
// expected-note@-1{{TRUE}}
|
||||
|
||||
testObj->origin = makePoint(0.0, 0.0);
|
||||
if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
|
||||
if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}}
|
||||
// expected-note@-1{{Taking false branch}}
|
||||
|
||||
// FIXME: Assigning to 'testObj->origin' kills the default binding for the
|
||||
|
|
@ -219,13 +219,13 @@ void PR14765_test_int() {
|
|||
// expected-note@-1{{TRUE}}
|
||||
|
||||
testObj->origin = makeIntPoint(1, 2);
|
||||
if (testObj->size > 0) { ; } // expected-note{{Assuming the condition is false}}
|
||||
if (testObj->size > 0) { ; } // expected-note{{Assuming field 'size' is <= 0}}
|
||||
// expected-note@-1{{Taking false branch}}
|
||||
// expected-note@-2{{Assuming the condition is false}}
|
||||
// expected-note@-2{{Assuming field 'size' is <= 0}}
|
||||
// expected-note@-3{{Taking false branch}}
|
||||
// expected-note@-4{{Assuming the condition is false}}
|
||||
// expected-note@-4{{Assuming field 'size' is <= 0}}
|
||||
// expected-note@-5{{Taking false branch}}
|
||||
// expected-note@-6{{Assuming the condition is false}}
|
||||
// expected-note@-6{{Assuming field 'size' is <= 0}}
|
||||
// expected-note@-7{{Taking false branch}}
|
||||
|
||||
// FIXME: Assigning to 'testObj->origin' kills the default binding for the
|
||||
|
|
@ -321,9 +321,12 @@ void testSmallStructInLargerStruct() {
|
|||
// expected-note@-1{{TRUE}}
|
||||
|
||||
testObj->origin = makeIntPoint2D(1, 2);
|
||||
if (testObj->size > 0) { ; } // expected-note{{Taking false branch}}
|
||||
if (testObj->size > 0) { ; } // expected-note{{Field 'size' is <= 0}}
|
||||
// expected-note@-1{{Taking false branch}}
|
||||
// expected-note@-2{{Taking false branch}}
|
||||
// expected-note@-2{{Field 'size' is <= 0}}
|
||||
// expected-note@-3{{Taking false branch}}
|
||||
// expected-note@-4{{Field 'size' is <= 0}}
|
||||
// expected-note@-5{{Taking false branch}}
|
||||
|
||||
clang_analyzer_eval(testObj->size == 0); // expected-warning{{TRUE}}
|
||||
// expected-note@-1{{TRUE}}
|
||||
|
|
|
|||
Loading…
Reference in New Issue