r269227 - [analyzer] Fix crash in ObjCGenericsChecker

Devin Coughlin via cfe-commits Wed, 11 May 2016 13:35:03 -0700

Author: dcoughlin
Date: Wed May 11 15:28:41 2016
New Revision: 269227

URL: http://llvm.org/viewvc/llvm-project?rev=269227&view=rev
Log:
[analyzer] Fix crash in ObjCGenericsChecker


Fix a crash in the generics checker where DynamicTypePropagation tries
to get the superclass of a root class.

This is a spot-fix for a deeper issue where the checker makes assumptions
that may not hold about subtyping between the symbolically-tracked type of
a value and the compile-time types of a cast on that value.

I've added a TODO to address the underlying issue.

rdar://problem/26086914

Modified:
    cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
    cfe/trunk/test/Analysis/generics.m

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=269227&r1=269226&r2=269227&view=diff
==============================================================================
--- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Wed May 11 
15:28:41 2016
@@ -387,6 +387,14 @@ static const ObjCObjectPointerType *getM
     }
     return From;
   }
+
+  if (To->getObjectType()->getSuperClassType().isNull()) {
+    // If To has no super class and From and To aren't the same then
+    // To was not actually a descendent of From. In this case the best we can
+    // do is 'From'.
+    return From;
+  }
+
   const auto *SuperOfTo =
       To->getObjectType()->getSuperClassType()->getAs<ObjCObjectType>();
   assert(SuperOfTo);
@@ -444,6 +452,23 @@ storeWhenMoreInformative(ProgramStateRef
                          const ObjCObjectPointerType *StaticLowerBound,
                          const ObjCObjectPointerType *StaticUpperBound,
                          ASTContext &C) {
+  // TODO: The above 4 cases are not exhaustive. In particular, it is possible
+  // for Current to be incomparable with StaticLowerBound, StaticUpperBound,
+  // or both.
+  //
+  // For example, suppose Foo<T> and Bar<T> are unrelated types.
+  //
+  //  Foo<T> *f = ...
+  //  Bar<T> *b = ...
+  //
+  //  id t1 = b;
+  //  f = t1;
+  //  id t2 = f; // StaticLowerBound is Foo<T>, Current is Bar<T>
+  //
+  // We should either constrain the callers of this function so that the stated
+  // preconditions hold (and assert it) or rewrite the function to expicitly
+  // handle the additional cases.
+
   // Precondition
   assert(StaticUpperBound->isSpecialized() ||
          StaticLowerBound->isSpecialized());

Modified: cfe/trunk/test/Analysis/generics.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/generics.m?rev=269227&r1=269226&r2=269227&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/generics.m (original)
+++ cfe/trunk/test/Analysis/generics.m Wed May 11 15:28:41 2016
@@ -328,6 +328,21 @@ void returnToIdVariable(NSArray<NSString
   NSNumber *res = a; // expected-warning {{Object has a dynamic type 'NSString 
*' which is incompatible with static type 'NSNumber *'}}
 }
 
+@interface UnrelatedTypeGeneric<T> : NSObject<NSCopying>
+- (void)takesType:(T)v;
+@end
+
+void testGetMostInformativeDerivedForId(NSArray<NSString *> *a,
+                                  UnrelatedTypeGeneric<NSString *> *b) {
+  id idB = b;
+  a = idB; // expected-warning {{Conversion from value of type 
'UnrelatedTypeGeneric<NSString *> *' to incompatible type 'NSArray<NSString *> 
*'}}
+
+  // rdar://problem/26086914 crash here caused by symbolic type being unrelated
+  // to compile-time source type of cast.
+  id x = a; // Compile-time type is NSArray<>, Symbolic type is 
UnrelatedTypeGeneric<>.
+  [x takesType:[[NSNumber alloc] init]]; // expected-warning {{Conversion from 
value of type 'NSNumber *' to incompatible type 'NSString *'}}
+}
+
 // CHECK: <key>diagnostics</key>
 // CHECK-NEXT: <array>
 // CHECK-NEXT:  <dict>
@@ -6626,4 +6641,262 @@ void returnToIdVariable(NSArray<NSString
 // CHECK-NEXT:   <key>file</key><integer>0</integer>
 // CHECK-NEXT:  </dict>
 // CHECK-NEXT:  </dict>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>path</key>
+// CHECK-NEXT:   <array>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>event</string>
+// CHECK-NEXT:     <key>location</key>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>line</key><integer>337</integer>
+// CHECK-NEXT:      <key>col</key><integer>12</integer>
+// CHECK-NEXT:      <key>file</key><integer>0</integer>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <key>ranges</key>
+// CHECK-NEXT:     <array>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>337</integer>
+// CHECK-NEXT:         <key>col</key><integer>12</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>337</integer>
+// CHECK-NEXT:         <key>col</key><integer>12</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </array>
+// CHECK-NEXT:     <key>depth</key><integer>0</integer>
+// CHECK-NEXT:     <key>extended_message</key>
+// CHECK-NEXT:     <string>Type &apos;UnrelatedTypeGeneric&lt;NSString *&gt; 
*&apos; is inferred from implicit cast (from 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to &apos;id&apos;)</string>
+// CHECK-NEXT:     <key>message</key>
+// CHECK-NEXT:     <string>Type &apos;UnrelatedTypeGeneric&lt;NSString *&gt; 
*&apos; is inferred from implicit cast (from 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to &apos;id&apos;)</string>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>control</string>
+// CHECK-NEXT:     <key>edges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:       <dict>
+// CHECK-NEXT:        <key>start</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>337</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>337</integer>
+// CHECK-NEXT:           <key>col</key><integer>4</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:        <key>end</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:       </dict>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>control</string>
+// CHECK-NEXT:     <key>edges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:       <dict>
+// CHECK-NEXT:        <key>start</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:        <key>end</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>7</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>338</integer>
+// CHECK-NEXT:           <key>col</key><integer>9</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:       </dict>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>event</string>
+// CHECK-NEXT:     <key>location</key>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>line</key><integer>338</integer>
+// CHECK-NEXT:      <key>col</key><integer>7</integer>
+// CHECK-NEXT:      <key>file</key><integer>0</integer>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <key>ranges</key>
+// CHECK-NEXT:     <array>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>338</integer>
+// CHECK-NEXT:         <key>col</key><integer>7</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>338</integer>
+// CHECK-NEXT:         <key>col</key><integer>9</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </array>
+// CHECK-NEXT:     <key>depth</key><integer>0</integer>
+// CHECK-NEXT:     <key>extended_message</key>
+// CHECK-NEXT:     <string>Conversion from value of type 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to incompatible type 
&apos;NSArray&lt;NSString *&gt; *&apos;</string>
+// CHECK-NEXT:     <key>message</key>
+// CHECK-NEXT:     <string>Conversion from value of type 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to incompatible type 
&apos;NSArray&lt;NSString *&gt; *&apos;</string>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:   </array>
+// CHECK-NEXT:   <key>description</key><string>Conversion from value of type 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to incompatible type 
&apos;NSArray&lt;NSString *&gt; *&apos;</string>
+// CHECK-NEXT:   <key>category</key><string>Core 
Foundation/Objective-C</string>
+// CHECK-NEXT:   <key>type</key><string>Generics</string>
+// CHECK-NEXT:   
<key>check_name</key><string>core.DynamicTypePropagation</string>
+// CHECK-NEXT:   <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT:   
<key>issue_hash_content_of_line_in_context</key><string>8347f65fb51a85ccd462d75ffd761078</string>
+// CHECK-NEXT:  <key>issue_context_kind</key><string>function</string>
+// CHECK-NEXT:  
<key>issue_context</key><string>testGetMostInformativeDerivedForId</string>
+// CHECK-NEXT:  <key>issue_hash_function_offset</key><string>2</string>
+// CHECK-NEXT:  <key>location</key>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>line</key><integer>338</integer>
+// CHECK-NEXT:   <key>col</key><integer>7</integer>
+// CHECK-NEXT:   <key>file</key><integer>0</integer>
+// CHECK-NEXT:  </dict>
+// CHECK-NEXT:  </dict>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>path</key>
+// CHECK-NEXT:   <array>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>event</string>
+// CHECK-NEXT:     <key>location</key>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>line</key><integer>337</integer>
+// CHECK-NEXT:      <key>col</key><integer>12</integer>
+// CHECK-NEXT:      <key>file</key><integer>0</integer>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <key>ranges</key>
+// CHECK-NEXT:     <array>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>337</integer>
+// CHECK-NEXT:         <key>col</key><integer>12</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>337</integer>
+// CHECK-NEXT:         <key>col</key><integer>12</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </array>
+// CHECK-NEXT:     <key>depth</key><integer>0</integer>
+// CHECK-NEXT:     <key>extended_message</key>
+// CHECK-NEXT:     <string>Type &apos;UnrelatedTypeGeneric&lt;NSString *&gt; 
*&apos; is inferred from implicit cast (from 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to &apos;id&apos;)</string>
+// CHECK-NEXT:     <key>message</key>
+// CHECK-NEXT:     <string>Type &apos;UnrelatedTypeGeneric&lt;NSString *&gt; 
*&apos; is inferred from implicit cast (from 
&apos;UnrelatedTypeGeneric&lt;NSString *&gt; *&apos; to &apos;id&apos;)</string>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>control</string>
+// CHECK-NEXT:     <key>edges</key>
+// CHECK-NEXT:      <array>
+// CHECK-NEXT:       <dict>
+// CHECK-NEXT:        <key>start</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>337</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>337</integer>
+// CHECK-NEXT:           <key>col</key><integer>4</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:        <key>end</key>
+// CHECK-NEXT:         <array>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>343</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:          <dict>
+// CHECK-NEXT:           <key>line</key><integer>343</integer>
+// CHECK-NEXT:           <key>col</key><integer>3</integer>
+// CHECK-NEXT:           <key>file</key><integer>0</integer>
+// CHECK-NEXT:          </dict>
+// CHECK-NEXT:         </array>
+// CHECK-NEXT:       </dict>
+// CHECK-NEXT:      </array>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:    <dict>
+// CHECK-NEXT:     <key>kind</key><string>event</string>
+// CHECK-NEXT:     <key>location</key>
+// CHECK-NEXT:     <dict>
+// CHECK-NEXT:      <key>line</key><integer>343</integer>
+// CHECK-NEXT:      <key>col</key><integer>3</integer>
+// CHECK-NEXT:      <key>file</key><integer>0</integer>
+// CHECK-NEXT:     </dict>
+// CHECK-NEXT:     <key>ranges</key>
+// CHECK-NEXT:     <array>
+// CHECK-NEXT:       <array>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>343</integer>
+// CHECK-NEXT:         <key>col</key><integer>16</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:        <dict>
+// CHECK-NEXT:         <key>line</key><integer>343</integer>
+// CHECK-NEXT:         <key>col</key><integer>38</integer>
+// CHECK-NEXT:         <key>file</key><integer>0</integer>
+// CHECK-NEXT:        </dict>
+// CHECK-NEXT:       </array>
+// CHECK-NEXT:     </array>
+// CHECK-NEXT:     <key>depth</key><integer>0</integer>
+// CHECK-NEXT:     <key>extended_message</key>
+// CHECK-NEXT:     <string>Conversion from value of type &apos;NSNumber 
*&apos; to incompatible type &apos;NSString *&apos;</string>
+// CHECK-NEXT:     <key>message</key>
+// CHECK-NEXT:     <string>Conversion from value of type &apos;NSNumber 
*&apos; to incompatible type &apos;NSString *&apos;</string>
+// CHECK-NEXT:    </dict>
+// CHECK-NEXT:   </array>
+// CHECK-NEXT:   <key>description</key><string>Conversion from value of type 
&apos;NSNumber *&apos; to incompatible type &apos;NSString *&apos;</string>
+// CHECK-NEXT:   <key>category</key><string>Core 
Foundation/Objective-C</string>
+// CHECK-NEXT:   <key>type</key><string>Generics</string>
+// CHECK-NEXT:   
<key>check_name</key><string>core.DynamicTypePropagation</string>
+// CHECK-NEXT:   <!-- This hash is experimental and going to change! -->
+// CHECK-NEXT:   
<key>issue_hash_content_of_line_in_context</key><string>6528db66f562ac0c2a94933f3ca5f6a8</string>
+// CHECK-NEXT:  <key>issue_context_kind</key><string>function</string>
+// CHECK-NEXT:  
<key>issue_context</key><string>testGetMostInformativeDerivedForId</string>
+// CHECK-NEXT:  <key>issue_hash_function_offset</key><string>7</string>
+// CHECK-NEXT:  <key>location</key>
+// CHECK-NEXT:  <dict>
+// CHECK-NEXT:   <key>line</key><integer>343</integer>
+// CHECK-NEXT:   <key>col</key><integer>3</integer>
+// CHECK-NEXT:   <key>file</key><integer>0</integer>
+// CHECK-NEXT:  </dict>
+// CHECK-NEXT:  </dict>
 // CHECK-NEXT: </array>


_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r269227 - [analyzer] Fix crash in ObjCGenericsChecker

Reply via email to