[LifetimeSafety] Add parameter lifetime tracking in CFG #169320
Merged
+109
−9
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
usx95
changed the title
usx95
force-pushed
the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
from
d64278c to
c9e98f4
Compare
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-analysis @llvm/pr-subscribers-clang-temporal-safety Author: Utkarsh Saxena (usx95) ChangesThis PR enhances the CFG builder to properly handle function parameters in lifetime analysis:
Previously, Clang's lifetime analysis was not properly tracking the lifetime of function parameters, causing it to miss important use-after-return bugs when parameter values were returned by reference or address. This change ensures that parameters are properly tracked in the CFG, allowing the analyzer to correctly identify when stack memory associated with parameters is returned. Fixes #169014 Full diff: https://github.com/llvm/llvm-project/pull/169320.diff 3 Files Affected:
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cdde849b0e026..c052af5e96b21 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1666,6 +1666,13 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) {
assert(Succ == &cfg->getExit());
Block = nullptr; // the EXIT block is empty. Create all other blocks lazily.
+ // Add parameters to the initial scope so to handle their dtos and lifetime
+ // ends.
+ LocalScope *paramScope = nullptr;
+ if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
+ for (ParmVarDecl *PD : FD->parameters())
+ paramScope = addLocalScopeForVarDecl(PD, paramScope);
+
if (BuildOpts.AddImplicitDtors)
if (const CXXDestructorDecl *DD = dyn_cast_or_null<CXXDestructorDecl>(D))
addImplicitDtorsForDestructor(DD);
@@ -2246,6 +2253,9 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD,
if (!VD->hasLocalStorage())
return Scope;
+ if (isa<ParmVarDecl>(VD) && VD->getType()->isReferenceType())
+ return Scope;
+
if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes &&
!needsAutomaticDestruction(VD)) {
assert(BuildOpts.AddImplicitDtors);
diff --git a/clang/test/Analysis/scopes-cfg-output.cpp b/clang/test/Analysis/scopes-cfg-output.cpp
index 6ed6f3638f75b..9c75492c33a42 100644
--- a/clang/test/Analysis/scopes-cfg-output.cpp
+++ b/clang/test/Analysis/scopes-cfg-output.cpp
@@ -1437,12 +1437,14 @@ void test_cleanup_functions() {
// CHECK-NEXT: 4: return;
// CHECK-NEXT: 5: CleanupFunction (cleanup_int)
// CHECK-NEXT: 6: CFGScopeEnd(i)
+// CHECK-NEXT: 7: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B2]
// CHECK-NEXT: 1: return;
// CHECK-NEXT: 2: CleanupFunction (cleanup_int)
// CHECK-NEXT: 3: CFGScopeEnd(i)
+// CHECK-NEXT: 4: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B3]
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index e80a05860389c..c08543c27778f 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -529,14 +529,14 @@ TriviallyDestructedClass* trivial_class_uar () {
return ptr; // expected-note {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
const int& return_parameter(int a) {
- return a;
+ return a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
int* return_pointer_to_parameter(int a) {
- return &a;
+ return &a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
const int& return_reference_to_parameter(int a)
@@ -788,9 +788,17 @@ const MyObj& lifetimebound_return_ref_to_local() {
// expected-note@-1 {{returned here}}
}
-// FIXME: Fails to diagnose UAR when a reference to a by-value param escapes via the return value.
View lifetimebound_return_of_by_value_param(MyObj stack_param) {
- return Identity(stack_param);
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+}
+
+void LambdaUARParam() {
+ auto lambda = [](MyObj stack_param) {
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+ };
+ lambda(MyObj{});
}
// FIXME: Fails to diagnose UAF when a reference to a by-value param escapes via an out-param.
|
Member
|
@llvm/pr-subscribers-clang Author: Utkarsh Saxena (usx95) ChangesThis PR enhances the CFG builder to properly handle function parameters in lifetime analysis:
Previously, Clang's lifetime analysis was not properly tracking the lifetime of function parameters, causing it to miss important use-after-return bugs when parameter values were returned by reference or address. This change ensures that parameters are properly tracked in the CFG, allowing the analyzer to correctly identify when stack memory associated with parameters is returned. Fixes #169014 Full diff: https://github.com/llvm/llvm-project/pull/169320.diff 3 Files Affected:
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cdde849b0e026..c052af5e96b21 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1666,6 +1666,13 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) {
assert(Succ == &cfg->getExit());
Block = nullptr; // the EXIT block is empty. Create all other blocks lazily.
+ // Add parameters to the initial scope so to handle their dtos and lifetime
+ // ends.
+ LocalScope *paramScope = nullptr;
+ if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
+ for (ParmVarDecl *PD : FD->parameters())
+ paramScope = addLocalScopeForVarDecl(PD, paramScope);
+
if (BuildOpts.AddImplicitDtors)
if (const CXXDestructorDecl *DD = dyn_cast_or_null<CXXDestructorDecl>(D))
addImplicitDtorsForDestructor(DD);
@@ -2246,6 +2253,9 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD,
if (!VD->hasLocalStorage())
return Scope;
+ if (isa<ParmVarDecl>(VD) && VD->getType()->isReferenceType())
+ return Scope;
+
if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes &&
!needsAutomaticDestruction(VD)) {
assert(BuildOpts.AddImplicitDtors);
diff --git a/clang/test/Analysis/scopes-cfg-output.cpp b/clang/test/Analysis/scopes-cfg-output.cpp
index 6ed6f3638f75b..9c75492c33a42 100644
--- a/clang/test/Analysis/scopes-cfg-output.cpp
+++ b/clang/test/Analysis/scopes-cfg-output.cpp
@@ -1437,12 +1437,14 @@ void test_cleanup_functions() {
// CHECK-NEXT: 4: return;
// CHECK-NEXT: 5: CleanupFunction (cleanup_int)
// CHECK-NEXT: 6: CFGScopeEnd(i)
+// CHECK-NEXT: 7: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B2]
// CHECK-NEXT: 1: return;
// CHECK-NEXT: 2: CleanupFunction (cleanup_int)
// CHECK-NEXT: 3: CFGScopeEnd(i)
+// CHECK-NEXT: 4: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B3]
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index e80a05860389c..c08543c27778f 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -529,14 +529,14 @@ TriviallyDestructedClass* trivial_class_uar () {
return ptr; // expected-note {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
const int& return_parameter(int a) {
- return a;
+ return a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
int* return_pointer_to_parameter(int a) {
- return &a;
+ return &a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
const int& return_reference_to_parameter(int a)
@@ -788,9 +788,17 @@ const MyObj& lifetimebound_return_ref_to_local() {
// expected-note@-1 {{returned here}}
}
-// FIXME: Fails to diagnose UAR when a reference to a by-value param escapes via the return value.
View lifetimebound_return_of_by_value_param(MyObj stack_param) {
- return Identity(stack_param);
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+}
+
+void LambdaUARParam() {
+ auto lambda = [](MyObj stack_param) {
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+ };
+ lambda(MyObj{});
}
// FIXME: Fails to diagnose UAF when a reference to a by-value param escapes via an out-param.
|
Member
|
@llvm/pr-subscribers-clang-static-analyzer-1 Author: Utkarsh Saxena (usx95) ChangesThis PR enhances the CFG builder to properly handle function parameters in lifetime analysis:
Previously, Clang's lifetime analysis was not properly tracking the lifetime of function parameters, causing it to miss important use-after-return bugs when parameter values were returned by reference or address. This change ensures that parameters are properly tracked in the CFG, allowing the analyzer to correctly identify when stack memory associated with parameters is returned. Fixes #169014 Full diff: https://github.com/llvm/llvm-project/pull/169320.diff 3 Files Affected:
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index cdde849b0e026..c052af5e96b21 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1666,6 +1666,13 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) {
assert(Succ == &cfg->getExit());
Block = nullptr; // the EXIT block is empty. Create all other blocks lazily.
+ // Add parameters to the initial scope so to handle their dtos and lifetime
+ // ends.
+ LocalScope *paramScope = nullptr;
+ if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D))
+ for (ParmVarDecl *PD : FD->parameters())
+ paramScope = addLocalScopeForVarDecl(PD, paramScope);
+
if (BuildOpts.AddImplicitDtors)
if (const CXXDestructorDecl *DD = dyn_cast_or_null<CXXDestructorDecl>(D))
addImplicitDtorsForDestructor(DD);
@@ -2246,6 +2253,9 @@ LocalScope* CFGBuilder::addLocalScopeForVarDecl(VarDecl *VD,
if (!VD->hasLocalStorage())
return Scope;
+ if (isa<ParmVarDecl>(VD) && VD->getType()->isReferenceType())
+ return Scope;
+
if (!BuildOpts.AddLifetime && !BuildOpts.AddScopes &&
!needsAutomaticDestruction(VD)) {
assert(BuildOpts.AddImplicitDtors);
diff --git a/clang/test/Analysis/scopes-cfg-output.cpp b/clang/test/Analysis/scopes-cfg-output.cpp
index 6ed6f3638f75b..9c75492c33a42 100644
--- a/clang/test/Analysis/scopes-cfg-output.cpp
+++ b/clang/test/Analysis/scopes-cfg-output.cpp
@@ -1437,12 +1437,14 @@ void test_cleanup_functions() {
// CHECK-NEXT: 4: return;
// CHECK-NEXT: 5: CleanupFunction (cleanup_int)
// CHECK-NEXT: 6: CFGScopeEnd(i)
+// CHECK-NEXT: 7: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B2]
// CHECK-NEXT: 1: return;
// CHECK-NEXT: 2: CleanupFunction (cleanup_int)
// CHECK-NEXT: 3: CFGScopeEnd(i)
+// CHECK-NEXT: 4: CFGScopeEnd(m)
// CHECK-NEXT: Preds (1): B3
// CHECK-NEXT: Succs (1): B0
// CHECK: [B3]
diff --git a/clang/test/Sema/warn-lifetime-safety.cpp b/clang/test/Sema/warn-lifetime-safety.cpp
index e80a05860389c..c08543c27778f 100644
--- a/clang/test/Sema/warn-lifetime-safety.cpp
+++ b/clang/test/Sema/warn-lifetime-safety.cpp
@@ -529,14 +529,14 @@ TriviallyDestructedClass* trivial_class_uar () {
return ptr; // expected-note {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
const int& return_parameter(int a) {
- return a;
+ return a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
-// FIXME: No lifetime warning for this as no expire facts are generated for parameters
int* return_pointer_to_parameter(int a) {
- return &a;
+ return &a; // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
}
const int& return_reference_to_parameter(int a)
@@ -788,9 +788,17 @@ const MyObj& lifetimebound_return_ref_to_local() {
// expected-note@-1 {{returned here}}
}
-// FIXME: Fails to diagnose UAR when a reference to a by-value param escapes via the return value.
View lifetimebound_return_of_by_value_param(MyObj stack_param) {
- return Identity(stack_param);
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+}
+
+void LambdaUARParam() {
+ auto lambda = [](MyObj stack_param) {
+ return Identity(stack_param); // expected-warning {{address of stack memory is returned later}}
+ // expected-note@-1 {{returned here}}
+ };
+ lambda(MyObj{});
}
// FIXME: Fails to diagnose UAF when a reference to a by-value param escapes via an out-param.
|
usx95
force-pushed
the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
from
c9e98f4 to
8b25d76
Compare
llvmbot
added
the
clang:dataflow
Xazax-hun
reviewed
Nov 24, 2025
Collaborator
There was a problem hiding this comment.
Could you add a test demonstrating in what order are the parameters' lifetime end when there are multiple parameters and double check if that is correct? Interestingly, I see different results across clang and gcc so I wonder if this one is not specified. But I think we should make an effort to analyze the same order that we use for execution.
usx95
force-pushed
the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
from
8b25d76 to
6a25dab
Compare
Contributor
Author
|
Right. This is unspecified and implementation dependent. GCC and Clang diverge here: https://godbolt.org/z/5qhz5cWjd |
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
usx95
force-pushed
the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
from
6a25dab to
9b1dbaf
Compare
usx95
requested review from
martinboehme
and removed request for
martinboehme
ymand
approved these changes
Nov 25, 2025
usx95
force-pushed
the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
from
9b1dbaf to
8187592
Compare
Contributor
Author
|
Happy to address any more comments post-commit. I will land this now. |
usx95
deleted the
users/usx95/11-24-fix-missing-lifetimeends-for-params
branch
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/206/builds/9589 Here is the relevant piece of the build log for the reference |
boomanaiden154
added a commit
to boomanaiden154/llvm-project
that referenced
this pull request
Nov 26, 2025
After the landing of llvm#169320, the clang CFG analyses are able to do slightly more analysis around destructors. This results in thread safety also seeing slightly more destructors. This exposed a bug in ThreadSafety, where we would call getDestructorDecl, which can return nullptr for base class destructors, but not do a null pointer check, resulting in a segmentation fault. This patch fixes the issue by adding a null pointer check and adds a regression test so this gets caught before downstream integration testing in the future.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR enhances the CFG builder to properly handle function parameters in lifetime analysis:
FunctionDeclandBlockDecltypesPreviously, Clang's lifetime analysis was not properly tracking the lifetime of function parameters, causing it to miss important use-after-return bugs when parameter values were returned by reference or address. This change ensures that parameters are properly tracked in the CFG, allowing the analyzer to correctly identify when stack memory associated with parameters is returned.
Fixes #169014