[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
tomasz-kaminski-sonarsource marked an inline comment as done.
Closed by commit rGfeafbb9fda57: [analyzer] Differentiate lifetime extended 
temporaries (authored by tomasz-kaminski-sonarsource).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,171 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+s

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > I think strictly speaking this might be "unsound" resulting in false 
> > positives in scenarios like:
> > ```
> > void f(bool reset) {
> >   static T&& extended = getTemp();
> >   if (reset) {
> > extended = getTemp();
> > return;
> >   }
> >   consume(std::move(extended));
> >   f(true);
> >   extended.use();
> > }
> > ```
> > 
> > In case the call to `f(true)` is not inlined (or in other cases there is 
> > some aliasing the analyzer does not know about), we might not realize that 
> > the object is reset and might falsely report a use after move.
> > 
> > But I think this is probably sufficiently rare that we do not care about 
> > those. 
> Aren't such case already excluded by the 
> `isa(MR->getMemorySpace())`, in a same way as we do for 
> variables?
> In such case, we should be creating 
> `getCXXStaticLifetimeExtendedObjectRegion` so it will not be located on stack.
Whoops, indeed, sorry. 



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > Is this a good wording for static lifetime extended temporaries?
> Lifetime extended static temporaries should never enter here, as they are not 
> in `StackSpaceRegion`. 
> Previously we have had `CXXStaticTempObjects` and now they are turned into 
> `CXXStaticLifetimeExtendedObjectRegion`.
Ah, my bad.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

xazax.hun wrote:
> I think strictly speaking this might be "unsound" resulting in false 
> positives in scenarios like:
> ```
> void f(bool reset) {
>   static T&& extended = getTemp();
>   if (reset) {
> extended = getTemp();
> return;
>   }
>   consume(std::move(extended));
>   f(true);
>   extended.use();
> }
> ```
> 
> In case the call to `f(true)` is not inlined (or in other cases there is some 
> aliasing the analyzer does not know about), we might not realize that the 
> object is reset and might falsely report a use after move.
> 
> But I think this is probably sufficiently rare that we do not care about 
> those. 
Aren't such case already excluded by the 
`isa(MR->getMemorySpace())`, in a same way as we do for 
variables?
In such case, we should be creating `getCXXStaticLifetimeExtendedObjectRegion` 
so it will not be located on stack.



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

xazax.hun wrote:
> Is this a good wording for static lifetime extended temporaries?
Lifetime extended static temporaries should never enter here, as they are not 
in `StackSpaceRegion`. 
Previously we have had `CXXStaticTempObjects` and now they are turned into 
`CXXStaticLifetimeExtendedObjectRegion`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp:556
+  bool IsLocal =
+  isa_and_nonnull(MR) &&
+  isa(MR->getMemorySpace());

I think strictly speaking this might be "unsound" resulting in false positives 
in scenarios like:
```
void f(bool reset) {
  static T&& extended = getTemp();
  if (reset) {
extended = getTemp();
return;
  }
  consume(std::move(extended));
  f(true);
  extended.use();
}
```

In case the call to `f(true)` is not inlined (or in other cases there is some 
aliasing the analyzer does not know about), we might not realize that the 
object is reset and might falsely report a use after move.

But I think this is probably sufficiently rare that we do not care about those. 



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:101
+QualType Ty = LER->getValueType().getLocalUnqualifiedType();
+os << "stack memory associated with temporary object of type '";
+Ty.print(os, Ctx.getPrintingPolicy());

Is this a good wording for static lifetime extended temporaries?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

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

xazax.hun accepted this revision.
xazax.hun added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

tomasz-kaminski-sonarsource wrote:
> xazax.hun wrote:
> > Do we actually need this `Expr` in the memory region?
> Yes they are needed for region identification, as one variable my be lifetime 
> extending multiple temporary.
> For illustration consider following example from test:
> ```
>   auto const& viaReference = RefAggregate{10, Composite{}}; // extends `int`, 
> `Composite`, and `RefAggregate`
>   clang_analyzer_dump(viaReference);// expected-warning-re 
> {{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
>   clang_analyzer_dump(viaReference.rx); // expected-warning-re 
> {{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
>   clang_analyzer_dump(viaReference.ry); // expected-warning-re 
> {{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
> ```
> `viaReference` declaration is extending 3 temporaries, and they get 
> identified by expression.
Ah, OK. Makes sense, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 537092.
tomasz-kaminski-sonarsource added a comment.

Reformatting and link to revelant core issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,171 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& front() && { r

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added a comment.

In D151325#4470832 , @xazax.hun wrote:

> Overall looks good to me. I think the changes to the notes already make the 
> analyzer more useful so there are some observable benefits to this patch.

Also, user-after-move now correctly detect the use of of move-from lifetime 
extended temporaries.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked an inline comment as done.
tomasz-kaminski-sonarsource added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

xazax.hun wrote:
> Do we actually need this `Expr` in the memory region?
Yes they are needed for region identification, as one variable my be lifetime 
extending multiple temporary.
For illustration consider following example from test:
```
  auto const& viaReference = RefAggregate{10, Composite{}}; // extends `int`, 
`Composite`, and `RefAggregate`
  clang_analyzer_dump(viaReference);// expected-warning-re 
{{&lifetime_extended_object{RefAggregate, viaReference, S{{[0-9]+}}} }}
  clang_analyzer_dump(viaReference.rx); // expected-warning-re 
{{&lifetime_extended_object{int, viaReference, S{{[0-9]+}}} }}
  clang_analyzer_dump(viaReference.ry); // expected-warning-re 
{{&lifetime_extended_object{Composite, viaReference, S{{[0-9]+}}} }}
```
`viaReference` declaration is extending 3 temporaries, and they get identified 
by expression.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

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

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

Overall looks good to me. I think the changes to the notes already make the 
analyzer more useful so there are some observable benefits to this patch.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h:1282
+  LLVM_ATTRIBUTE_RETURNS_NONNULL
+  const Expr *getExpr() const { return Ex; }
+  LLVM_ATTRIBUTE_RETURNS_NONNULL

Do we actually need this `Expr` in the memory region?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Ping @NoQ @xazax.hun


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 527291.
tomasz-kaminski-sonarsource added a comment.

Rebasing on main


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& front() && { return static_cast(array[0]); 

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 527027.
tomasz-kaminski-sonarsource added a comment.

Added function to query stack frame for temporary object.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& fr

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 525978.
tomasz-kaminski-sonarsource added a comment.

Fixed formatting to match clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& front() && { return s

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 525961.
tomasz-kaminski-sonarsource added a comment.

Remove excessive reformating


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& front() && { return static_cast

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 525950.
tomasz-kaminski-sonarsource added a comment.

Updated MoveChecker to handle CXXLifetimeExtendedObjects


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/MoveChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp
  clang/test/Analysis/use-after-move.cpp

Index: clang/test/Analysis/use-after-move.cpp
===
--- clang/test/Analysis/use-after-move.cpp
+++ clang/test/Analysis/use-after-move.cpp
@@ -613,6 +613,14 @@
   }
 }
 
+void lifeTimeExtendTest() {
+  A&& a = A{};
+  A b = std::move(a); // peaceful-note {{Object is moved}}
+
+  a.foo(); // peaceful-warning {{Method called on moved-from object}}
+   // peaceful-note@-1 {{Method called on moved-from object}}
+}
+
 void interFunTest1(A &a) {
   a.bar(); // peaceful-warning {{Method called on moved-from object 'a'}}
// peaceful-note@-1 {{Method called on moved-from object 'a'}}
Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-analyzer-config elide-constructors=false\
+// RUN:-Wno-dangling -Wno-c++1z-extensions\
+// RUN:-verify=expected,cpp14\
+// RUN:-x c++ -std=c++14 %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangling -verify=expected,cpp17\
+// RUN:-x c++ -std=c++17 %s
+
+template
+void clang_analyzer_dump(T&&) {}
+
+template
+T create() { return T{}; }
+
+template
+T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
+
+struct Composite {
+  int x;
+  int y;
+};
+
+struct Derived : Composite {
+  int z;
+};
+
+template
+struct Array {
+  T array[20];
+
+  T&& fro

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added a comment.

In D151325#4374326 , @NoQ wrote:

> I totally support this! There's no reason why the region wouldn't carry such 
> information.
>
> This patch is surprisingly tiny, but this matches my grep observations: 
> almost nothing in the static analyzer treats `CXXTempObjectRegion` specially. 
> Even in case of live symbols/regions analysis, it simply relies on how 
> temporary region is bound in the Store to the lifetime-extending reference, 
> so it doesn't need to be handled explicitly.

Indeed. However, I think that this could be a side product of 
`CXXTempObjectRegion` not representing actual short lived temporaries.

> I see that there's no change in `MoveChecker` which treats 
> `CXXTempObjectRegion` specially, and I suspect it may start emitting more 
> warnings after this patch, but I'm not sure if it's a good thing or a bad 
> thing. Could be worth experimenting (eg. by analyzing LLVM itself, it has 
> enough move semantics to demonstrate potential problems).

I have performed our internal test, which includes LLVM, and it has no changes 
in the results due to this patch. 
For the `MoveChecker`, after inspecting the code I believe that we should 
exclude only `CXXTempObjectsRegions` from that checker, as it is certainly 
possible to perform operations on moved from lifetime extended object.

> Do you have any preferences on creating a common base class for 
> `CXXTempObjectRegion` and `CXXLifetimeExtendedObjectRegion`? Or, as an 
> opposite extreme, simply add a nullable `ExD` field to `CXXTempObjectRegion` 
> instead of making a new class? Both could save us some code when these 
> regions do need to be treated uniformly, but it doesn't look like a popular 
> situation to worry about.

Despite them, both being referred to as temporary, they are very different in 
nature. Thus I believe they should be considered separately by each checker, 
thus I prefer them to not be related. This is why I skipped `Temp` word in the 
name of the region.
If any, I would group them with `VarRegion`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

I totally support this! There's no reason why the region wouldn't carry such 
information.

This patch is surprisingly tiny, but this matches my grep observations: almost 
nothing in the static analyzer treats `CXXTempObjectRegion` specially. Even in 
case of live symbols/regions analysis, it simply relies on how temporary region 
is bound in the Store to the lifetime-extending reference, so it doesn't need 
to be handled explicitly.

I see that there's no change in `MoveChecker` which treats 
`CXXTempObjectRegion` specially, and I suspect it may start emitting more 
warnings after this patch, but I'm not sure if it's a good thing or a bad 
thing. Could be worth experimenting (eg. by analyzing LLVM itself, it has 
enough move semantics to demonstrate potential problems).

Do you have any preferences on creating a common base class for 
`CXXTempObjectRegion` and `CXXLifetimeExtendedObjectRegion`? Or, as an opposite 
extreme, simply add a nullable `ExD` field to `CXXTempObjectRegion` instead of 
making a new class? Both could save us some code when these regions do need to 
be treated uniformly, but it doesn't look like a popular situation to worry 
about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

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

xazax.hun added a comment.

I generally support the idea of a CXXLifetimeExtendedObj region, definitely 
cleaner than a CXXTempObjectRegion with static lifetime.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Please leave a link in the summary to the RFC to discuss.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D151325/new/

https://reviews.llvm.org/D151325

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

[PATCH] D151325: [analyzer] Differentiate lifetime extended temporaries

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
tomasz-kaminski-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch introduces a new `CXXLifetimeExtendedObjectRegion` as a 
representation
of the memory for the temporary object that is lifetime extended by the 
reference
to which they are bound.

This separation provides an ability to detect the use of dangling pointers
(either binding or dereference) in a robust manner.
For example, the `ref` is conditionally dangling in the following example:

  template
  T const& select(bool cond, T const& t, T const& u) { return cond ? t : u; }
  
  int const& le = Composite{}.x;
  auto&& ref = select(cond, le, 10);

Before the change, regardless of the value of `cond`, the `select()` call would
have returned a `temp_object` region.
With the proposed change we would produce a (non-dangling) 
`lifetime_extended_object`
region with lifetime bound to `le` or a `temp_object` region for the dangling 
case.

We believe that such separation is desired, as such lifetime extended 
temporaries
are closer to the variables. For example, they may have a static storage 
duration
(this patch removes a static temporary region, which was an abomination).
We also think that alternative approaches are not viable.

While for some cases it may be possible to determine if the region is lifetime
extended by searching the parents of the initializer expr, this quickly becomes
complex in the presence of the conditions operators like this one:

  Composite cc;
  // Ternary produces prvalue 'int' which is extended, as branches differ in 
value category
  auto&& x = cond ? Composite{}.x : cc.x;
  
  // Ternary produces xvalue, and extends the Composite object
  auto&& y = cond ? Composite{}.x : std::move(cc).x;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151325

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/Regions.def
  clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/lib/StaticAnalyzer/Core/Store.cpp
  clang/test/Analysis/lifetime-extended-regions.cpp
  clang/test/Analysis/stack-addr-ps.cpp

Index: clang/test/Analysis/stack-addr-ps.cpp
===
--- clang/test/Analysis/stack-addr-ps.cpp
+++ clang/test/Analysis/stack-addr-ps.cpp
@@ -30,13 +30,13 @@
 
 const int &get_reference2() {
   const int &x = get_value(); // expected-note {{binding reference variable 'x' here}}
-  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x' returned to caller}} expected-warning {{returning reference to local temporary}} 
 }
 
 const int &get_reference3() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning reference to local temporary}}
+  return x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning reference to local temporary}}
 }
 
 int global_var;
@@ -60,7 +60,7 @@
 const int *f4() {
   const int &x1 = get_value(); // expected-note {{binding reference variable 'x1' here}}
   const int &x2 = x1; // expected-note {{binding reference variable 'x2' here}}
-  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' returned}} expected-warning {{returning address of local temporary}}
+  return &x2; // expected-warning{{Address of stack memory associated with temporary object of type 'int' lifetime extended by local variable 'x1' returned to caller}} expected-warning {{returning address of local temporary}}
 }
 
 struct S {
Index: clang/test/Analysis/lifetime-extended-regions.cpp
===
--- /dev/null
+++ clang/test/Analysis/lifetime-extended-regions.cpp
@@ -0,0 +1,170 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.core\
+// RUN:-analyzer-checker=debug.ExprInspection\
+// RUN:-Wno-dangl