[cfe-commits] r161682 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/inlining/InlineObjCClassMethod.m test/Analysis/inlining/InlineObjCInstanceMethod.h test/Analysis/inlining/InlineObjCInstanceMethod.m test/Analysis/inlining/ObjCDynTypePopagation.m test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m
Ted Kremenek
kremenek at apple.com
Tue Aug 21 15:26:55 PDT 2012
(Catching up on commits)
The DynamicTypeInfo needs a bit more documentation in the comments. Right now the comments are a bit vague. It's also not clear what it is modeling by looking at the class interface.
For example, the doxygen comment says:
> /// \class Represents the dynamic type information.
"Represents the dynamic type information" of *what*? And *when*? The comment needs this information.
It's also not clear why this is in ProgramState.h. This is a convenient place to put it, but it seems out of place. The definition is necessary for the accessors in ProgramState, but this seems like this belongs in some header relating to dynamic dispatch logic.
Other comments:
> bool isValid() const { return !T.isNull(); }
Is it possible to have a non-valid DynamicTypeInfo? What does that mean? Please add a comment.
>
> QualType getType() const { return T; }
> bool canBeASubClass() const { return CanBeASubClass; }
These two methods form the crux of the model, but it's not clear from the class what dynamic type information is getting modeled. I can extrapolate that his means the currently inferred strictest bound on the runtime type (the value of getType()), and that canBeASubClass() means that getType() really returns an upper bound in a lattice of types. It's not really clear, however. Please add doxygen comments that describe this in a bit more detail.
The reason I think this detail is important is because "getType()" probably means something a little different than in other parts of Clang. We typically associate the "type" of something as a static property that is fixed. This is something a little different.
>
> void Profile(llvm::FoldingSetNodeID &ID) const {
> T.Profile(ID);
> ID.AddInteger((unsigned)CanBeASubClass);
> }
> bool operator==(const DynamicTypeInfo &X) const {
> return T == X.T && CanBeASubClass == X.CanBeASubClass;
> }
> };
On Aug 10, 2012, at 11:55 AM, Anna Zaks <ganna at apple.com> wrote:
> Author: zaks
> Date: Fri Aug 10 13:55:58 2012
> New Revision: 161682
>
> URL: http://llvm.org/viewvc/llvm-project?rev=161682&view=rev
> Log:
> [analyzer] Track if a region can be a subclass in the dynamic type info.
>
> When object is allocated with alloc or init, we assume it cannot be a
> subclass (currently used only for bifurcation purposes).
>
> Modified:
> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
> cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
> cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m
> cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.h
> cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.m
> cfe/trunk/test/Analysis/inlining/ObjCDynTypePopagation.m
> cfe/trunk/test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m
>
> Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h (original)
> +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h Fri Aug 10 13:55:58 2012
> @@ -61,16 +61,20 @@
> /// dispatch implementation.
> class DynamicTypeInfo {
> QualType T;
> + bool CanBeASubClass;
>
> public:
> DynamicTypeInfo() : T(QualType()) {}
> - DynamicTypeInfo(QualType WithType) : T(WithType) {}
> - QualType getType() {return T;}
> + DynamicTypeInfo(QualType WithType, bool CanBeSub = true):
> + T(WithType), CanBeASubClass(CanBeSub) {}
> + QualType getType() { return T; }
> + bool canBeASubClass() { return CanBeASubClass; }
> void Profile(llvm::FoldingSetNodeID &ID) const {
> T.Profile(ID);
> + ID.AddInteger((unsigned)CanBeASubClass);
> }
> bool operator==(const DynamicTypeInfo &X) const {
> - return T == X.T;
> + return T == X.T && CanBeASubClass == X.CanBeASubClass;
> }
> };
>
> @@ -340,8 +344,9 @@
>
> /// \brief Set dynamic type information of the region; return the new state.
> ProgramStateRef setDynamicTypeInfo(const MemRegion *Reg,
> - QualType NewTy) const {
> - return setDynamicTypeInfo(Reg, DynamicTypeInfo(NewTy));
> + QualType NewTy,
> + bool CanBeSubClassed = true) const {
> + return setDynamicTypeInfo(Reg, DynamicTypeInfo(NewTy, CanBeSubClassed));
> }
>
> //==---------------------------------------------------------------------==//
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/DynamicTypePropagation.cpp Fri Aug 10 13:55:58 2012
> @@ -68,7 +68,7 @@
> return;
> QualType DynResTy =
> C.getASTContext().getObjCObjectPointerType(QualType(ObjTy, 0));
> - C.addTransition(State->setDynamicTypeInfo(RetReg, DynResTy));
> + C.addTransition(State->setDynamicTypeInfo(RetReg, DynResTy, false));
> break;
> }
> case OMF_init: {
>
> Modified: cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp (original)
> +++ cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp Fri Aug 10 13:55:58 2012
> @@ -721,6 +721,7 @@
>
> // Find the the receiver type.
> const ObjCObjectPointerType *ReceiverT = 0;
> + bool CanBeSubClassed = false;
> QualType SupersType = E->getSuperType();
> const MemRegion *Receiver = 0;
>
> @@ -733,17 +734,25 @@
> if (!Receiver)
> return RuntimeDefinition();
>
> - QualType DynType = getState()->getDynamicTypeInfo(Receiver).getType();
> + DynamicTypeInfo DTI = getState()->getDynamicTypeInfo(Receiver);
> + QualType DynType = DTI.getType();
> + CanBeSubClassed = DTI.canBeASubClass();
> ReceiverT = dyn_cast<ObjCObjectPointerType>(DynType);
> +
> + if (ReceiverT && CanBeSubClassed)
> + if (ObjCInterfaceDecl *IDecl = ReceiverT->getInterfaceDecl())
> + if (!canBeOverridenInSubclass(IDecl, Sel))
> + CanBeSubClassed = false;
> }
>
> // Lookup the method implementation.
> if (ReceiverT)
> if (ObjCInterfaceDecl *IDecl = ReceiverT->getInterfaceDecl()) {
> - if (canBeOverridenInSubclass(IDecl, Sel))
> - return RuntimeDefinition(IDecl->lookupPrivateMethod(Sel), Receiver);
> + const ObjCMethodDecl *MD = IDecl->lookupPrivateMethod(Sel);
> + if (CanBeSubClassed)
> + return RuntimeDefinition(MD, Receiver);
> else
> - return RuntimeDefinition(IDecl->lookupPrivateMethod(Sel), 0);
> + return RuntimeDefinition(MD, 0);
> }
>
> } else {
>
> Modified: cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m (original)
> +++ cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m Fri Aug 10 13:55:58 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-ipa=dynamic -verify %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-ipa=dynamic-bifurcate -verify %s
>
> // Test inlining of ObjC class methods.
>
>
> Modified: cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.h?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.h (original)
> +++ cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.h Fri Aug 10 13:55:58 2012
> @@ -11,6 +11,7 @@
> @protocol NSObject - (BOOL)isEqual:(id)object; @end
> @interface NSObject <NSObject> {}
> +(id)alloc;
> ++(id)new;
> -(id)init;
> -(id)autorelease;
> -(id)copy;
> @@ -24,3 +25,12 @@
>
> @interface PublicSubClass : PublicClass
> @end
> +
> + at interface PublicParent : NSObject
> +- (int)getZeroOverridden;
> + at end
> +
> + at interface PublicSubClass2 : PublicParent
> +- (int) getZeroOverridden;
> + at end
> +
>
> Modified: cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.m?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.m (original)
> +++ cfe/trunk/test/Analysis/inlining/InlineObjCInstanceMethod.m Fri Aug 10 13:55:58 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-ipa=dynamic -verify %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core -analyzer-ipa=dynamic-bifurcate -verify %s
>
> #include "InlineObjCInstanceMethod.h"
>
>
> Modified: cfe/trunk/test/Analysis/inlining/ObjCDynTypePopagation.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/ObjCDynTypePopagation.m?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/ObjCDynTypePopagation.m (original)
> +++ cfe/trunk/test/Analysis/inlining/ObjCDynTypePopagation.m Fri Aug 10 13:55:58 2012
> @@ -1,28 +1,12 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=dynamic -verify %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=dynamic-bifurcate -verify %s
>
> -typedef signed char BOOL;
> -typedef struct objc_class *Class;
> -typedef struct objc_object {
> - Class isa;
> -} *id;
> -
> -void clang_analyzer_eval(BOOL);
> -
> - at protocol NSObject - (BOOL)isEqual:(id)object; @end
> - at interface NSObject <NSObject> {}
> -+(id)alloc;
> --(id)init;
> -+(id)new;
> --(id)autorelease;
> --(id)copy;
> -- (Class)class;
> --(id)retain;
> - at end
> +#include "InlineObjCInstanceMethod.h"
>
> - at interface MyParent : NSObject
> -- (int)getZeroOverridden;
> - at end
> - at implementation MyParent
> +void clang_analyzer_eval(int);
> +
> +PublicSubClass2 *getObj();
> +
> + at implementation PublicParent
> - (int) getZeroOverridden {
> return 1;
> }
> @@ -31,19 +15,12 @@
> }
> @end
>
> - at interface MyClass : MyParent
> -- (int) getZeroOverridden;
> - at end
> -
> -MyClass *getObj();
> -
> - at implementation MyClass
> + at implementation PublicSubClass2
> - (int) getZeroOverridden {
> return 0;
> }
>
> /* Test that we get the right type from call to alloc. */
> -
> + (void) testAllocSelf {
> id a = [self alloc];
> clang_analyzer_eval([a getZeroOverridden] == 0); // expected-warning{{TRUE}}
> @@ -51,7 +28,7 @@
>
>
> + (void) testAllocClass {
> - id a = [MyClass alloc];
> + id a = [PublicSubClass2 alloc];
> clang_analyzer_eval([a getZeroOverridden] == 0); // expected-warning{{TRUE}}
> }
>
> @@ -79,19 +56,19 @@
> // Casting to parent should not pessimize the dynamic type.
> + (void) testCastToParent {
> id a = [[self alloc] init];
> - MyParent *p = a;
> + PublicParent *p = a;
> clang_analyzer_eval([p getZeroOverridden] == 0); // expected-warning{{TRUE}}
> }
>
> // The type of parameter gets used.
> -+ (void)testTypeFromParam:(MyParent*) p {
> ++ (void)testTypeFromParam:(PublicParent*) p {
> clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}
> }
>
> // Test implicit cast.
> // Note, in this case, p could also be a subclass of MyParent.
> + (void) testCastFromId:(id) a {
> - MyParent *p = a;
> + PublicParent *p = a;
> clang_analyzer_eval([p getZero] == 0); // expected-warning{{TRUE}}
> }
> @end
> @@ -99,7 +76,7 @@
> // TODO: Would be nice to handle the case of dynamically obtained class info
> // as well. We need a MemRegion for class types for this.
> int testDynamicClass(BOOL coin) {
> - Class AllocClass = (coin ? [NSObject class] : [MyClass class]);
> + Class AllocClass = (coin ? [NSObject class] : [PublicSubClass2 class]);
> id x = [[AllocClass alloc] init];
> if (coin)
> return [x getZero];
>
> Modified: cfe/trunk/test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m?rev=161682&r1=161681&r2=161682&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m (original)
> +++ cfe/trunk/test/Analysis/inlining/ObjCImproperDynamictallyDetectableCast.m Fri Aug 10 13:55:58 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=dynamic -verify %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-ipa=dynamic-bifurcate -verify %s
>
> typedef signed char BOOL;
> @protocol NSObject - (BOOL)isEqual:(id)object; @end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list