[cfe-commits] r61147 - in /cfe/trunk: lib/Analysis/CFRefCount.cpp test/Analysis/uninit-vals-ps.c

[Ted Kremenek]

On Mar 9, 2009, at 2:41 AM, Zhongxing Xu wrote:

> Hi Ted,
>
> This patch report false warning on this test case:
>
> #include <sys/socket.h>
> void f(int sock) {
>   struct sockaddr_storage storage;
>   struct sockaddr* sockaddr = (struct sockaddr*)&storage;
>   socklen_t addrlen = sizeof(storage);
>   getsockname(sock, sockaddr, &addrlen);
>   switch (sockaddr->sa_family) {
>   default:
>     ;
>   }
> }
>
> $ clang -analyze -analyzer-store=region -checker-cfref 1.c
> 1.c:7:3: warning: Branch condition evaluates to an uninitialized  
> value.
>   switch (sockaddr->sa_family) {
>   ^       ~~~~~~~~~~~~~~~~~~~
> 1 diagnostic generated.
>
> Perhaps we should not 'blast through' TypedViewRegion?

The motivation for ignoring the TypedViewRegions has to do with  
typedefs.  Conceptually we want to handle bindings through typedefs  
and the desugared type as the same:

typedef struct s* MyPointer;

MyPointer *p = foo();
p->f = ...
struct s* q = p;
... = q->f;

Here 'p' will bind to a TypedViewRegion that wraps a SymbolicRegion.

Depending on the return type of foo() (i.e., if it is 'void*' or  
'struct s*') then 'q' should bind either to a TypedViewRegion or a  
SymbolicRegion with type 'struct s*'.

In this case, we should be reasoning about the same locations for 'q- 
 >f' and 'p->f'.

I admit that this work should probably happen in RegionStore.  Indeed,  
all invalidation of values should probably go directly through the  
StoreManager at some point.

The issue here is that some type views are "sugar" and others change  
the nature of the binding (e.g., layerind "struct s*' on top of  
'void*').

Perhaps we can add a "SugarTypedRegion" (or something with a better  
name) to represent region views that are just sugar but don't change  
the semantics?  This could be useful for RegionStore to help  
canonicalize the names of locations.  Alternatively, the StoreManager  
can implement a method called "getCanonicalRegion()" to to transform a  
MemRegion* into its canonical version that is used for binding values.