forked from OSchip/llvm-project
[analyzer] Check for returning null references in ReturnUndefChecker.
Officially in the C++ standard, a null reference cannot exist. However, it's still very easy to create one: int &getNullRef() { int *p = 0; return *p; } We already check that binds to reference regions don't create null references. This patch checks that we don't create null references by returning, either. <rdar://problem/13364378> llvm-svn: 176601
This commit is contained in:
parent
6f09928d5b
commit
b41977f852
|
@ -24,9 +24,13 @@ using namespace clang;
|
|||
using namespace ento;
|
||||
|
||||
namespace {
|
||||
class ReturnUndefChecker :
|
||||
public Checker< check::PreStmt<ReturnStmt> > {
|
||||
mutable OwningPtr<BuiltinBug> BT;
|
||||
class ReturnUndefChecker : public Checker< check::PreStmt<ReturnStmt> > {
|
||||
mutable OwningPtr<BuiltinBug> BT_Undef;
|
||||
mutable OwningPtr<BuiltinBug> BT_NullReference;
|
||||
|
||||
void emitUndef(CheckerContext &C, const Expr *RetE) const;
|
||||
void checkReference(CheckerContext &C, const Expr *RetE,
|
||||
DefinedOrUnknownSVal RetVal) const;
|
||||
public:
|
||||
void checkPreStmt(const ReturnStmt *RS, CheckerContext &C) const;
|
||||
};
|
||||
|
@ -34,43 +38,75 @@ public:
|
|||
|
||||
void ReturnUndefChecker::checkPreStmt(const ReturnStmt *RS,
|
||||
CheckerContext &C) const {
|
||||
|
||||
const Expr *RetE = RS->getRetValue();
|
||||
if (!RetE)
|
||||
return;
|
||||
|
||||
if (!C.getState()->getSVal(RetE, C.getLocationContext()).isUndef())
|
||||
return;
|
||||
|
||||
// "return;" is modeled to evaluate to an UndefinedValue. Allow UndefinedValue
|
||||
// to be returned in functions returning void to support the following pattern:
|
||||
// void foo() {
|
||||
// return;
|
||||
// }
|
||||
// void test() {
|
||||
// return foo();
|
||||
// }
|
||||
SVal RetVal = C.getSVal(RetE);
|
||||
|
||||
const StackFrameContext *SFC = C.getStackFrame();
|
||||
QualType RT = CallEvent::getDeclaredResultType(SFC->getDecl());
|
||||
if (!RT.isNull() && RT->isSpecificBuiltinType(BuiltinType::Void))
|
||||
|
||||
if (RetVal.isUndef()) {
|
||||
// "return;" is modeled to evaluate to an UndefinedVal. Allow UndefinedVal
|
||||
// to be returned in functions returning void to support this pattern:
|
||||
// void foo() {
|
||||
// return;
|
||||
// }
|
||||
// void test() {
|
||||
// return foo();
|
||||
// }
|
||||
if (RT.isNull() || !RT->isVoidType())
|
||||
emitUndef(C, RetE);
|
||||
return;
|
||||
}
|
||||
|
||||
if (RT.isNull())
|
||||
return;
|
||||
|
||||
ExplodedNode *N = C.generateSink();
|
||||
if (RT->isReferenceType()) {
|
||||
checkReference(C, RetE, RetVal.castAs<DefinedOrUnknownSVal>());
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
static void emitBug(CheckerContext &C, BuiltinBug &BT, const Expr *RetE,
|
||||
const Expr *TrackingE = 0) {
|
||||
ExplodedNode *N = C.generateSink();
|
||||
if (!N)
|
||||
return;
|
||||
|
||||
if (!BT)
|
||||
BT.reset(new BuiltinBug("Garbage return value",
|
||||
"Undefined or garbage value returned to caller"));
|
||||
|
||||
BugReport *report =
|
||||
new BugReport(*BT, BT->getDescription(), N);
|
||||
|
||||
report->addRange(RetE->getSourceRange());
|
||||
bugreporter::trackNullOrUndefValue(N, RetE, *report);
|
||||
BugReport *Report = new BugReport(BT, BT.getDescription(), N);
|
||||
|
||||
C.emitReport(report);
|
||||
Report->addRange(RetE->getSourceRange());
|
||||
bugreporter::trackNullOrUndefValue(N, TrackingE ? TrackingE : RetE, *Report);
|
||||
|
||||
C.emitReport(Report);
|
||||
}
|
||||
|
||||
void ReturnUndefChecker::emitUndef(CheckerContext &C, const Expr *RetE) const {
|
||||
if (!BT_Undef)
|
||||
BT_Undef.reset(new BuiltinBug("Garbage return value",
|
||||
"Undefined or garbage value "
|
||||
"returned to caller"));
|
||||
emitBug(C, *BT_Undef, RetE);
|
||||
}
|
||||
|
||||
void ReturnUndefChecker::checkReference(CheckerContext &C, const Expr *RetE,
|
||||
DefinedOrUnknownSVal RetVal) const {
|
||||
ProgramStateRef StNonNull, StNull;
|
||||
llvm::tie(StNonNull, StNull) = C.getState()->assume(RetVal);
|
||||
|
||||
if (StNonNull) {
|
||||
// Going forward, assume the location is non-null.
|
||||
C.addTransition(StNonNull);
|
||||
return;
|
||||
}
|
||||
|
||||
// The return value is known to be null. Emit a bug report.
|
||||
if (!BT_NullReference)
|
||||
BT_NullReference.reset(new BuiltinBug("Returning null reference"));
|
||||
|
||||
emitBug(C, *BT_NullReference, RetE, bugreporter::getDerefExpr(RetE));
|
||||
}
|
||||
|
||||
void ento::registerReturnUndefChecker(CheckerManager &mgr) {
|
||||
|
|
|
@ -110,6 +110,18 @@ namespace References {
|
|||
object->doSomething();
|
||||
#ifndef SUPPRESSED
|
||||
// expected-warning@-2 {{Called C++ object pointer is null}}
|
||||
#endif
|
||||
}
|
||||
|
||||
SomeClass *getNull() {
|
||||
return 0;
|
||||
}
|
||||
|
||||
SomeClass &returnNullReference() {
|
||||
SomeClass *x = getNull();
|
||||
return *x;
|
||||
#ifndef SUPPRESSED
|
||||
// expected-warning@-2 {{Returning null reference}}
|
||||
#endif
|
||||
}
|
||||
}
|
||||
|
|
|
@ -184,6 +184,15 @@ namespace ReturnZeroNote {
|
|||
}
|
||||
}
|
||||
|
||||
|
||||
int &returnNullReference() {
|
||||
int *x = 0;
|
||||
// expected-note@-1 {{'x' initialized to a null pointer value}}
|
||||
return *x; // expected-warning{{Returning null reference}}
|
||||
// expected-note@-1 {{Returning null reference}}
|
||||
}
|
||||
|
||||
|
||||
// CHECK: <key>diagnostics</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
|
@ -3214,4 +3223,113 @@ namespace ReturnZeroNote {
|
|||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>path</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>kind</key><string>event</string>
|
||||
// CHECK-NEXT: <key>location</key>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>189</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <key>ranges</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>189</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>189</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>8</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: <key>depth</key><integer>0</integer>
|
||||
// CHECK-NEXT: <key>extended_message</key>
|
||||
// CHECK-NEXT: <string>'x' initialized to a null pointer value</string>
|
||||
// CHECK-NEXT: <key>message</key>
|
||||
// CHECK-NEXT: <string>'x' initialized to a null pointer value</string>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>kind</key><string>control</string>
|
||||
// CHECK-NEXT: <key>edges</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>start</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>189</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>189</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>5</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: <key>end</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>8</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>kind</key><string>event</string>
|
||||
// CHECK-NEXT: <key>location</key>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <key>ranges</key>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <array>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>10</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>11</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: <key>depth</key><integer>0</integer>
|
||||
// CHECK-NEXT: <key>extended_message</key>
|
||||
// CHECK-NEXT: <string>Returning null reference</string>
|
||||
// CHECK-NEXT: <key>message</key>
|
||||
// CHECK-NEXT: <string>Returning null reference</string>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
// CHECK-NEXT: <key>description</key><string>Returning null reference</string>
|
||||
// CHECK-NEXT: <key>category</key><string>Logic error</string>
|
||||
// CHECK-NEXT: <key>type</key><string>Returning null reference</string>
|
||||
// CHECK-NEXT: <key>issue_context_kind</key><string>function</string>
|
||||
// CHECK-NEXT: <key>issue_context</key><string>returnNullReference</string>
|
||||
// CHECK-NEXT: <key>issue_hash</key><string>3</string>
|
||||
// CHECK-NEXT: <key>location</key>
|
||||
// CHECK-NEXT: <dict>
|
||||
// CHECK-NEXT: <key>line</key><integer>191</integer>
|
||||
// CHECK-NEXT: <key>col</key><integer>3</integer>
|
||||
// CHECK-NEXT: <key>file</key><integer>0</integer>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </dict>
|
||||
// CHECK-NEXT: </array>
|
||||
|
|
|
@ -135,6 +135,20 @@ void testFunctionPointerReturn(void *opaque) {
|
|||
clang_analyzer_eval(x == 42); // expected-warning{{TRUE}}
|
||||
}
|
||||
|
||||
int &testReturnNullReference() {
|
||||
int *x = 0;
|
||||
return *x; // expected-warning{{Returning null reference}}
|
||||
}
|
||||
|
||||
char &refFromPointer() {
|
||||
return *ptr();
|
||||
}
|
||||
|
||||
void testReturnReference() {
|
||||
clang_analyzer_eval(ptr() == 0); // expected-warning{{UNKNOWN}}
|
||||
clang_analyzer_eval(&refFromPointer() == 0); // expected-warning{{FALSE}}
|
||||
}
|
||||
|
||||
|
||||
// ------------------------------------
|
||||
// False negatives
|
||||
|
@ -147,9 +161,4 @@ namespace rdar11212286 {
|
|||
B *x = 0;
|
||||
return *x; // should warn here!
|
||||
}
|
||||
|
||||
B &testRef() {
|
||||
B *x = 0;
|
||||
return *x; // should warn here!
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue