NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, george.karpenkov.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, rnkovacs,
szepet, baloghadamsoftware.
This patch extends https://reviews.llvm.org/D44120 to Objective-C messages that
can also sometimes return C++ objects (in Objective-C++), but aren't inheriting
from `CallExpr`. We'll now be able to properly destroy temporaries returned
from such messages and/or materialize them. The analyzer picks up the newly
added context automatically, as demonstrated by the new test case.
I removed the `getCallReturnType()` check because most of its branches aren't
useful in our case.
Repository:
rC Clang
https://reviews.llvm.org/D48608
Files:
include/clang/Analysis/CFG.h
lib/Analysis/CFG.cpp
test/Analysis/cfg-rich-constructors.mm
test/Analysis/lifetime-extension.mm
Index: test/Analysis/lifetime-extension.mm
===================================================================
--- /dev/null
+++ 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 <typename T> struct AddressVector {
+ T *buf[10];
+ int len;
+
+ AddressVector() : len(0) {}
+
+ void push(T *t) {
+ buf[len] = t;
+ ++len;
+ }
+};
+
+class C {
+ AddressVector<C> &v;
+
+public:
+ C(AddressVector<C> &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<C> &)v;
+@end
+
+@implementation Foo
+-(C) make: (AddressVector<C> &)v {
+ return C(v);
+}
+@end
+
+void testReturnByValueFromMessage(Foo *foo) {
+ AddressVector<C> 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}}
+}
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: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -755,7 +755,7 @@
cachedEntry->second = B;
if (BuildOpts.AddRichCXXConstructors) {
- if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context)) {
+ if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE)) {
if (const ConstructionContextLayer *Layer =
ConstructionContextMap.lookup(CE)) {
cleanupConstructionContext(CE);
@@ -788,6 +788,28 @@
B->appendMemberDtor(FD, cfg->getBumpVectorContext());
}
+ void appendObjCMessage(CFGBlock *B, ObjCMessageExpr *ME) {
+ if (alwaysAdd(ME) && cachedEntry)
+ cachedEntry->second = B;
+
+ if (BuildOpts.AddRichCXXConstructors) {
+ if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(ME)) {
+ if (const ConstructionContextLayer *Layer =
+ ConstructionContextMap.lookup(ME)) {
+ cleanupConstructionContext(ME);
+ if (const auto *CC = ConstructionContext::createFromLayers(
+ cfg->getBumpVectorContext(), Layer)) {
+ B->appendCXXRecordTypedCall(ME, CC, cfg->getBumpVectorContext());
+ return;
+ }
+ }
+ }
+ }
+
+ B->appendStmt(const_cast<ObjCMessageExpr *>(ME),
+ cfg->getBumpVectorContext());
+ }
+
void appendTemporaryDtor(CFGBlock *B, CXXBindTemporaryExpr *E) {
B->appendTemporaryDtor(E, cfg->getBumpVectorContext());
}
@@ -1233,6 +1255,8 @@
void CFGBuilder::consumeConstructionContext(
const ConstructionContextLayer *Layer, Expr *E) {
+ assert((isa<CXXConstructExpr>(E) || isa<CallExpr>(E) ||
+ isa<ObjCMessageExpr>(E)) && "Expression cannot construct an object!");
if (const ConstructionContextLayer *PreviouslyStoredLayer =
ConstructionContextMap.lookup(E)) {
(void)PreviouslyStoredLayer;
@@ -1277,10 +1301,11 @@
case Stmt::CallExprClass:
case Stmt::CXXMemberCallExprClass:
case Stmt::CXXOperatorCallExprClass:
- case Stmt::UserDefinedLiteralClass: {
- auto *CE = cast<CallExpr>(Child);
- if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(CE, *Context))
- consumeConstructionContext(Layer, CE);
+ case Stmt::UserDefinedLiteralClass:
+ case Stmt::ObjCMessageExprClass: {
+ auto *E = cast<Expr>(Child);
+ if (CFGCXXRecordTypedCall::isCXXRecordTypedCall(E))
+ consumeConstructionContext(Layer, E);
break;
}
case Stmt::ExprWithCleanupsClass: {
@@ -3603,7 +3628,7 @@
Arg);
autoCreateBlock();
- appendStmt(Block, ME);
+ appendObjCMessage(Block, ME);
return VisitChildren(ME);
}
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,15 @@
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<CallExpr>(E) || isa<ObjCMessageExpr>(E));
+ 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<TemporaryObjectConstructionContext>(C) ||
// These are possible in C++17 due to mandatory copy elision.
isa<ReturnedValueConstructionContext>(C) ||
@@ -878,10 +879,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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
