From da822bb6244dfc205c1a7f617226ef0d26b352c9 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Wed, 11 May 2016 20:28:41 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@269227 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Checkers/DynamicTypePropagation.cpp | 25 ++ test/Analysis/generics.m | 273 ++++++++++++++++++ 2 files changed, 298 insertions(+) diff --git a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp index 008872fea4..b8e43325da 100644 --- a/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp +++ b/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp @@ -387,6 +387,14 @@ static const ObjCObjectPointerType *getMostInformativeDerivedClassImpl( } 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(); assert(SuperOfTo); @@ -444,6 +452,23 @@ storeWhenMoreInformative(ProgramStateRef &State, SymbolRef Sym, 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 and Bar are unrelated types. + // + // Foo *f = ... + // Bar *b = ... + // + // id t1 = b; + // f = t1; + // id t2 = f; // StaticLowerBound is Foo, Current is Bar + // + // 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()); diff --git a/test/Analysis/generics.m b/test/Analysis/generics.m index b64d06935d..490ac5dc8a 100644 --- a/test/Analysis/generics.m +++ b/test/Analysis/generics.m @@ -328,6 +328,21 @@ void returnToIdVariable(NSArray *arr) { NSNumber *res = a; // expected-warning {{Object has a dynamic type 'NSString *' which is incompatible with static type 'NSNumber *'}} } +@interface UnrelatedTypeGeneric : NSObject +- (void)takesType:(T)v; +@end + +void testGetMostInformativeDerivedForId(NSArray *a, + UnrelatedTypeGeneric *b) { + id idB = b; + a = idB; // expected-warning {{Conversion from value of type 'UnrelatedTypeGeneric *' to incompatible type 'NSArray *'}} + + // 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: diagnostics // CHECK-NEXT: // CHECK-NEXT: @@ -6626,4 +6641,262 @@ void returnToIdVariable(NSArray *arr) { // CHECK-NEXT: file0 // CHECK-NEXT: // CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Type 'UnrelatedTypeGeneric<NSString *> *' is inferred from implicit cast (from 'UnrelatedTypeGeneric<NSString *> *' to 'id') +// CHECK-NEXT: message +// CHECK-NEXT: Type 'UnrelatedTypeGeneric<NSString *> *' is inferred from implicit cast (from 'UnrelatedTypeGeneric<NSString *> *' to 'id') +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col9 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col9 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Conversion from value of type 'UnrelatedTypeGeneric<NSString *> *' to incompatible type 'NSArray<NSString *> *' +// CHECK-NEXT: message +// CHECK-NEXT: Conversion from value of type 'UnrelatedTypeGeneric<NSString *> *' to incompatible type 'NSArray<NSString *> *' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionConversion from value of type 'UnrelatedTypeGeneric<NSString *> *' to incompatible type 'NSArray<NSString *> *' +// CHECK-NEXT: categoryCore Foundation/Objective-C +// CHECK-NEXT: typeGenerics +// CHECK-NEXT: check_namecore.DynamicTypePropagation +// CHECK-NEXT: +// CHECK-NEXT: issue_hash_content_of_line_in_context8347f65fb51a85ccd462d75ffd761078 +// CHECK-NEXT: issue_context_kindfunction +// CHECK-NEXT: issue_contexttestGetMostInformativeDerivedForId +// CHECK-NEXT: issue_hash_function_offset2 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line338 +// CHECK-NEXT: col7 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: path +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col12 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Type 'UnrelatedTypeGeneric<NSString *> *' is inferred from implicit cast (from 'UnrelatedTypeGeneric<NSString *> *' to 'id') +// CHECK-NEXT: message +// CHECK-NEXT: Type 'UnrelatedTypeGeneric<NSString *> *' is inferred from implicit cast (from 'UnrelatedTypeGeneric<NSString *> *' to 'id') +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindcontrol +// CHECK-NEXT: edges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: start +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line337 +// CHECK-NEXT: col4 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: end +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: kindevent +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: ranges +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col16 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col38 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: depth0 +// CHECK-NEXT: extended_message +// CHECK-NEXT: Conversion from value of type 'NSNumber *' to incompatible type 'NSString *' +// CHECK-NEXT: message +// CHECK-NEXT: Conversion from value of type 'NSNumber *' to incompatible type 'NSString *' +// CHECK-NEXT: +// CHECK-NEXT: +// CHECK-NEXT: descriptionConversion from value of type 'NSNumber *' to incompatible type 'NSString *' +// CHECK-NEXT: categoryCore Foundation/Objective-C +// CHECK-NEXT: typeGenerics +// CHECK-NEXT: check_namecore.DynamicTypePropagation +// CHECK-NEXT: +// CHECK-NEXT: issue_hash_content_of_line_in_context6528db66f562ac0c2a94933f3ca5f6a8 +// CHECK-NEXT: issue_context_kindfunction +// CHECK-NEXT: issue_contexttestGetMostInformativeDerivedForId +// CHECK-NEXT: issue_hash_function_offset7 +// CHECK-NEXT: location +// CHECK-NEXT: +// CHECK-NEXT: line343 +// CHECK-NEXT: col3 +// CHECK-NEXT: file0 +// CHECK-NEXT: +// CHECK-NEXT: // CHECK-NEXT: -- 2.40.0