[clang] Diagnose misuse of the cleanup attribute (PR #80040)

Bhuminjay Soni via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 30 09:55:25 PST 2024


https://github.com/11happy created https://github.com/llvm/llvm-project/pull/80040

**Overview:**
This pull request fixes #79443 when the cleanup attribute is intended to be applied to a variable declaration, passing its address to a specified function. The problem arises when standard functions like free, closedir, fclose, etc., are used incorrectly with this attribute, leading to incorrect behavior. 

**Testing:**
- Tested the updated code.
- Verified that other functionalities remain unaffected.

![Screenshot from 2024-01-30 23-15-51](https://github.com/llvm/llvm-project/assets/76656712/5d6dad20-2c8c-48e4-b6d6-dcabefe378ad)

![Screenshot from 2024-01-30 23-22-17](https://github.com/llvm/llvm-project/assets/76656712/e046e308-f93b-4ea2-abe2-ac7630a64a89)



**Dependencies:**
- No dependencies on other pull requests.

**CC:**
- @AaronBallman 

>From 93adb872d0e18ff3a1356ab47527d81b61c920cd Mon Sep 17 00:00:00 2001
From: 11happy <soni5happy at gmail.com>
Date: Tue, 30 Jan 2024 23:19:04 +0530
Subject: [PATCH] Diagnose misuse of the cleanup attribute

Signed-off-by: 11happy <soni5happy at gmail.com>
---
 .../include/clang/Basic/DiagnosticSemaKinds.td  |  4 ++++
 clang/include/clang/Sema/Sema.h                 |  2 ++
 clang/lib/Sema/SemaDeclAttr.cpp                 |  7 +++++++
 clang/lib/Sema/SemaExpr.cpp                     | 17 +++++++++++++++++
 4 files changed, 30 insertions(+)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 24d32cb87c89e..99ef803b1e0ec 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8265,6 +8265,10 @@ def warn_condition_is_assignment : Warning<"using the result of an "
 def warn_free_nonheap_object
   : Warning<"attempt to call %0 on non-heap %select{object %2|object: block expression|object: lambda-to-function-pointer conversion}1">,
     InGroup<FreeNonHeapObject>;
+def warn_free_called_on_unallocated_object : Warning<
+  "'%0' called on unallocated object '%1'">,
+  InGroup<FreeNonHeapObject>;
+  
 
 // Completely identical except off by default.
 def warn_condition_is_idiomatic_assignment : Warning<"using the result "
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index b5946e3f3178f..535c479aeb7c5 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12932,6 +12932,8 @@ class Sema final {
 
   bool IsStringLiteralToNonConstPointerConversion(Expr *From, QualType ToType);
 
+  bool IsPointerToPointer(QualType LHSType, QualType RHSType);
+
   bool CheckExceptionSpecCompatibility(Expr *From, QualType ToType);
 
   ExprResult PerformImplicitConversion(Expr *From, QualType ToType,
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 069571fcf7864..e149f745cc2f9 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3782,6 +3782,13 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD));
+
+  if (S.IsPointerToPointer(ParamTy, Ty)) {
+    VarDecl *VD = cast<VarDecl>(D);
+    S.Diag(Loc, diag::warn_free_called_on_unallocated_object)
+        << NI.getName() << VD;
+    return;
+  }
 }
 
 static void handleEnumExtensibilityAttr(Sema &S, Decl *D,
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 2f1ddfb215116..255e0be3cc842 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10098,6 +10098,23 @@ static bool isVector(QualType QT, QualType ElementType) {
   return false;
 }
 
+bool Sema::IsPointerToPointer(QualType LHSType, QualType RHSType) {
+  if (const PointerType *LHSPointer = dyn_cast<PointerType>(LHSType)) {
+    // Check if LHS is a single pointer, not a pointer to a pointer.
+    if (!isa<PointerType>(LHSPointer->getPointeeType())) {
+      if (isa<PointerType>(RHSType)) {
+        if (const PointerType *RHSPtr = dyn_cast<PointerType>(RHSType)) {
+          // If RHSType is a pointer to a pointer type, return True
+          if (isa<PointerType>(RHSPtr->getPointeeType())) {
+            return true;
+          }
+        }
+      }
+    }
+  }
+  return false;
+}
+
 /// CheckAssignmentConstraints (C99 6.5.16) - This routine currently
 /// has code to accommodate several GCC extensions when type checking
 /// pointers. Here are some objectionable examples that GCC considers warnings:



More information about the cfe-commits mailing list