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.

We now properly bind return value of the trivial copy constructor
amd assigments of the empty objects. Such operations does not
perform any loads from the source, however they preserve identity
of the assigmend object:

  Empty e;
  auto& x = (e = Empty());
  clang_analyzer_dump(x); // &e, was Unknown


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155442

Files:
  clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
  clang/test/Analysis/ctor-trivial-copy.cpp


Index: clang/test/Analysis/ctor-trivial-copy.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/ctor-trivial-copy.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection 
-analyzer-config c++-inlining=constructors -verify %s
+
+
+template<typename T>
+void clang_analyzer_dump(T&);
+
+struct aggr {
+  int x;
+  int y;
+};
+
+struct empty {
+};
+
+void test_copy_return() {
+  aggr s1 = {1, 2};
+  aggr const& cr1 = aggr(s1);
+  clang_analyzer_dump(cr1); // expected-warning-re 
{{&lifetime_extended_object{aggr, cr1, S{{[0-9]+}}} }}
+
+  empty s2;
+  empty const& cr2 = empty{s2};
+  clang_analyzer_dump(cr2); // expected-warning-re 
{{&lifetime_extended_object{empty, cr2, S{{[0-9]+}}} }}
+}
+
+void test_assign_return() {
+  aggr s1 = {1, 2};
+  aggr d1;
+  clang_analyzer_dump(d1 = s1); // expected-warning {{&d1 }}
+
+  empty s2;
+  empty d2;
+  clang_analyzer_dump(d2 = s2); // expected-warning {{&d2 }} was Unknown
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -61,30 +61,30 @@
     AlwaysReturnsLValue = true;
   }
 
-  assert(ThisRD);
-  if (ThisRD->isEmpty()) {
-    // Do nothing for empty classes. Otherwise it'd retrieve an UnknownVal
-    // and bind it and RegionStore would think that the actual value
-    // in this region at this offset is unknown.
-    return;
-  }
-
   const LocationContext *LCtx = Pred->getLocationContext();
+  const Expr *CallExpr = Call.getOriginExpr();
 
   ExplodedNodeSet Dst;
   Bldr.takeNodes(Pred);
 
-  SVal V = Call.getArgSVal(0);
-
-  // If the value being copied is not unknown, load from its location to get
-  // an aggregate rvalue.
-  if (std::optional<Loc> L = V.getAs<Loc>())
-    V = Pred->getState()->getSVal(*L);
-  else
-    assert(V.isUnknownOrUndef());
+  assert(ThisRD);
+  if (!ThisRD->isEmpty()) {
+    // Load the source value only for non-empty classes.
+    // Otherwise it'd retrieve an UnknownVal
+    // and bind it and RegionStore would think that the actual value
+    // in this region at this offset is unknown.
+    SVal V = Call.getArgSVal(0);
 
-  const Expr *CallExpr = Call.getOriginExpr();
-  evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+    // If the value being copied is not unknown, load from its location to get
+    // an aggregate rvalue.
+    if (std::optional<Loc> L = V.getAs<Loc>())
+      V = Pred->getState()->getSVal(*L);
+    else
+      assert(V.isUnknownOrUndef());
+    evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+  } else {
+    Dst.Add(Pred);
+  }
 
   PostStmt PS(CallExpr, LCtx);
   for (ExplodedNode *N : Dst) {


Index: clang/test/Analysis/ctor-trivial-copy.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/ctor-trivial-copy.cpp
@@ -0,0 +1,33 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -analyzer-config c++-inlining=constructors -verify %s
+
+
+template<typename T>
+void clang_analyzer_dump(T&);
+
+struct aggr {
+  int x;
+  int y;
+};
+
+struct empty {
+};
+
+void test_copy_return() {
+  aggr s1 = {1, 2};
+  aggr const& cr1 = aggr(s1);
+  clang_analyzer_dump(cr1); // expected-warning-re {{&lifetime_extended_object{aggr, cr1, S{{[0-9]+}}} }}
+
+  empty s2;
+  empty const& cr2 = empty{s2};
+  clang_analyzer_dump(cr2); // expected-warning-re {{&lifetime_extended_object{empty, cr2, S{{[0-9]+}}} }}
+}
+
+void test_assign_return() {
+  aggr s1 = {1, 2};
+  aggr d1;
+  clang_analyzer_dump(d1 = s1); // expected-warning {{&d1 }}
+
+  empty s2;
+  empty d2;
+  clang_analyzer_dump(d2 = s2); // expected-warning {{&d2 }} was Unknown
+}
\ No newline at end of file
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
@@ -61,30 +61,30 @@
     AlwaysReturnsLValue = true;
   }
 
-  assert(ThisRD);
-  if (ThisRD->isEmpty()) {
-    // Do nothing for empty classes. Otherwise it'd retrieve an UnknownVal
-    // and bind it and RegionStore would think that the actual value
-    // in this region at this offset is unknown.
-    return;
-  }
-
   const LocationContext *LCtx = Pred->getLocationContext();
+  const Expr *CallExpr = Call.getOriginExpr();
 
   ExplodedNodeSet Dst;
   Bldr.takeNodes(Pred);
 
-  SVal V = Call.getArgSVal(0);
-
-  // If the value being copied is not unknown, load from its location to get
-  // an aggregate rvalue.
-  if (std::optional<Loc> L = V.getAs<Loc>())
-    V = Pred->getState()->getSVal(*L);
-  else
-    assert(V.isUnknownOrUndef());
+  assert(ThisRD);
+  if (!ThisRD->isEmpty()) {
+    // Load the source value only for non-empty classes.
+    // Otherwise it'd retrieve an UnknownVal
+    // and bind it and RegionStore would think that the actual value
+    // in this region at this offset is unknown.
+    SVal V = Call.getArgSVal(0);
 
-  const Expr *CallExpr = Call.getOriginExpr();
-  evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+    // If the value being copied is not unknown, load from its location to get
+    // an aggregate rvalue.
+    if (std::optional<Loc> L = V.getAs<Loc>())
+      V = Pred->getState()->getSVal(*L);
+    else
+      assert(V.isUnknownOrUndef());
+    evalBind(Dst, CallExpr, Pred, ThisVal, V, true);
+  } else {
+    Dst.Add(Pred);
+  }
 
   PostStmt PS(CallExpr, LCtx);
   for (ExplodedNode *N : Dst) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D155442: [analyze... Tomasz Kamiński via Phabricator via cfe-commits

Reply via email to