[LifetimeSafety] Add origin tracking for pointer dereference

[usx95]

Fix lifetime safety analysis for pointer dereference operations by properly tracking origin flow.

Added support in FactsGenerator::VisitUnaryOperator() to handle the dereference operator (UO_Deref). This change ensures that when a pointer is dereferenced, the origin of the pointer is properly propagated to the dereference expression.

[usx95]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• [LifetimeSafety] Track moved declarations to prevent false positives #170007
• [LifetimeSafety] Add origin tracking for pointer dereference #170006 👈 (View in Graphite)
• [LifetimeSafety] Add implicit tracking for STL functions #170005
• [LifetimeSafety] Implement multi-level origins #168344
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[github-actions]

🐧 Linux x64 Test Results

The build failed before running any tests. Click on a failure below to see the details.

tools/clang/lib/Analysis/LifetimeSafety/CMakeFiles/obj.clangAnalysisLifetimeSafety.dir/Origins.cpp.o

FAILED: tools/clang/lib/Analysis/LifetimeSafety/CMakeFiles/obj.clangAnalysisLifetimeSafety.dir/Origins.cpp.o
sccache /opt/llvm/bin/clang++ -DCLANG_EXPORTS -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GLIBCXX_USE_CXX11_ABI=1 -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/lib/Analysis/LifetimeSafety -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Analysis/LifetimeSafety -I/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/tools/clang/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/build/include -I/home/gha/actions-runner/_work/llvm-project/llvm-project/llvm/include -gmlt -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wno-pass-failed -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -std=c++17  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -MD -MT tools/clang/lib/Analysis/LifetimeSafety/CMakeFiles/obj.clangAnalysisLifetimeSafety.dir/Origins.cpp.o -MF tools/clang/lib/Analysis/LifetimeSafety/CMakeFiles/obj.clangAnalysisLifetimeSafety.dir/Origins.cpp.o.d -o tools/clang/lib/Analysis/LifetimeSafety/CMakeFiles/obj.clangAnalysisLifetimeSafety.dir/Origins.cpp.o -c /home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Analysis/LifetimeSafety/Origins.cpp
/home/gha/actions-runner/_work/llvm-project/llvm-project/clang/lib/Analysis/LifetimeSafety/Origins.cpp:97:46: error: use of undeclared identifier 'Depth'
97 |     return getOrCreateList(EC->getSubExpr(), Depth);
|                                              ^~~~~
1 error generated.

If these failures are unrelated to your changes (for example tests are broken or flaky at HEAD), please open an issue at https://github.com/llvm/llvm-project/issues and add the infrastructure label.

[llvmbot]

@llvm/pr-subscribers-clang-analysis
@llvm/pr-subscribers-clang-temporal-safety

@llvm/pr-subscribers-clang

Author: Utkarsh Saxena (usx95)

Changes

Fix lifetime safety analysis for pointer dereference operations by properly tracking origin flow.

Added support in FactsGenerator::VisitUnaryOperator() to handle the dereference operator (UO_Deref). This change ensures that when a pointer is dereferenced, the origin of the pointer is properly propagated to the dereference expression.

---

Full diff: https://github.com/llvm/llvm-project/pull/170006.diff

3 Files Affected:

• (modified) clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp (+4)
• (modified) clang/test/Sema/warn-lifetime-safety-dataflow.cpp (+6)
• (modified) clang/test/Sema/warn-lifetime-safety.cpp (+11-11)

diff --git a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
index 4f05d7ea94aa7..b27dcb6163449 100644
--- a/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
+++ b/clang/lib/Analysis/LifetimeSafety/FactsGenerator.cpp
@@ -218,6 +218,10 @@ void FactsGenerator::VisitUnaryOperator(const UnaryOperator *UO) {
     // origin of this UnaryOperator expression.
     killAndFlowOrigin(*UO, *SubExpr);
   }
+  if (UO->getOpcode() == UO_Deref) {
+    const Expr *SubExpr = UO->getSubExpr();
+    killAndFlowOrigin(*UO, *SubExpr);
+  }
 }
 
 void FactsGenerator::VisitReturnStmt(const ReturnStmt *RS) {
diff --git a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
index 6d5711deba1cf..6fc7c776f935c 100644
--- a/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
+++ b/clang/test/Sema/warn-lifetime-safety-dataflow.cpp
@@ -152,6 +152,12 @@ void pointer_indirection() {
 // CHECK-NEXT:       Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *)
 // CHECK-NEXT:       Src:  [[O_PP_INNER]] (Decl: pp, Type : int *)
 // CHECK:   OriginFlow:
+// CHECK-NEXT:       Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *&)
+// CHECK-NEXT:       Src:  {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int **)
+// CHECK:   OriginFlow:
+// CHECK-NEXT:       Dest: {{[0-9]+}} (Expr: UnaryOperator, Type : int *)
+// CHECK-NEXT:       Src:  {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *)
+// CHECK:   OriginFlow:
 // CHECK-NEXT:       Dest: {{[0-9]+}} (Expr: ImplicitCastExpr, Type : int *)
 // CHECK-NEXT:       Src:  {{[0-9]+}} (Expr: UnaryOperator, Type : int *)
 // CHECK:   OriginFlow:
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index e62c3b69b040b..f22c73cfeb784 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -596,10 +596,10 @@ const int* return_pointer_to_parameter_via_reference(int a, int b, bool cond) {
     const int* d = &c;
     return d;                     // expected-note 2 {{returned here}}
 }
-// FIXME: Dereference of a pointer does not track the reference.
+
 const int& return_pointer_to_parameter_via_reference_1(int a) {
-    const int* d = &a;
-    return *d;
+    const int* d = &a; // expected-warning {{address of stack memory is returned later}}
+    return *d;    // expected-note {{returned here}}
 }
 
 const int& get_ref_to_local() {
@@ -1118,24 +1118,24 @@ struct MyObjStorage {
   const MyObj *end() const { return objs + 1; }
 };
 
-// FIXME: Detect use-after-scope. Dereference pointer does not propagate the origins.
 void range_based_for_use_after_scope() {
   View v;
   {
     MyObjStorage s;
-    for (const MyObj &o : s) {
+    for (const MyObj &o : s) { // expected-warning {{object whose reference is captured does not live long enough}}
       v = o;
     }
-  }
-  v.use();
+  } // expected-note {{destroyed here}}
+  v.use(); // expected-note {{later used here}}
 }
-// FIXME: Detect use-after-return. Dereference pointer does not propagate the origins.
+
 View range_based_for_use_after_return() {
   MyObjStorage s;
-  for (const MyObj &o : s) {
-    return o;
+  for (const MyObj &o : s) { // expected-warning {{address of stack memory is returned later}}
+    return o;  // expected-note {{returned here}}
   }
-  return *s.begin();
+  return *s.begin();  // expected-warning {{address of stack memory is returned later}}
+                      // expected-note@-1 {{returned here}}
 }
 
 void range_based_for_not_reference() {

[Xazax-hun]

Looks great, thanks!

Do we want to have some built-in handling for overloaded operators as well? It could be a follow-up PR though.

I suspect that this PR might not address the case when the dereference is the LHS on an assignment (like *p = a). But I think that could be a follow-up PR. We might need to change the assignment handling for more complicated LHS expression to handle things like (cond? a : b) = p.

[usx95]

Do we want to have some built-in handling for overloaded operators as well? It could be a follow-up PR though.

Added some more tests for overloaded operators. I think they used to work before through CallExpr handling.

I suspect that this PR might not address the case when the dereference is the LHS on an assignment (like *p = a).

Right. This case is added in the tests as a TODO.

[ymand]

Nit: move to a switch now that you have 2 cases.