[cfe-commits] r53115 - in /cfe/trunk: lib/Analysis/LiveVariables.cpp test/Analysis/dead-stores.m

[Ted Kremenek]

Author: kremenek
Date: Thu Jul  3 17:25:27 2008
New Revision: 53115

URL: http://llvm.org/viewvc/llvm-project?rev=53115&view=rev
Log:
Fix a bug in the dead stores checker reported in the following email:

  http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-July/002157.html

Essentially the observer mechanism in LiveVariables was observing block-level
expressions multiple times, leading to a case where the dead store checker could
see a value as dead when it was really live.

Added:
    cfe/trunk/test/Analysis/dead-stores.m
Modified:
    cfe/trunk/lib/Analysis/LiveVariables.cpp

Modified: cfe/trunk/lib/Analysis/LiveVariables.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/LiveVariables.cpp?rev=53115&r1=53114&r2=53115&view=diff

==============================================================================
--- cfe/trunk/lib/Analysis/LiveVariables.cpp (original)
+++ cfe/trunk/lib/Analysis/LiveVariables.cpp Thu Jul  3 17:25:27 2008
@@ -133,15 +133,23 @@
 };
       
 void TransferFuncs::Visit(Stmt *S) {
-  if (AD.Observer)
-    AD.Observer->ObserveStmt(S,AD,LiveState);
   
   if (S == getCurrentBlkStmt()) {
+    
+    if (AD.Observer)
+      AD.Observer->ObserveStmt(S,AD,LiveState);
+    
     if (getCFG().isBlkExpr(S)) LiveState(S,AD) = Dead;
     StmtVisitor<TransferFuncs,void>::Visit(S);
   }
-  else if (!getCFG().isBlkExpr(S))
+  else if (!getCFG().isBlkExpr(S)) {
+    
+    if (AD.Observer)
+      AD.Observer->ObserveStmt(S,AD,LiveState);
+    
     StmtVisitor<TransferFuncs,void>::Visit(S);
+    
+  }
   else
     // For block-level expressions, mark that they are live.
     LiveState(S,AD) = Alive;

Added: cfe/trunk/test/Analysis/dead-stores.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.m?rev=53115&view=auto

==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.m (added)
+++ cfe/trunk/test/Analysis/dead-stores.m Thu Jul  3 17:25:27 2008
@@ -0,0 +1,36 @@
+// RUN: clang -warn-dead-stores -verify %s
+
+typedef signed char BOOL;
+typedef unsigned int NSUInteger;
+typedef struct _NSZone NSZone;
+ at class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
+ at protocol NSObject  - (BOOL)isEqual:(id)object; @end
+ at protocol NSCopying  - (id)copyWithZone:(NSZone *)zone; @end
+ at protocol NSCoding  - (void)encodeWithCoder:(NSCoder *)aCoder; @end
+ at interface NSObject <NSObject> {} @end
+extern id NSAllocateObject(Class aClass, NSUInteger extraBytes, NSZone *zone);
+ at interface NSValue : NSObject <NSCopying, NSCoding>  - (void)getValue:(void *)value; @end
+typedef float CGFloat;
+typedef struct _NSPoint {} NSRange;
+ at interface NSValue (NSValueRangeExtensions)  + (NSValue *)valueWithRange:(NSRange)range;
+- (BOOL)containsObject:(id)anObject;
+ at end
+ at class NSURLAuthenticationChallenge;
+ at interface NSResponder : NSObject <NSCoding> {} @end
+ at class NSArray, NSDictionary, NSString;
+ at interface NSObject (NSKeyValueBindingCreation)
++ (void)exposeBinding:(NSString *)binding;
+- (NSArray *)exposedBindings;
+ at end
+extern NSString *NSAlignmentBinding;
+
+// This test case was reported as a false positive due to a bug in the
+// LiveVariables <-> DeadStores interplay.  We should not flag a warning
+// here.  The test case was reported in:
+//  http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-July/002157.html
+void DeadStoreTest(NSObject *anObject) {
+  NSArray *keys;
+  if ((keys = [anObject exposedBindings]) &&   // no-warning
+      ([keys containsObject:@"name"] && [keys containsObject:@"icon"])) {}
+}
+