[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC322795: [analyzer] Suppress this pointer escape 
during construction. (authored by dergachev, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D41797

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/NewDeleteLeaks-PR19102.cpp


Index: test/Analysis/NewDeleteLeaks-PR19102.cpp
===
--- test/Analysis/NewDeleteLeaks-PR19102.cpp
+++ test/Analysis/NewDeleteLeaks-PR19102.cpp
@@ -1,4 +1,5 @@
 // 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 {};
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -672,8 +672,13 @@
 
 void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
RegionAndSymbolInvalidationTraits *ETraits) const {
-  if (Data)
-Values.push_back(loc::MemRegionVal(static_cast(Data)));
+  if (Data) {
+loc::MemRegionVal MV(static_cast(Data));
+if (SymbolRef Sym = MV.getAsSymbol(true))
+  ETraits->setTrait(Sym,
+RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+Values.push_back(MV);
+  }
 }
 
 void CXXConstructorCall::getInitialStackFrameContents(


Index: test/Analysis/NewDeleteLeaks-PR19102.cpp
===
--- test/Analysis/NewDeleteLeaks-PR19102.cpp
+++ test/Analysis/NewDeleteLeaks-PR19102.cpp
@@ -1,4 +1,5 @@
 // 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 {};
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -672,8 +672,13 @@
 
 void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
RegionAndSymbolInvalidationTraits *ETraits) const {
-  if (Data)
-Values.push_back(loc::MemRegionVal(static_cast(Data)));
+  if (Data) {
+loc::MemRegionVal MV(static_cast(Data));
+if (SymbolRef Sym = MV.getAsSymbol(true))
+  ETraits->setTrait(Sym,
+RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+Values.push_back(MV);
+  }
 }
 
 void CXXConstructorCall::getInitialStackFrameContents(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rC Clang

https://reviews.llvm.org/D41797



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Hi Artem,
I think that global suppression is fine. If one really wants to check such 
escapes, he can implement a separate callback for this.


Repository:
  rC Clang

https://reviews.llvm.org/D41797



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I am fine with suppressing the escape globally.
I did see some code in the wild where the constructors registered the objects 
with a (global) map.
But I think it is still easier to annotate code that does something 
unconventional than the other way around.


Repository:
  rC Clang

https://reviews.llvm.org/D41797



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41797: [analyzer] Suppress escape of this-pointer during construction.

[Artem Dergachev via Phabricator via cfe-commits]

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

This one's easy. Under `-analyzer-config c++-allocator-inlining=true`, since 
https://reviews.llvm.org/D41406, we've teached `MallocChecker` to use the new 
callback, `check::NewAllocator`, which fires between the end of the `operator 
new()` call and the beginning of the constructor call. Because the constructor 
call causes a pointer escape event, during which the pointer returned by 
`operator new()` immediately escapes, `MallocChecker` immediately loses track 
of the allocated symbol and becomes completely useless. In particular, 
`NewDeleteLeaks` suffers a lot.

I'm open to suggestions regarding whether we should suppress this sort of 
pointer escape on the checker side or globally.


Repository:
  rC Clang

https://reviews.llvm.org/D41797

Files:
  lib/StaticAnalyzer/Core/CallEvent.cpp
  test/Analysis/NewDeleteLeaks-PR19102.cpp


Index: test/Analysis/NewDeleteLeaks-PR19102.cpp
===
--- test/Analysis/NewDeleteLeaks-PR19102.cpp
+++ test/Analysis/NewDeleteLeaks-PR19102.cpp
@@ -1,4 +1,5 @@
 // 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 {};
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -672,8 +672,13 @@
 
 void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
RegionAndSymbolInvalidationTraits *ETraits) const {
-  if (Data)
-Values.push_back(loc::MemRegionVal(static_cast(Data)));
+  if (Data) {
+loc::MemRegionVal MV(static_cast(Data));
+if (SymbolRef Sym = MV.getAsSymbol(true))
+  ETraits->setTrait(Sym,
+RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+Values.push_back(MV);
+  }
 }
 
 void CXXConstructorCall::getInitialStackFrameContents(


Index: test/Analysis/NewDeleteLeaks-PR19102.cpp
===
--- test/Analysis/NewDeleteLeaks-PR19102.cpp
+++ test/Analysis/NewDeleteLeaks-PR19102.cpp
@@ -1,4 +1,5 @@
 // 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 {};
 
Index: lib/StaticAnalyzer/Core/CallEvent.cpp
===
--- lib/StaticAnalyzer/Core/CallEvent.cpp
+++ lib/StaticAnalyzer/Core/CallEvent.cpp
@@ -672,8 +672,13 @@
 
 void CXXConstructorCall::getExtraInvalidatedValues(ValueList ,
RegionAndSymbolInvalidationTraits *ETraits) const {
-  if (Data)
-Values.push_back(loc::MemRegionVal(static_cast(Data)));
+  if (Data) {
+loc::MemRegionVal MV(static_cast(Data));
+if (SymbolRef Sym = MV.getAsSymbol(true))
+  ETraits->setTrait(Sym,
+RegionAndSymbolInvalidationTraits::TK_SuppressEscape);
+Values.push_back(MV);
+  }
 }
 
 void CXXConstructorCall::getInitialStackFrameContents(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits