Index: include/clang/Analysis/CFG.h =================================================================== --- include/clang/Analysis/CFG.h +++ include/clang/Analysis/CFG.h @@ -15,9 +15,10 @@ #ifndef LLVM_CLANG_ANALYSIS_CFG_H #define LLVM_CLANG_ANALYSIS_CFG_H -#include "clang/AST/ExprCXX.h" #include "clang/Analysis/Support/BumpVector.h" #include "clang/Analysis/ConstructionContext.h" +#include "clang/AST/ExprCXX.h" +#include "clang/AST/ExprObjC.h" #include "clang/Basic/LLVM.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/GraphTraits.h" @@ -179,15 +180,18 @@ public: /// Returns true when call expression \p CE needs to be represented /// by CFGCXXRecordTypedCall, as opposed to a regular CFGStmt. - static bool isCXXRecordTypedCall(CallExpr *CE, const ASTContext &ACtx) { - return CE->getCallReturnType(ACtx).getCanonicalType()->getAsCXXRecordDecl(); + static bool isCXXRecordTypedCall(Expr *E) { + assert(isa(E) || isa(E)); + // There is no such thing as reference-type expression. If the function + // returns a reference, it'll return the respective lvalue or xvalue + // instead, and we're only interested in objects. + return !E->isGLValue() && + E->getType().getCanonicalType()->getAsCXXRecordDecl(); } - explicit CFGCXXRecordTypedCall(CallExpr *CE, const ConstructionContext *C) - : CFGStmt(CE, CXXRecordTypedCall) { - // FIXME: This is not protected against squeezing a non-record-typed-call - // into the constructor. An assertion would require passing an ASTContext - // which would mean paying for something we don't use. + explicit CFGCXXRecordTypedCall(Expr *E, const ConstructionContext *C) + : CFGStmt(E, CXXRecordTypedCall) { + assert(isCXXRecordTypedCall(E)); assert(C && (isa(C) || // These are possible in C++17 due to mandatory copy elision. isa(C) || @@ -874,10 +878,10 @@ Elements.push_back(CFGConstructor(CE, CC), C); } - void appendCXXRecordTypedCall(CallExpr *CE, + void appendCXXRecordTypedCall(Expr *E, const ConstructionContext *CC, BumpVectorContext &C) { - Elements.push_back(CFGCXXRecordTypedCall(CE, CC), C); + Elements.push_back(CFGCXXRecordTypedCall(E, CC), C); } void appendInitializer(CXXCtorInitializer *initializer, Index: lib/Analysis/CFG.cpp =================================================================== --- lib/Analysis/CFG.cpp +++ lib/Analysis/CFG.cpp @@ -743,6 +743,19 @@ void addLocalScopeAndDtors(Stmt *S); + const ConstructionContext *retrieveAndCleanupConstructionContext(Expr *E) { + if (!BuildOpts.AddRichCXXConstructors) + return nullptr; + + const ConstructionContextLayer *Layer = ConstructionContextMap.lookup(E); + if (!Layer) + return nullptr; + + cleanupConstructionContext(E); + return ConstructionContext::createFromLayers(cfg->getBumpVectorContext(), + Layer); + } + // Interface to CFGBlock - adding CFGElements. void appendStmt(CFGBlock *B, const Stmt *S) { @@ -755,16 +768,10 @@ } void appendConstructor(CFGBlock *B, CXXConstructExpr *CE) { - if (BuildOpts.AddRichCXXConstructors) { - if (const ConstructionContextLayer *Layer = - ConstructionContextMap.lookup(CE)) { - cleanupConstructionContext(CE); - if (const auto *CC = ConstructionContext::createFromLayers( - cfg->getBumpVectorContext(), Layer)) { - B->appendConstructor(CE, CC, cfg->getBumpVectorContext()); - return; - } - } + if (const ConstructionContext *CC = + retrieveAndCleanupConstructionContext(CE)) { + B->appendConstructor(CE, CC, cfg->getBumpVectorContext()); + return; } // No valid construction context found. Fall back to statement. @@ -775,18 +782,10 @@ if (alwaysAdd(CE) && cachedEntry) cachedEntry->second = B; - if (BuildOpts.AddRichCXXConstructors) { - if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) { - if (const ConstructionContextLayer *Layer = - ConstructionContextMap.lookup(CE)) { - cleanupConstructionContext(CE); - if (const auto *CC = ConstructionContext::createFromLayers( - cfg->getBumpVectorContext(), Layer)) { - B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext()); - return; - } - } - } + if (const ConstructionContext *CC = + retrieveAndCleanupConstructionContext(CE)) { + B->appendCXXRecordTypedCall(CE, CC, cfg->getBumpVectorContext()); + return; } // No valid construction context found. Fall back to statement. @@ -809,6 +808,20 @@ B->appendMemberDtor(FD, cfg->getBumpVectorContext()); } + void appendObjCMessage(CFGBlock *B, ObjCMessageExpr *ME) { + if (alwaysAdd(ME) && cachedEntry) + cachedEntry->second = B; + + if (const ConstructionContext *CC = + retrieveAndCleanupConstructionContext(ME)) { + B->appendCXXRecordTypedCall(ME, CC, cfg->getBumpVectorContext()); + return; + } + + B->appendStmt(const_cast(ME), + cfg->getBumpVectorContext()); + } + void appendTemporaryDtor(CFGBlock *B, CXXBindTemporaryExpr *E) { B->appendTemporaryDtor(E, cfg->getBumpVectorContext()); } @@ -1254,6 +1267,8 @@ void CFGBuilder::consumeConstructionContext( const ConstructionContextLayer *Layer, Expr *E) { + assert((isa(E) || isa(E) || + isa(E)) && "Expression cannot construct an object!"); if (const ConstructionContextLayer *PreviouslyStoredLayer = ConstructionContextMap.lookup(E)) { (void)PreviouslyStoredLayer; @@ -1297,10 +1312,11 @@ case Stmt::CallExprClass: case Stmt::CXXMemberCallExprClass: case Stmt::CXXOperatorCallExprClass: - case Stmt::UserDefinedLiteralClass: { - auto *CE = cast(Child); - if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) - consumeConstructionContext(Layer, CE); + case Stmt::UserDefinedLiteralClass: + case Stmt::ObjCMessageExprClass: { + auto *E = cast(Child); + if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(E)) + consumeConstructionContext(Layer, E); break; } case Stmt::ExprWithCleanupsClass: { @@ -3605,7 +3621,7 @@ findConstructionContextsForArguments(ME); autoCreateBlock(); - appendStmt(Block, ME); + appendObjCMessage(Block, ME); return VisitChildren(ME); } Index: test/Analysis/cfg-rich-constructors.mm =================================================================== --- test/Analysis/cfg-rich-constructors.mm +++ test/Analysis/cfg-rich-constructors.mm @@ -15,6 +15,7 @@ @interface E {} -(void) foo: (D) d; +-(D) bar; @end // FIXME: Find construction context for the argument. @@ -39,3 +40,27 @@ void passArgumentIntoMessage(E *e) { [e foo: D()]; } + +// CHECK: void returnObjectFromMessage(E *e) +// CHECK: [B1] +// CHECK-NEXT: 1: e +// CHECK-NEXT: 2: [B1.1] (ImplicitCastExpr, LValueToRValue, E *) +// Double brackets trigger FileCheck variables, escape. +// CXX11-ELIDE-NEXT: 3: {{\[}}[B1.2] bar] (CXXRecordTypedCall, [B1.4], [B1.6], [B1.7]) +// CXX11-NOELIDE-NEXT: 3: {{\[}}[B1.2] bar] (CXXRecordTypedCall, [B1.4], [B1.6]) +// CXX11-NEXT: 4: [B1.3] (BindTemporary) +// CXX11-NEXT: 5: [B1.4] (ImplicitCastExpr, NoOp, const class D) +// CXX11-NEXT: 6: [B1.5] +// CXX11-NEXT: 7: [B1.6] (CXXConstructExpr, [B1.8], class D) +// CXX11-NEXT: 8: D d = [e bar]; +// CXX11-NEXT: 9: ~D() (Temporary object destructor) +// CXX11-NEXT: 10: [B1.8].~D() (Implicit destructor) +// Double brackets trigger FileCheck variables, escape. +// CXX17-NEXT: 3: {{\[}}[B1.2] bar] (CXXRecordTypedCall, [B1.5], [B1.4]) +// CXX17-NEXT: 4: [B1.3] (BindTemporary) +// CXX17-NEXT: 5: D d = [e bar]; +// CXX17-NEXT: 6: ~D() (Temporary object destructor) +// CXX17-NEXT: 7: [B1.5].~D() (Implicit destructor) +void returnObjectFromMessage(E *e) { + D d = [e bar]; +} Index: test/Analysis/lifetime-extension.mm =================================================================== --- test/Analysis/lifetime-extension.mm +++ test/Analysis/lifetime-extension.mm @@ -0,0 +1,64 @@ +// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -Wno-unused -std=c++17 -analyzer-checker=core,debug.ExprInspection -verify %s +// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -DMOVES -verify %s +// RUN: %clang_analyze_cc1 -Wno-unused -std=c++17 -analyzer-checker=core,debug.ExprInspection -DMOVES -verify %s + +void clang_analyzer_eval(bool); +void clang_analyzer_checkInlined(bool); + +template struct AddressVector { + T *buf[10]; + int len; + + AddressVector() : len(0) {} + + void push(T *t) { + buf[len] = t; + ++len; + } +}; + +class C { + AddressVector &v; + +public: + C(AddressVector &v) : v(v) { v.push(this); } + ~C() { v.push(this); } + +#ifdef MOVES + C(C &&c) : v(c.v) { v.push(this); } +#endif + + // Note how return-statements prefer move-constructors when available. + C(const C &c) : v(c.v) { +#ifdef MOVES + clang_analyzer_checkInlined(false); // no-warning +#else + v.push(this); +#endif + } // no-warning +}; + +@interface NSObject {} +@end; +@interface Foo: NSObject {} + -(C) make: (AddressVector &)v; +@end + +@implementation Foo +-(C) make: (AddressVector &)v { + return C(v); +} +@end + +void testReturnByValueFromMessage(Foo *foo) { + AddressVector v; + { + const C &c = [foo make: v]; + } + // 0. Construct the return value of -make (copy/move elided) and + // lifetime-extend it directly via reference 'c', + // 1. Destroy the temporary lifetime-extended by 'c'. + clang_analyzer_eval(v.len == 2); // expected-warning{{TRUE}} + clang_analyzer_eval(v.buf[0] == v.buf[1]); // expected-warning{{TRUE}} +}