[cfe-commits] r71094 - in /cfe/trunk: lib/Analysis/CFRefCount.cpp lib/Analysis/Store.cpp test/Analysis/pr_4164.c
Ted Kremenek
kremenek at apple.com
Wed May 6 11:19:24 PDT 2009
Author: kremenek
Date: Wed May 6 13:19:24 2009
New Revision: 71094
URL: http://llvm.org/viewvc/llvm-project?rev=71094&view=rev
Log:
Fix analyzer regression reported in PR 4164:
- Update the old StoreManager::CastRegion to strip off 'ElementRegions' when
casting to void* (Zhongxing: please validate)
- Pass-by-reference argument invalidation logic in CFRefCount.cpp:
- Strip ElementRegions when the ElementRegion is just a 'raw data' view
on top of the underlying typed region.
Added:
cfe/trunk/test/Analysis/pr_4164.c
Modified:
cfe/trunk/lib/Analysis/CFRefCount.cpp
cfe/trunk/lib/Analysis/Store.cpp
Modified: cfe/trunk/lib/Analysis/CFRefCount.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFRefCount.cpp?rev=71094&r1=71093&r2=71094&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/CFRefCount.cpp (original)
+++ cfe/trunk/lib/Analysis/CFRefCount.cpp Wed May 6 13:19:24 2009
@@ -2606,7 +2606,29 @@
const TypedRegion* R = dyn_cast<TypedRegion>(MR->getRegion());
- if (R) {
+ if (R) {
+ // Are we dealing with an ElementRegion? If the element type is
+ // a basic integer type (e.g., char, int) and the underying region
+ // is also typed 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
+ // about (char*), etc., is just a form of passing raw bytes.
+ // e.g., void *p = alloca(); foo((char*)p);
+ if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
+ // Checking for 'integral type' is probably too promiscuous, but
+ // 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
+ // still allowing us to do checker-specific logic (e.g.,
+ // invalidating reference counts), probably via callbacks.
+ if (ER->getElementType()->isIntegralType())
+ if (const TypedRegion *superReg =
+ dyn_cast<TypedRegion>(ER->getSuperRegion()))
+ R = superReg;
+ // FIXME: What about layers of ElementRegions?
+ }
+
// Is the invalidated variable something that we were tracking?
SymbolRef Sym = state.GetSValAsScalarOrLoc(R).getAsLocSymbol();
Modified: cfe/trunk/lib/Analysis/Store.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Store.cpp?rev=71094&r1=71093&r2=71094&view=diff
==============================================================================
--- cfe/trunk/lib/Analysis/Store.cpp (original)
+++ cfe/trunk/lib/Analysis/Store.cpp Wed May 6 13:19:24 2009
@@ -44,18 +44,37 @@
QualType Pointee = PTy->getPointeeType();
if (Pointee->isVoidType()) {
- // Casts to void* only removes TypedViewRegion. If there is no
- // TypedViewRegion, leave the region untouched. This happens when:
- //
- // void foo(void*);
- // ...
- // void bar() {
- // int x;
- // foo(&x);
- // }
-
- if (const TypedViewRegion *TR = dyn_cast<TypedViewRegion>(R))
- R = TR->removeViews();
+ do {
+ if (const TypedViewRegion *TR = dyn_cast<TypedViewRegion>(R)) {
+ // Casts to void* removes TypedViewRegion. This happens when:
+ //
+ // void foo(void*);
+ // ...
+ // void bar() {
+ // int x;
+ // foo(&x);
+ // }
+ //
+ R = TR->removeViews();
+ continue;
+ }
+ else if (const ElementRegion *ER = dyn_cast<ElementRegion>(R)) {
+ // Casts to void* also removes ElementRegions. This happens when:
+ //
+ // void foo(void*);
+ // ...
+ // void bar() {
+ // int x;
+ // foo((char*)&x);
+ // }
+ //
+ R = ER->getSuperRegion();
+ continue;
+ }
+ else
+ break;
+ }
+ while (0);
return CastResult(state, R);
}
Added: cfe/trunk/test/Analysis/pr_4164.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/pr_4164.c?rev=71094&view=auto
==============================================================================
--- cfe/trunk/test/Analysis/pr_4164.c (added)
+++ cfe/trunk/test/Analysis/pr_4164.c Wed May 6 13:19:24 2009
@@ -0,0 +1,41 @@
+// RUN: clang-cc -triple x86_64-apple-darwin9 -analyze -checker-cfref -analyzer-store=basic -verify %s &&
+// RUN: clang-cc -triple x86_64-apple-darwin9 -analyze -checker-cfref -analyzer-store=region -verify %s
+
+// PR 4164: http://llvm.org/bugs/show_bug.cgi?id=4164
+//
+// Eventually this should be pulled into misc-ps.m. This is in a separate test
+// file for now to play around with the specific issues for BasicStoreManager
+// and StoreManager (i.e., we can make a copy of this file for either
+// StoreManager should one start to fail in the near future).
+//
+// The basic issue is that the VarRegion for 'size' is casted to (char*),
+// resulting in an ElementRegion. 'getsockopt' is an unknown function that
+// takes a void*, which means the ElementRegion should get stripped off.
+typedef unsigned int __uint32_t;
+typedef __uint32_t __darwin_socklen_t;
+typedef __darwin_socklen_t socklen_t;
+int getsockopt(int, int, int, void * restrict, socklen_t * restrict);
+
+int test1() {
+ int s = -1;
+ int size;
+ socklen_t size_len = sizeof(size);
+ if (getsockopt(s, 0xffff, 0x1001, (char *)&size, &size_len) < 0)
+ return -1;
+
+ return size; // no-warning
+}
+
+// Similar case: instead of passing a 'void*', we pass 'char*'. In this
+// case we pass an ElementRegion to the invalidation logic. Since it is
+// an ElementRegion that just layers on top of another typed region and the
+// ElementRegion itself has elements whose type are integral (essentially raw
+// data) we strip off the ElementRegion when doing the invalidation.
+int takes_charptr(char* p);
+int test2() {
+ int size;
+ if (takes_charptr((char*)&size))
+ return -1;
+ return size; // no-warning
+}
+
More information about the cfe-commits
mailing list