[analyzer] Correctly propagate ConstructionContextLayer thru ParenExpr

[tomasz-kaminski-sonarsource]

Summary

Previously, information about ConstructionContextLayer was not propagated thru causing the expression like:
Var c = (createVar());
To produce unrelated temporary for the createVar() result and conjure new symbol for value of c in C++17 mode.

[tomasz-kaminski-sonarsource]

Added // no warn comment to line that was raising warning before the fix.

[Szelethus]

This looks great!

clang/test/Analysis/NewDelete-checker-test.cpp

44–55

This mail adds some insight into the usage of c++-allocator-inlining:

https://lists.llvm.org/pipermail/cfe-dev/2020-February/064754.html

About the code in MallocChecker that handles this flag (NewDelete is a part of this as well):

Yes, we should remove the old code for c++-allocator-inlining=false. The worry we've had back then as that in the new mode we've disabled aggressive behavior of MallocChecker in which it reacted to some overloaded operator new invocations but i think this was the right thing to do and also nobody complained; also nothing prevents us from bringing back the old behavior in a much less confusing way.

As a response, D75432 removed all code that handed c++-allocator-inlinging=false. So these RUN lines can be removed I think.

57–60

This last RUN seems to be the same as the first.

[tomasz-kaminski-sonarsource]

clang/test/Analysis/NewDelete-checker-test.cpp

57–60

This checker enables NewDeleteLeaks, while first enables NewDelete. From what I have checked, this two seem to have different behavior.

[tomasz-kaminski-sonarsource]

Removed duplicated runs that were only differing by specifying c++-allocator-inlinging=true, which is the default value.

[tomasz-kaminski-sonarsource]

clang/test/Analysis/NewDelete-checker-test.cpp

44–55

I have removed new runs with the c++-allocator-inlinging=true, and checked other files that have duplicated runs, and removed them.

[steakhal]

Looks good to me.

[Szelethus]

clang/test/Analysis/NewDelete-checker-test.cpp

57–60

Oh, silly me. you're right. :)

[tomasz-kaminski-sonarsource]

Adjusted Clang.Analysis::NewDelete-path-notes.cpp.plist for removed lines.

[tomasz-kaminski-sonarsource]

I do not have commit rights. If there are no additional changes needed, would it be possible to you to commit the changes on my behalf?

[steakhal]

In D102835#2773818, @tomasz-kaminski-sonarsource wrote:

I do not have commit rights. If there are no additional changes needed, would it be possible to you to commit the changes on my behalf?

I wanted to wait for a couple of days to see if anyone else has something to say.
I'm gonna commit your change on your behalf on Monday if it's good for you.

clang/lib/Analysis/CFG.cpp

case Stmt::InitListExprClass: {
     auto *ILE = cast<InitListExpr>(Child);
     if (ILE->isTransparent()) {
       findConstructionContexts(Layer, ILE->getInit(0));
       break;
     }
     // TODO: Handle other cases. For now, fail to find construction contexts.
     break;
   }
+  case Stmt::ParenExprClass: {
+    // If expression is placed into parenthesis we should propagate the parent
+    // construction context to subexpressions.
+    auto *PE = cast<ParenExpr>(Child);
+    findConstructionContexts(Layer, PE->getSubExpr());
+    break;
+  }
   default:
     break;
   }
 }
 
 void CFGBuilder::cleanupConstructionContext(Expr *E) {
   assert(BuildOpts.AddRichCXXConstructors &&
          "We should not be managing construction contexts!");

clang/test/Analysis/NewDelete-checker-test.cpp

 // RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
 // RUN:   -verify=expected,newdelete \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDelete
 //
 // RUN: %clang_analyze_cc1 -DLEAKS -std=c++11 -fblocks %s \
 // RUN:   -verify=expected,newdelete,leak \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
 //
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks %s \
+// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN:   -verify=expected,leak \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
+//
+// RUN: %clang_analyze_cc1 -std=c++17 -fblocks %s \
 // RUN:   -verify=expected,newdelete \
 // RUN:   -analyzer-checker=core \
-// RUN:   -analyzer-checker=cplusplus.NewDelete \
-// RUN:   -analyzer-config c++-allocator-inlining=true
+// RUN:   -analyzer-checker=cplusplus.NewDelete
 //
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -DLEAKS -std=c++17 -fblocks %s \
 // RUN:   -verify=expected,newdelete,leak \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDelete \
-// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks \
-// RUN:   -analyzer-config c++-allocator-inlining=true
+// RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
 //
-// RUN: %clang_analyze_cc1 -std=c++11 -fblocks -verify %s \
+// RUN: %clang_analyze_cc1 -std=c++17 -fblocks -verify %s \
 // RUN:   -verify=expected,leak \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=cplusplus.NewDeleteLeaks
 
 #include "Inputs/system-header-simulator-cxx.h"
 
 typedef __typeof__(sizeof(int)) size_t;
 extern "C" void *malloc(size_t);
 extern "C" void free (void* ptr);
 int *global;
 
 //------------------
 // check for leaks
 //------------------
 
 //----- Standard non-placement operators
 void testGlobalOpNew() {
   void *p = operator new(0);
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalOpNewArray() {
   void *p = operator new[](0);
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNewExpr() {
   int *p = new int;
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNewExprArray() {
   int *p = new int[0];
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 //----- Standard nothrow placement operators
 void testGlobalNoThrowPlacementOpNewBeforeOverload() {
   void *p = operator new(0, std::nothrow);
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
 void testGlobalNoThrowPlacementExprNewBeforeOverload() {
   int *p = new(std::nothrow) int;
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
class shared_ptr {
     T *p;
     control_block *control;
 
   public:
     shared_ptr() : p(0), control(0) {}
     explicit shared_ptr(T *p) : p(p), control(new control_block) {
       control->retain();
     }
-    shared_ptr(shared_ptr &other) : p(other.p), control(other.control) {
+    shared_ptr(const shared_ptr &other) : p(other.p), control(other.control) {
       if (control)
           control->retain();
     }
     ~shared_ptr() {
       if (control && control->release() == 0) {
         delete p;
         delete control;
       }
void swap(shared_ptr &other) {
       other.p = tmp;
 
       control_block *ctrlTmp = control;
       control = other.control;
       other.control = ctrlTmp;
     }
   };
 
+  template <typename T, typename... Args>
+  shared_ptr<T> make_shared(Args &&...args) {
+    return shared_ptr<T>(new T(static_cast<Args &&>(args)...));
+  }
+
   void testSingle() {
     shared_ptr<int> a(new int);
     *a = 1;
   }
 
+  void testMake() {
+    shared_ptr<int> a = make_shared<int>();
+    *a = 1;
+  }
+
+  void testMakeInParens() {
+    shared_ptr<int> a = (make_shared<int>()); // no warn
+    *a = 1;
+  }
+
   void testDouble() {
     shared_ptr<int> a(new int);
     shared_ptr<int> b = a;
     *a = 1;
   }
 
   void testInvalidated() {
     shared_ptr<int> a(new int);

clang/test/Analysis/NewDelete-path-notes.cpp

 // RUN: %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
 // RUN:   -analyzer-config add-pop-up-notes=false \
 // RUN:   -analyzer-output=text -verify %s
 // RUN: %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
-// RUN:   -analyzer-config c++-allocator-inlining=true \
-// RUN:   -analyzer-config add-pop-up-notes=false \
-// RUN:   -analyzer-output=text -verify %s
-// RUN: %clang_analyze_cc1 \
-// RUN:   -analyzer-checker=cplusplus.NewDelete,unix.Malloc \
 // RUN:   -analyzer-config add-pop-up-notes=false \
 // RUN:   -analyzer-output=plist %s -o %t.plist
 // RUN: %normalize_plist <%t.plist | diff -ub \
 // RUN:   %S/Inputs/expected-plists/NewDelete-path-notes.cpp.plist -
 
 void test() {
   int *p = new int;
   // expected-note@-1 {{Memory is allocated}}

clang/test/Analysis/NewDeleteLeaks-PR19102.cpp

 // RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -verify %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,cplusplus.NewDeleteLeaks -analyzer-config c++-allocator-inlining=true -verify %s
 
 class A0 {};
 
 class A1 {
 public:
   A1(int);
 };