[analyzer] Do not invalidate arguments when the parameter's
type is a pointer to const. (radar://10595327)
The regions corresponding to the pointer and reference arguments to
a function get invalidated by the calls since a function call can
possibly modify the pointed to data. With this change, we are not going
to invalidate the data if the argument is a pointer to const. This
change makes the analyzer more optimistic in reporting errors.
(Support for C, C++ and Obj C)
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@147002 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
index add3479..ed589f9 100644
--- a/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
+++ b/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
@@ -246,6 +246,9 @@
SVal getCXXCallee() const;
SVal getInstanceMessageReceiver(const LocationContext *LC) const;
+ /// Get the declaration of the function or method.
+ const Decl *getDecl() const;
+
unsigned getNumArgs() const {
if (!CallE)
return Msg.getNumArgs();
diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index dc49c85..46ebd84 100644
--- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -63,6 +63,46 @@
B.generateNode(state);
}
+static bool isPointerToConst(const ParmVarDecl *ParamDecl) {
+ QualType PointeeTy = ParamDecl->getOriginalType()->getPointeeType();
+ if (PointeeTy != QualType() && PointeeTy.isConstQualified() &&
+ !PointeeTy->isAnyPointerType() && !PointeeTy->isReferenceType()) {
+ return true;
+ }
+ return false;
+}
+
+// Try to retrieve the function declaration and find the function parameter
+// types which are pointers/references to a non-pointer const.
+// We do not invalidate the corresponding argument regions.
+static void findPtrToConstParams(llvm::SmallSet<unsigned, 1> &PreserveArgs,
+ const CallOrObjCMessage &Call) {
+ const Decl *CallDecl = Call.getDecl();
+ if (!CallDecl)
+ return;
+
+ if (const FunctionDecl *FDecl = dyn_cast<FunctionDecl>(CallDecl)) {
+ for (unsigned Idx = 0, E = Call.getNumArgs(); Idx != E; ++Idx) {
+ if (FDecl && Idx < FDecl->getNumParams()) {
+ if (isPointerToConst(FDecl->getParamDecl(Idx)))
+ PreserveArgs.insert(Idx);
+ }
+ }
+ return;
+ }
+
+ if (const ObjCMethodDecl *MDecl = dyn_cast<ObjCMethodDecl>(CallDecl)) {
+ assert(MDecl->param_size() <= Call.getNumArgs());
+ unsigned Idx = 0;
+ for (clang::ObjCMethodDecl::param_const_iterator
+ I = MDecl->param_begin(), E = MDecl->param_end(); I != E; ++I, ++Idx) {
+ if (isPointerToConst(*I))
+ PreserveArgs.insert(Idx);
+ }
+ return;
+ }
+}
+
const ProgramState *
ExprEngine::invalidateArguments(const ProgramState *State,
const CallOrObjCMessage &Call,
@@ -84,13 +124,21 @@
} else if (Call.isFunctionCall()) {
// Block calls invalidate all captured-by-reference values.
- if (const MemRegion *Callee = Call.getFunctionCallee().getAsRegion()) {
+ SVal CalleeVal = Call.getFunctionCallee();
+ if (const MemRegion *Callee = CalleeVal.getAsRegion()) {
if (isa<BlockDataRegion>(Callee))
RegionsToInvalidate.push_back(Callee);
}
}
+ // Indexes of arguments whose values will be preserved by the call.
+ llvm::SmallSet<unsigned, 1> PreserveArgs;
+ findPtrToConstParams(PreserveArgs, Call);
+
for (unsigned idx = 0, e = Call.getNumArgs(); idx != e; ++idx) {
+ if (PreserveArgs.count(idx))
+ continue;
+
SVal V = Call.getArgSVal(idx);
// If we are passing a location wrapped as an integer, unwrap it and
@@ -104,7 +152,7 @@
// Invalidate the value of the variable passed by reference.
// Are we dealing with an ElementRegion? If the element type is
- // a basic integer type (e.g., char, int) and the underying region
+ // a basic integer type (e.g., char, int) and the underlying region
// is a variable region then strip off the ElementRegion.
// FIXME: We really need to think about this for the general case
// as sometimes we are reasoning about arrays and other times
@@ -115,7 +163,7 @@
// we'll leave it in for now until we have a systematic way of
// handling all of these cases. Eventually we need to come up
// with an interface to StoreManager so that this logic can be
- // approriately delegated to the respective StoreManagers while
+ // appropriately delegated to the respective StoreManagers while
// still allowing us to do checker-specific logic (e.g.,
// invalidating reference counts), probably via callbacks.
if (ER->getElementType()->isIntegralOrEnumerationType()) {
diff --git a/lib/StaticAnalyzer/Core/ObjCMessage.cpp b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
index 0974fe8..1edc376 100644
--- a/lib/StaticAnalyzer/Core/ObjCMessage.cpp
+++ b/lib/StaticAnalyzer/Core/ObjCMessage.cpp
@@ -13,6 +13,7 @@
//===----------------------------------------------------------------------===//
#include "clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h"
+#include "clang/AST/DeclCXX.h"
using namespace clang;
using namespace ento;
@@ -162,3 +163,21 @@
assert(isObjCMessage());
return Msg.getInstanceReceiverSVal(State, LC);
}
+
+const Decl *CallOrObjCMessage::getDecl() const {
+ if (isCXXCall()) {
+ const CXXMemberCallExpr *CE =
+ cast<CXXMemberCallExpr>(CallE.dyn_cast<const CallExpr *>());
+ assert(CE);
+ return CE->getMethodDecl();
+ } else if (isObjCMessage()) {
+ return Msg.getMethodDecl();
+ } else if (isFunctionCall()) {
+ // In case of a C style call, use the path sensitive information to find
+ // the function declaration.
+ SVal CalleeVal = getFunctionCallee();
+ return CalleeVal.getAsFunctionDecl();
+ }
+ return 0;
+}
+
diff --git a/test/Analysis/method-call-intra-p.cpp b/test/Analysis/method-call-intra-p.cpp
new file mode 100644
index 0000000..701479f
--- /dev/null
+++ b/test/Analysis/method-call-intra-p.cpp
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-store region -verify %s
+
+// Intra-procedural C++ tests.
+
+// Test relaxing function call arguments invalidation to be aware of const
+// arguments. radar://10595327
+struct InvalidateArgs {
+ void ttt(const int &nptr);
+ virtual void vttt(const int *nptr);
+};
+struct ChildOfInvalidateArgs: public InvalidateArgs {
+ virtual void vttt(const int *nptr);
+};
+void declarationFun(int x) {
+ InvalidateArgs t;
+ x = 3;
+ int y = x + 1;
+ int *p = 0;
+ t.ttt(y);
+ if (x == y)
+ y = *p; // no-warning
+}
+void virtualFun(int x) {
+ ChildOfInvalidateArgs t;
+ InvalidateArgs *pt = &t;
+ x = 3;
+ int y = x + 1;
+ int *p = 0;
+ pt->vttt(&y);
+ if (x == y)
+ y = *p; // no-warning
+}
diff --git a/test/Analysis/misc-ps.m b/test/Analysis/misc-ps.m
index 520b944..a1319fb 100644
--- a/test/Analysis/misc-ps.m
+++ b/test/Analysis/misc-ps.m
@@ -274,6 +274,17 @@
*p = 1; // expected-warning{{Dereference of null pointer}}
}
+// Check that the pointer-to-conts arguments do not get invalidated by Obj C
+// interfaces. radar://10595327
+int rdar_10595327(char *str) {
+ char fl = str[0];
+ int *p = 0;
+ NSString *s = [NSString stringWithUTF8String:str];
+ if (str[0] != fl)
+ return *p; // no-warning
+ return 0;
+}
+
// For pointer arithmetic, --/++ should be treated as preserving non-nullness,
// regardless of how well the underlying StoreManager reasons about pointer
// arithmetic.
diff --git a/test/Analysis/null-deref-ps.c b/test/Analysis/null-deref-ps.c
index 5da9699..8435080 100644
--- a/test/Analysis/null-deref-ps.c
+++ b/test/Analysis/null-deref-ps.c
@@ -289,4 +289,25 @@
pr4759_aux(p); // expected-warning{{Function call argument is an uninitialized value}}
}
-
+// Relax function call arguments invalidation to be aware of const
+// arguments. Test with function pointers. radar://10595327
+void ttt(const int *nptr);
+void ttt2(const int *nptr);
+typedef void (*NoConstType)(int*);
+int foo10595327(int b) {
+ void (*fp)(int *);
+ // We use path sensitivity to get the function declaration. Even when the
+ // function pointer is cast to non pointer-to-const parameter type, we can
+ // find the right function declaration.
+ if (b > 5)
+ fp = (NoConstType)ttt2;
+ else
+ fp = (NoConstType)ttt;
+ int x = 3;
+ int y = x + 1;
+ int *p = 0;
+ fp(&y);
+ if (x == y)
+ return *p; // no-warning
+ return 0;
+}
diff --git a/test/Analysis/string.c b/test/Analysis/string.c
index 89283be..fcbe298 100644
--- a/test/Analysis/string.c
+++ b/test/Analysis/string.c
@@ -133,6 +133,18 @@
(void)*(char*)0; // expected-warning{{null}}
}
+void strlen_indirect2(char *x) {
+ size_t a = strlen(x);
+ char *p = x;
+ char **p2 = &p;
+ extern void use_string_ptr2(char**);
+ use_string_ptr2(p2);
+
+ size_t c = strlen(x);
+ if (a == 0 && c != 0)
+ (void)*(char*)0; // expected-warning{{null}}
+}
+
void strlen_liveness(const char *x) {
if (strlen(x) < 5)
return;
diff --git a/test/Analysis/taint-tester.c b/test/Analysis/taint-tester.c
index 02afe6d..4aa170c 100644
--- a/test/Analysis/taint-tester.c
+++ b/test/Analysis/taint-tester.c
@@ -164,15 +164,10 @@
int d = atoi(s); // expected-warning + {{tainted}}
int td = d; // expected-warning + {{tainted}}
- // TODO: We shouldn't have to redefine the taint source here.
- char sl[80];
- scanf("%s", sl);
- long l = atol(sl); // expected-warning + {{tainted}}
+ long l = atol(s); // expected-warning + {{tainted}}
int tl = l; // expected-warning + {{tainted}}
- char sll[80];
- scanf("%s", sll);
- long long ll = atoll(sll); // expected-warning + {{tainted}}
+ long long ll = atoll(s); // expected-warning + {{tainted}}
int tll = ll; // expected-warning + {{tainted}}
}