r176601 - [analyzer] Check for returning null references in ReturnUndefChecker.
Jordan Rose
jordan_rose at apple.com
Wed Mar 6 17:23:25 PST 2013
Author: jrose
Date: Wed Mar 6 19:23:25 2013
New Revision: 176601
URL: http://llvm.org/viewvc/llvm-project?rev=176601&view=rev
Log:
[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>
Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
cfe/trunk/test/Analysis/inlining/false-positive-suppression.cpp
cfe/trunk/test/Analysis/inlining/path-notes.cpp
cfe/trunk/test/Analysis/reference.cpp
Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp?rev=176601&r1=176600&r2=176601&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ReturnUndefChecker.cpp Wed Mar 6 19:23:25 2013
@@ -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;
+ }
- ExplodedNode *N = C.generateSink();
+ if (RT.isNull())
+ return;
+ 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);
+
+ 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"));
- C.emitReport(report);
+ emitBug(C, *BT_NullReference, RetE, bugreporter::getDerefExpr(RetE));
}
void ento::registerReturnUndefChecker(CheckerManager &mgr) {
Modified: cfe/trunk/test/Analysis/inlining/false-positive-suppression.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/false-positive-suppression.cpp?rev=176601&r1=176600&r2=176601&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/false-positive-suppression.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/false-positive-suppression.cpp Wed Mar 6 19:23:25 2013
@@ -112,4 +112,16 @@ namespace References {
// expected-warning at -2 {{Called C++ object pointer is null}}
#endif
}
+
+ SomeClass *getNull() {
+ return 0;
+ }
+
+ SomeClass &returnNullReference() {
+ SomeClass *x = getNull();
+ return *x;
+#ifndef SUPPRESSED
+ // expected-warning at -2 {{Returning null reference}}
+#endif
+ }
}
Modified: cfe/trunk/test/Analysis/inlining/path-notes.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/path-notes.cpp?rev=176601&r1=176600&r2=176601&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/inlining/path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/path-notes.cpp Wed Mar 6 19:23:25 2013
@@ -184,6 +184,15 @@ namespace ReturnZeroNote {
}
}
+
+int &returnNullReference() {
+ int *x = 0;
+ // expected-note at -1 {{'x' initialized to a null pointer value}}
+ return *x; // expected-warning{{Returning null reference}}
+ // expected-note at -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>
Modified: cfe/trunk/test/Analysis/reference.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/reference.cpp?rev=176601&r1=176600&r2=176601&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/reference.cpp (original)
+++ cfe/trunk/test/Analysis/reference.cpp Wed Mar 6 19:23:25 2013
@@ -135,6 +135,20 @@ void testFunctionPointerReturn(void *opa
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!
- }
}
More information about the cfe-commits
mailing list