NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a.sidorin, george.karpenkov, szepet, 
rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, baloghadamsoftware.

This is similar to https://reviews.llvm.org/D45650. Constructors of 
pass-by-value arguments aren't similar to other temporary constructors, we 
should treat them differently. For now we don't handle them, so we shouldn't 
produce temporary-like construction contexts for them. 
https://reviews.llvm.org/D45650 fixed this bug for call-expressions, but 
construct-expressions and Objective-C message expressions both aren't 
call-expressions, so they need a separate fix.


Repository:
  rC Clang

https://reviews.llvm.org/D48249

Files:
  lib/Analysis/CFG.cpp
  lib/Analysis/ConstructionContext.cpp
  test/Analysis/temporaries.cpp
  test/Analysis/temporaries.mm

Index: test/Analysis/temporaries.mm
===================================================================
--- /dev/null
+++ test/Analysis/temporaries.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker core,cplusplus -verify %s
+
+// expected-no-diagnostics
+
+// Stripped down unique_ptr<int>
+struct IntPtr {
+  IntPtr(): i(new int) {}
+  IntPtr(IntPtr &&o): i(o.i) { o.i = nullptr; }
+  ~IntPtr() { delete i; }
+
+  int *i;
+};
+
+@interface Foo {}
+  -(void) foo: (IntPtr)arg;
+@end
+
+void bar(Foo *f) {
+  IntPtr ptr;
+  int *i = ptr.i;
+  [f foo: static_cast<IntPtr &&>(ptr)];
+  *i = 99; // no-warning
+}
Index: test/Analysis/temporaries.cpp
===================================================================
--- test/Analysis/temporaries.cpp
+++ test/Analysis/temporaries.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++03 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify -w -std=c++11 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -verify -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++11
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus,debug.ExprInspection -DTEMPORARY_DTORS -w -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true %s -std=c++17
 
 // Note: The C++17 run-line doesn't -verify yet - it is a no-crash test.
 
@@ -945,3 +945,28 @@
 const C &bar1() { return foo1(); } // no-crash
 C &&bar2() { return foo2(); } // no-crash
 } // end namespace pass_references_through
+
+
+namespace ctor_argument {
+struct IntPtr {
+  IntPtr(): i(new int) {}
+  IntPtr(IntPtr &&o): i(o.i) { o.i = 0; }
+  ~IntPtr() { delete i; }
+
+  int *i;
+};
+
+struct Foo {
+  Foo(IntPtr);
+  void bar();
+
+  IntPtr i;
+};
+
+void bar() {
+  IntPtr ptr;
+  int *i = ptr.i;
+  Foo f(static_cast<IntPtr &&>(ptr));
+  *i = 99; // no-warning
+}
+} // namespace ctor_argument
Index: lib/Analysis/ConstructionContext.cpp
===================================================================
--- lib/Analysis/ConstructionContext.cpp
+++ lib/Analysis/ConstructionContext.cpp
@@ -15,6 +15,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "clang/Analysis/ConstructionContext.h"
+#include "clang/AST/ExprObjC.h"
 
 using namespace clang;
 
@@ -111,7 +112,9 @@
         assert(ParentLayer->isLast());
 
         // This is a constructor into a function argument. Not implemented yet.
-        if (isa<CallExpr>(ParentLayer->getTriggerStmt()))
+        if (isa<CallExpr>(ParentLayer->getTriggerStmt()) ||
+            isa<CXXConstructExpr>(ParentLayer->getTriggerStmt()) ||
+            isa<ObjCMessageExpr>(ParentLayer->getTriggerStmt()))
           return nullptr;
         // This is C++17 copy-elided construction into return statement.
         if (auto *RS = dyn_cast<ReturnStmt>(ParentLayer->getTriggerStmt())) {
@@ -173,7 +176,9 @@
       return create<SimpleReturnedValueConstructionContext>(C, RS);
     }
     // This is a constructor into a function argument. Not implemented yet.
-    if (isa<CallExpr>(TopLayer->getTriggerStmt()))
+    if (isa<CallExpr>(TopLayer->getTriggerStmt()) ||
+        isa<CXXConstructExpr>(TopLayer->getTriggerStmt()) ||
+        isa<ObjCMessageExpr>(TopLayer->getTriggerStmt()))
       return nullptr;
     llvm_unreachable("Unexpected construction context with statement!");
   } else if (const CXXCtorInitializer *I = TopLayer->getTriggerInit()) {
Index: lib/Analysis/CFG.cpp
===================================================================
--- lib/Analysis/CFG.cpp
+++ lib/Analysis/CFG.cpp
@@ -569,6 +569,7 @@
   CFGBlock *VisitObjCAtTryStmt(ObjCAtTryStmt *S);
   CFGBlock *VisitObjCAutoreleasePoolStmt(ObjCAutoreleasePoolStmt *S);
   CFGBlock *VisitObjCForCollectionStmt(ObjCForCollectionStmt *S);
+  CFGBlock *VisitObjCMessageExpr(ObjCMessageExpr *E, AddStmtChoice asc);
   CFGBlock *VisitPseudoObjectExpr(PseudoObjectExpr *E);
   CFGBlock *VisitReturnStmt(ReturnStmt *R);
   CFGBlock *VisitSEHExceptStmt(SEHExceptStmt *S);
@@ -2102,6 +2103,9 @@
     case Stmt::ObjCForCollectionStmtClass:
       return VisitObjCForCollectionStmt(cast<ObjCForCollectionStmt>(S));
 
+    case Stmt::ObjCMessageExprClass:
+      return VisitObjCMessageExpr(cast<ObjCMessageExpr>(S), asc);
+
     case Stmt::OpaqueValueExprClass:
       return Block;
 
@@ -2384,12 +2388,16 @@
     if (!boundType.isNull()) calleeType = boundType;
   }
 
+  // A stub for the code that'll eventually be used for finding construction
+  // contexts for constructors of C++ object-type arguments passed into
+  // constructor C.
   // FIXME: Once actually implemented, this construction context layer should
-  // include the number of the argument as well.
-  for (auto Arg: C->arguments()) {
-    findConstructionContexts(
-        ConstructionContextLayer::create(cfg->getBumpVectorContext(), C), Arg);
-  }
+  // include the index of the argument as well.
+  for (auto Arg: C->arguments())
+    if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())
+      findConstructionContexts(
+          ConstructionContextLayer::create(cfg->getBumpVectorContext(), C),
+          Arg);
 
   // If this is a call to a no-return function, this stops the block here.
   bool NoReturn = getFunctionExtInfo(*calleeType).getNoReturn();
@@ -3581,6 +3589,25 @@
   return VisitStmt(S, AddStmtChoice::AlwaysAdd);
 }
 
+CFGBlock *CFGBuilder::VisitObjCMessageExpr(ObjCMessageExpr *ME,
+                                           AddStmtChoice asc) {
+  // A stub for the code that'll eventually be used for finding construction
+  // contexts for constructors of C++ object-type arguments passed into
+  // constructor C.
+  // FIXME: Once actually implemented, this construction context layer should
+  // include the index of the argument as well.
+  for (auto Arg: ME->arguments())
+    if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())
+      findConstructionContexts(
+          ConstructionContextLayer::create(cfg->getBumpVectorContext(), ME),
+          Arg);
+
+  autoCreateBlock();
+  appendStmt(Block, ME);
+
+  return VisitChildren(ME);
+}
+
 CFGBlock *CFGBuilder::VisitCXXThrowExpr(CXXThrowExpr *T) {
   // If we were in the middle of a block we stop processing that block.
   if (badCFG)
@@ -4245,6 +4272,17 @@
 
 CFGBlock *CFGBuilder::VisitCXXConstructExpr(CXXConstructExpr *C,
                                             AddStmtChoice asc) {
+  // A stub for the code that'll eventually be used for finding construction
+  // contexts for constructors of C++ object-type arguments passed into
+  // constructor C.
+  // FIXME: Once actually implemented, this construction context layer should
+  // include the index of the argument as well.
+  for (auto Arg: C->arguments())
+    if (Arg->getType()->getAsCXXRecordDecl() && !Arg->isGLValue())
+      findConstructionContexts(
+          ConstructionContextLayer::create(cfg->getBumpVectorContext(), C),
+          Arg);
+
   autoCreateBlock();
   appendConstructor(Block, C);
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to