[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [cfe-commits] r74525 - in /cfe/trunk/lib/Analysis:
From:       Ted Kremenek <kremenek () apple ! com>
Date:       2009-06-30 19:31:08
Message-ID: F7264689-4F92-4296-B7B4-89E919BE8E51 () apple ! com
[Download RAW message or body]

This looks good to me.  One comment inline.

On Jun 30, 2009, at 6:01 AM, Zhongxing Xu wrote:

> Author: zhongxingxu
> Date: Tue Jun 30 08:00:53 2009
> New Revision: 74525
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=74525&view=rev
> Log:
> Instead of r74522, use another approach to fix  
> xfail_regionstore_wine_crash.c.
> Mark the super region of the binding of block level expr in the  
> Environment
> as live.
> 
> Modified:
> cfe/trunk/lib/Analysis/Environment.cpp
> cfe/trunk/lib/Analysis/LiveVariables.cpp
> 
> Modified: cfe/trunk/lib/Analysis/Environment.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/Environment.cpp?rev=74525&r1=74524&r2=74525&view=diff
>  
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/Environment.cpp (original)
> +++ cfe/trunk/lib/Analysis/Environment.cpp Tue Jun 30 08:00:53 2009
> @@ -143,8 +143,17 @@
> SVal X = I.getData();
> 
> // If the block expr's value is a memory region, then mark  
> that region.
> -      if (isa<loc::MemRegionVal>(X))
> -        DRoots.push_back(cast<loc::MemRegionVal>(X).getRegion());
> +      if (isa<loc::MemRegionVal>(X)) {
> +        const MemRegion* R = cast<loc::MemRegionVal>(X).getRegion();
> +        DRoots.push_back(R);
> +        // Mark the super region of the RX as live.
> +        // e.g.: int x; char *y = (char*) &x; if (*y) ...
> +        // 'y' => element region. 'x' is its super region.
> +        // We only add one level super region for now.

Is there a reason we only add one level of super region?  If we need  
to add the entire region hierarchy, I'd rather do it now before we  
forget about it.   A recursive function should do the trick.  At the  
very least there should be a FIXME comment if this is something that  
needs to be fixed.

> +        if (const SubRegion *SR = dyn_cast<SubRegion>(R)) {
> +          DRoots.push_back(SR->getSuperRegion());
> +        }
> +      }
> 
> // Mark all symbols in the block expr's value live.
> MarkLiveCallback cb(SymReaper);
> 
> Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=74525&r1=74524&r2=74525&view=diff
>  
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
> +++ cfe/trunk/lib/Analysis/LiveVariables.cpp Tue Jun 30 08:00:53 2009
> @@ -138,7 +138,6 @@
> else {
> // For block-level expressions, mark that they are live.
> LiveState(S,AD) = Alive;
> -    StmtVisitor<TransferFuncs,void>::Visit(S);
> }
> }
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]