From b6029b7ef449305ac58845e99306b9faa2d240e7 Mon Sep 17 00:00:00 2001 From: Devin Coughlin Date: Wed, 25 Nov 2015 22:35:37 +0000 Subject: [PATCH] [analyzer] Include block capture copy expressions in the CFG. This prevents spurious dead store warnings when a C++ lambda is casted to a block. I've also added several tests documenting our still-incomplete support for lambda-to-block casts. rdar://problem/22236293 llvm-svn: 254107 --- clang/lib/Analysis/CFG.cpp | 15 ++++++- clang/test/Analysis/blocks.mm | 75 ++++++++++++++++++++++++++++++++++ clang/test/Analysis/lambdas.mm | 42 ++++++++++++++++++- 3 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/blocks.mm diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index fba9a41f2322..cff15cb3fd80 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -460,6 +460,7 @@ private: CFGBlock *VisitImplicitCastExpr(ImplicitCastExpr *E, AddStmtChoice asc); CFGBlock *VisitIndirectGotoStmt(IndirectGotoStmt *I); CFGBlock *VisitLabelStmt(LabelStmt *L); + CFGBlock *VisitBlockExpr(BlockExpr *E, AddStmtChoice asc); CFGBlock *VisitLambdaExpr(LambdaExpr *E, AddStmtChoice asc); CFGBlock *VisitLogicalOperator(BinaryOperator *B); std::pair VisitLogicalOperator(BinaryOperator *B, @@ -1453,7 +1454,7 @@ CFGBlock *CFGBuilder::Visit(Stmt * S, AddStmtChoice asc) { return VisitBinaryOperator(cast(S), asc); case Stmt::BlockExprClass: - return VisitNoRecurse(cast(S), asc); + return VisitBlockExpr(cast(S), asc); case Stmt::BreakStmtClass: return VisitBreakStmt(cast(S)); @@ -2333,6 +2334,18 @@ CFGBlock *CFGBuilder::VisitLabelStmt(LabelStmt *L) { return LabelBlock; } +CFGBlock *CFGBuilder::VisitBlockExpr(BlockExpr *E, AddStmtChoice asc) { + CFGBlock *LastBlock = VisitNoRecurse(E, asc); + for (const BlockDecl::Capture &CI : E->getBlockDecl()->captures()) { + if (Expr *CopyExpr = CI.getCopyExpr()) { + CFGBlock *Tmp = Visit(CopyExpr); + if (Tmp) + LastBlock = Tmp; + } + } + return LastBlock; +} + CFGBlock *CFGBuilder::VisitLambdaExpr(LambdaExpr *E, AddStmtChoice asc) { CFGBlock *LastBlock = VisitNoRecurse(E, asc); for (LambdaExpr::capture_init_iterator it = E->capture_init_begin(), diff --git a/clang/test/Analysis/blocks.mm b/clang/test/Analysis/blocks.mm new file mode 100644 index 000000000000..5f93888d45d0 --- /dev/null +++ b/clang/test/Analysis/blocks.mm @@ -0,0 +1,75 @@ +// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -analyze -analyzer-checker=core -fblocks -analyzer-opt-analyze-nested-blocks -verify -x objective-c++ %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -analyze -analyzer-checker=core,debug.DumpCFG -fblocks -analyzer-opt-analyze-nested-blocks %s > %t 2>&1 +// RUN: FileCheck --input-file=%t %s + +// expected-no-diagnostics + +void testBlockWithoutCopyExpression(int i) { + // Captures i, with no copy expression. + (void)(^void() { + (void)i; + }); +} + +// CHECK-LABEL:void testBlockWithoutCopyExpression(int i) +// CHECK-NEXT: [B2 (ENTRY)] +// CHECK-NEXT: Succs (1): B1 + +// CHECK: [B1] +// CHECK-NEXT: 1: ^{ } +// CHECK-NEXT: 2: (void)([B1.1]) (CStyleCastExpr, ToVoid, void) +// CHECK-NEXT: Preds (1): B2 +// CHECK-NEXT: Succs (1): B0 + +// CHECK: [B0 (EXIT)] +// CHECK-NEXT: Preds (1): B1 + +struct StructWithCopyConstructor { + StructWithCopyConstructor(int i); + StructWithCopyConstructor(const StructWithCopyConstructor &s); +}; +void testBlockWithCopyExpression(StructWithCopyConstructor s) { + // Captures s, with a copy expression calling the copy constructor for StructWithCopyConstructor. + (void)(^void() { + (void)s; + }); +} + +// CHECK-LABEL:void testBlockWithCopyExpression(StructWithCopyConstructor s) +// CHECK-NEXT: [B2 (ENTRY)] +// CHECK-NEXT: Succs (1): B1 + +// CHECK: [B1] +// CHECK-NEXT: 1: s +// CHECK-NEXT: 2: [B1.1] (CXXConstructExpr, const struct StructWithCopyConstructor) +// CHECK-NEXT: 3: ^{ } +// CHECK-NEXT: 4: (void)([B1.3]) (CStyleCastExpr, ToVoid, void) +// CHECK-NEXT: Preds (1): B2 +// CHECK-NEXT: Succs (1): B0 + +// CHECK: [B0 (EXIT)] +// CHECK-NEXT: Preds (1): B1 + +void testBlockWithCaptureByReference() { + __block StructWithCopyConstructor s(5); + // Captures s by reference, so no copy expression. + (void)(^void() { + (void)s; + }); +} + +// CHECK-LABEL:void testBlockWithCaptureByReference() +// CHECK-NEXT: [B2 (ENTRY)] +// CHECK-NEXT: Succs (1): B1 + +// CHECK: [B1] +// CHECK-NEXT: 1: 5 +// CHECK-NEXT: 2: [B1.1] (CXXConstructExpr, struct StructWithCopyConstructor) +// CHECK-NEXT: 3: StructWithCopyConstructor s(5) __attribute__((blocks("byref"))); +// CHECK-NEXT: 4: ^{ } +// CHECK-NEXT: 5: (void)([B1.4]) (CStyleCastExpr, ToVoid, void) +// CHECK-NEXT: Preds (1): B2 +// CHECK-NEXT: Succs (1): B0 + +// CHECK: [B0 (EXIT)] +// CHECK-NEXT: Preds (1): B1 diff --git a/clang/test/Analysis/lambdas.mm b/clang/test/Analysis/lambdas.mm index f2cd5aefffec..1061bbf7160d 100644 --- a/clang/test/Analysis/lambdas.mm +++ b/clang/test/Analysis/lambdas.mm @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wno-objc-root-class -analyze -analyzer-checker=core,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fblocks -Wno-objc-root-class -analyze -analyzer-checker=core,deadcode,debug.ExprInspection -analyzer-config inline-lambdas=true -verify %s int clang_analyzer_eval(int); @@ -44,3 +44,43 @@ int clang_analyzer_eval(int); } @end + +int getValue(); +void useValue(int v); + +void castToBlockNoDeadStore() { + int v = getValue(); // no-warning + + (void)(void(^)())[v]() { // This capture should count as a use, so no dead store warning above. + }; +} + +void takesBlock(void(^block)()); + +void passToFunctionTakingBlockNoDeadStore() { + int v = 7; // no-warning + int x = 8; // no-warning + takesBlock([&v, x]() { + (void)v; + }); +} + +void castToBlockAndInline() { + int result = ((int(^)(int))[](int p) { + return p; + })(7); + + // FIXME: This should be TRUE. We're not handling lambda to block conversions + // properly in ExprEngine::VisitBlockExpr. + clang_analyzer_eval(result == 7); // expected-warning{{UNKNOWN}} +} + +void castLambdaInLocalBlock() { + // FIXME: This results in a spurious + // "Address of stack-allocated block declared on line XX returned to caller" warning + // because we're not handling lambda to block conversions properly in ExprEngine. + auto lambda = []{ }; // expected-warning {{Address of stack-allocated block declared on line}} + + void(^block)() = lambda; + (void)block; +}