seaneveson updated this revision to Diff 36845. seaneveson added a comment.
Move PR21606 test into const-method-call.cpp. Added test for const calls on member objects. Added tests for inherited const methods. Changed TK_PreserveContents to be set for the base region. This is so memory is still preserved when 'this' is a member of another object. Added code to get the instance's class, rather than the method's class. This solves issues where the method's class has no mutable members, but the class which inherits the method does. http://reviews.llvm.org/D13099 Files: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h lib/StaticAnalyzer/Core/CallEvent.cpp test/Analysis/const-method-call.cpp
Index: test/Analysis/const-method-call.cpp =================================================================== --- test/Analysis/const-method-call.cpp +++ test/Analysis/const-method-call.cpp @@ -0,0 +1,205 @@ +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify %s + +void clang_analyzer_eval(bool); + +struct A { + int x; + void foo() const; + void bar(); +}; + +struct B { + mutable int mut; + void foo() const; +}; + +struct C { + int *p; + void foo() const; +}; + +struct MutBase { + mutable int b_mut; +}; + +struct MutDerived : MutBase { + void foo() const; +}; + +struct PBase { + int *p; +}; + +struct PDerived : PBase { + void foo() const; +}; + +struct Inner { + int x; + int *p; + void bar() const; +}; + +struct Outer { + int x; + Inner in; + void foo() const; +}; + +void checkThatConstMethodWithoutDefinitionDoesNotInvalidateObject() { + A t; + t.x = 3; + t.foo(); + clang_analyzer_eval(t.x == 3); // expected-warning{{TRUE}} + // Test non-const does invalidate + t.bar(); + clang_analyzer_eval(t.x); // expected-warning{{UNKNOWN}} +} + +void checkThatConstMethodDoesInvalidateMutableFields() { + B t; + t.mut = 4; + t.foo(); + clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}} +} + +void checkThatConstMethodDoesInvalidatePointedAtMemory() { + int x = 1; + C t; + t.p = &x; + t.foo(); + clang_analyzer_eval(x); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}} +} + +void checkThatConstMethodDoesInvalidateInheritedMutableFields() { + MutDerived t; + t.b_mut = 4; + t.foo(); + clang_analyzer_eval(t.b_mut); // expected-warning{{UNKNOWN}} +} + +void checkThatConstMethodDoesInvalidateInheritedPointedAtMemory() { + int x = 1; + PDerived t; + t.p = &x; + t.foo(); + clang_analyzer_eval(x); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}} +} + +void checkThatConstMethodDoesInvalidateContainedPointedAtMemory() { + int x = 1; + Outer t; + t.x = 2; + t.in.p = &x; + t.foo(); + clang_analyzer_eval(x); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(t.in.p == &x); // expected-warning{{TRUE}} +} + +void checkThatContainedConstMethodDoesNotInvalidateObjects() { + Outer t; + t.x = 1; + t.in.x = 2; + t.in.bar(); + clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}} +} + +// --- Versions of the above tests where the const method is inherited --- // + +struct B1 { + void foo() const; +}; + +struct D1 : public B1 { + int x; +}; + +struct D2 : public B1 { + mutable int mut; +}; + +struct D3 : public B1 { + int *p; +}; + +struct DInner : public B1 { + int x; + int *p; +}; + +struct DOuter : public B1 { + int x; + DInner in; +}; + +void checkThatInheritedConstMethodDoesNotInvalidateObject() { + D1 t; + t.x = 1; + t.foo(); + clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}} +} + +void checkThatInheritedConstMethodDoesInvalidateMutableFields() { + D2 t; + t.mut = 1; + t.foo(); + clang_analyzer_eval(t.mut); // expected-warning{{UNKNOWN}} +} + +void checkThatInheritedConstMethodDoesInvalidatePointedAtMemory() { + int x = 1; + D3 t; + t.p = &x; + t.foo(); + clang_analyzer_eval(x); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(t.p == &x); // expected-warning{{TRUE}} +} + +void checkThatInheritedConstMethodDoesInvalidateContainedPointedAtMemory() { + int x = 1; + DOuter t; + t.x = 2; + t.in.x = 3; + t.in.p = &x; + t.foo(); + clang_analyzer_eval(x); // expected-warning{{UNKNOWN}} + clang_analyzer_eval(t.x == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(t.in.x == 3); // expected-warning{{TRUE}} + clang_analyzer_eval(t.in.p == &x); // expected-warning{{TRUE}} +} + +void checkThatInheritedContainedConstMethodDoesNotInvalidateObjects() { + DOuter t; + t.x = 1; + t.in.x = 2; + t.in.foo(); + clang_analyzer_eval(t.x == 1); // expected-warning{{TRUE}} + clang_analyzer_eval(t.in.x == 2); // expected-warning{{TRUE}} +} + +// --- PR21606 --- // + +struct s1 { + void g(const int *i) const; +}; + +struct s2 { + void f(int *i) { + m_i = i; + m_s.g(m_i); + if (m_i) + *i = 42; // no-warning + } + + int *m_i; + s1 m_s; +}; + +void PR21606() +{ + s2().f(0); +} Index: lib/StaticAnalyzer/Core/CallEvent.cpp =================================================================== --- lib/StaticAnalyzer/Core/CallEvent.cpp +++ lib/StaticAnalyzer/Core/CallEvent.cpp @@ -148,7 +148,7 @@ SmallVector<SVal, 8> ValuesToInvalidate; RegionAndSymbolInvalidationTraits ETraits; - getExtraInvalidatedValues(ValuesToInvalidate); + getExtraInvalidatedValues(ValuesToInvalidate, &ETraits); // Indexes of arguments whose values will be preserved by the call. llvm::SmallSet<unsigned, 4> PreserveArgs; @@ -403,8 +403,31 @@ return getSVal(CE->getCallee()).getAsFunctionDecl(); } -void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values) const { - Values.push_back(getCXXThisVal()); +void CXXInstanceCall::getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const { + SVal ThisVal = getCXXThisVal(); + Values.push_back(ThisVal); + + // Don't invalidate if the method is const and there are no mutable fields + if (const CXXMethodDecl *D = cast_or_null<CXXMethodDecl>(getDecl())) { + if (!D->isConst()) + return; + // Get the record decl for the class of 'This'. D->getParent() may return a + // base class decl, rather than the class of the instance which needs to be + // checked for mutable fields. + const Expr *Ex = getCXXThisExpr(); + while (isa<ImplicitCastExpr>(Ex)) { + Ex = cast<ImplicitCastExpr>(Ex)->getSubExpr(); + } + const CXXRecordDecl *ParentRecord = Ex->getType()->getAsCXXRecordDecl(); + if (!ParentRecord || ParentRecord->hasMutableFields()) + return; + // Preserve CXXThis. + const MemRegion *ThisRegion = ThisVal.getAsRegion(); + assert(ThisRegion && "ThisValue was not a memory region"); + ETraits->setTrait(ThisRegion->getBaseRegion(), + RegionAndSymbolInvalidationTraits::TK_PreserveContents); + } } SVal CXXInstanceCall::getCXXThisVal() const { @@ -550,7 +573,8 @@ return D->parameters(); } -void BlockCall::getExtraInvalidatedValues(ValueList &Values) const { +void BlockCall::getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const { // FIXME: This also needs to invalidate captured globals. if (const MemRegion *R = getBlockRegion()) Values.push_back(loc::MemRegionVal(R)); @@ -571,7 +595,8 @@ return UnknownVal(); } -void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values) const { +void CXXConstructorCall::getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const { if (Data) Values.push_back(loc::MemRegionVal(static_cast<const MemRegion *>(Data))); } @@ -613,7 +638,8 @@ } void -ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values) const { +ObjCMethodCall::getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const { Values.push_back(getReceiverSVal()); } Index: include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h =================================================================== --- include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h +++ include/clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h @@ -164,7 +164,8 @@ /// \brief Used to specify non-argument regions that will be invalidated as a /// result of this call. - virtual void getExtraInvalidatedValues(ValueList &Values) const {} + virtual void getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const {} public: virtual ~CallEvent() {} @@ -472,7 +473,8 @@ BlockCall(const BlockCall &Other) : CallEvent(Other) {} void cloneTo(void *Dest) const override { new (Dest) BlockCall(*this); } - void getExtraInvalidatedValues(ValueList &Values) const override; + void getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const override; public: virtual const CallExpr *getOriginExpr() const { @@ -521,7 +523,8 @@ /// it is written. class CXXInstanceCall : public AnyFunctionCall { protected: - void getExtraInvalidatedValues(ValueList &Values) const override; + void getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const override; CXXInstanceCall(const CallExpr *CE, ProgramStateRef St, const LocationContext *LCtx) @@ -704,7 +707,8 @@ CXXConstructorCall(const CXXConstructorCall &Other) : AnyFunctionCall(Other){} void cloneTo(void *Dest) const override { new (Dest) CXXConstructorCall(*this); } - void getExtraInvalidatedValues(ValueList &Values) const override; + void getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const override; public: virtual const CXXConstructExpr *getOriginExpr() const { @@ -803,7 +807,8 @@ ObjCMethodCall(const ObjCMethodCall &Other) : CallEvent(Other) {} void cloneTo(void *Dest) const override { new (Dest) ObjCMethodCall(*this); } - void getExtraInvalidatedValues(ValueList &Values) const override; + void getExtraInvalidatedValues(ValueList &Values, + RegionAndSymbolInvalidationTraits *ETraits) const override; /// Check if the selector may have multiple definitions (may have overrides). virtual bool canBeOverridenInSubclass(ObjCInterfaceDecl *IDecl,
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits