Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][Sema] Warn on return of pointer/reference to compound literal #83741

Merged
merged 1 commit into from Mar 5, 2024

Conversation

JOE1994
Copy link
Member

@JOE1994 JOE1994 commented Mar 3, 2024

Emit a warning if pointer/reference to compound literal is returned from a function.

In C, compound literals in block scope are lvalues that have automatic storage duration. In C++, compound literals in block scope are temporaries.

In either case, returning a pointer/reference to a compound literal can cause a use-after-free bug.

Fixes #8678

Emit a warning if pointer/reference to compound literal is returned from a function.

In C, compound literals in block scope are lvalues that have automatic storage duration.
In C++, compound literals in block scope are temporaries.

In either case, returning a pointer/reference to a compound literal
can cause a use-after-free bug.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 3, 2024

@llvm/pr-subscribers-clang

Author: Youngsuk Kim (JOE1994)

Changes

Emit a warning if pointer/reference to compound literal is returned from a function.

In C, compound literals in block scope are lvalues that have automatic storage duration. In C++, compound literals in block scope are temporaries.

In either case, returning a pointer/reference to a compound literal can cause a use-after-free bug.

Fixes #8678


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

3 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1)
  • (modified) clang/lib/Sema/SemaInit.cpp (+12)
  • (modified) clang/test/Analysis/stack-addr-ps.c (+2-2)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 91105d4231f06a..f5c88b8ae5aade 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9898,7 +9898,7 @@ def err_lifetimebound_ctor_dtor : Error<
 // CHECK: returning address/reference of stack memory
 def warn_ret_stack_addr_ref : Warning<
   "%select{address of|reference to}0 stack memory associated with "
-  "%select{local variable|parameter}2 %1 returned">,
+  "%select{local variable|parameter|compound literal}2 %1 returned">,
   InGroup<ReturnStackAddress>;
 def warn_ret_local_temp_addr_ref : Warning<
   "returning %select{address of|reference to}0 local temporary object">,
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 0fd458837163e5..93b125382b164f 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7734,6 +7734,14 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
     break;
   }
 
+  case Stmt::CompoundLiteralExprClass: {
+    if (auto *CLE = dyn_cast<CompoundLiteralExpr>(Init)) {
+      if (!CLE->isFileScope())
+        Visit(Path, Local(CLE), RK);
+    }
+    break;
+  }
+
   // FIXME: Visit the left-hand side of an -> or ->*.
 
   default:
@@ -8289,6 +8297,10 @@ void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
         if (LK == LK_StmtExprResult)
           return false;
         Diag(DiagLoc, diag::warn_ret_addr_label) << DiagRange;
+      } else if (auto *CLE = dyn_cast<CompoundLiteralExpr>(L)) {
+        Diag(DiagLoc, diag::warn_ret_stack_addr_ref)
+            << Entity.getType()->isReferenceType() << CLE->getInitializer() << 2
+            << DiagRange;
       } else {
         Diag(DiagLoc, diag::warn_ret_local_temp_addr_ref)
          << Entity.getType()->isReferenceType() << DiagRange;
diff --git a/clang/test/Analysis/stack-addr-ps.c b/clang/test/Analysis/stack-addr-ps.c
index 26e1cc58350cab..e469396e1bb22a 100644
--- a/clang/test/Analysis/stack-addr-ps.c
+++ b/clang/test/Analysis/stack-addr-ps.c
@@ -20,13 +20,13 @@ int* f3(int x, int *y) {
 
 void* compound_literal(int x, int y) {
   if (x)
-    return &(unsigned short){((unsigned short)0x22EF)}; // expected-warning{{Address of stack memory}}
+    return &(unsigned short){((unsigned short)0x22EF)}; // expected-warning{{Address of stack memory}} expected-warning{{address of stack memory}}
 
   int* array[] = {};
   struct s { int z; double y; int w; };
   
   if (y)
-    return &((struct s){ 2, 0.4, 5 * 8 }); // expected-warning{{Address of stack memory}}
+    return &((struct s){ 2, 0.4, 5 * 8 }); // expected-warning{{Address of stack memory}} expected-warning{{address of stack memory}}
     
   
   void* p = &((struct s){ 42, 0.4, x ? 42 : 0 });

Copy link

github-actions bot commented Mar 3, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 0c90e8837a9e5f27985ccaf85120083db9e1b43e fc0cb9c44ec7945f1a88420675b667167908e07c -- clang/lib/Sema/SemaInit.cpp clang/test/Analysis/stack-addr-ps.c
View the diff from clang-format here.
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 93b125382b..1d81330b7e 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -7742,7 +7742,7 @@ static void visitLocalsRetainedByReferenceBinding(IndirectLocalPath &Path,
     break;
   }
 
-  // FIXME: Visit the left-hand side of an -> or ->*.
+    // FIXME: Visit the left-hand side of an -> or ->*.
 
   default:
     break;

Copy link
Contributor

@AtariDreams AtariDreams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this warning a duplicate of what is already here?

@JOE1994
Copy link
Member Author

JOE1994 commented Mar 3, 2024

Isn't this warning a duplicate of what is already here?

The existing warning is not emitted when clang is run without the -analyze flag.

@JOE1994 JOE1994 requested a review from shafik March 3, 2024 20:26
@JOE1994 JOE1994 merged commit 9dab2e3 into llvm:main Mar 5, 2024
6 of 7 checks passed
@JOE1994 JOE1994 deleted the warn_retStackAddr_compoundliteral branch March 5, 2024 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

return of C99 compound literal addresses doesn't yield warnings
3 participants