[clang] [Clang][Sema] Disallow applying `onwership_returns` to functions that return non-pointers (PR #99564)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Pavel Skripkin (pskrgag)

<details>
<summary>Changes</summary>

`onwership_returns`  works only with pointers, since it models user-defined memory allocation functions. Make semantics more clear and report an error if attribute is attached to wrong function.

Closes #<!-- -->99501

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


6 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+4) 
- (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) 
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+11) 
- (modified) clang/test/AST/attr-print-emit.cpp (+2-2) 
- (modified) clang/test/Sema/attr-ownership.c (+4-1) 
- (modified) clang/test/Sema/attr-ownership.cpp (+3-3) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e0e86af257a19..4ecdf41a59234 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -1308,6 +1308,10 @@ Crash and bug fixes
 Improvements
 ^^^^^^^^^^^^
 
+- Improved handling of ``__attribute__((ownership_returns(class, idx)))``. Now clang
+  reports an error if attribute is attached to a function that returns non-pointer
+  value
+
 Moved checkers
 ^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d60f32674ca3a..09d19e6e4b4fc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3324,6 +3324,8 @@ def err_attribute_invalid_implicit_this_argument : Error<
   "%0 attribute is invalid for the implicit this argument">;
 def err_ownership_type : Error<
   "%0 attribute only applies to %select{pointer|integer}1 arguments">;
+def err_ownership_takes_return_type : Error<
+  "'ownership_returns' attribute only applies to functions that return pointers">;
 def err_ownership_returns_index_mismatch : Error<
   "'ownership_returns' attribute index does not match; here it is %0">;
 def note_ownership_returns_index_mismatch : Note<
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 41295bfb3b94f..06fd5c756a94d 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -1481,6 +1481,17 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     break;
   }
 
+  // Allow only pointers to be return type for functions with ownership_takes
+  // attribute. This matches with current OwnershipAttr::Takes semantics
+  if (K == OwnershipAttr::Returns) {
+    QualType RetType = getFunctionOrMethodResultType(D);
+
+    if (!RetType->isPointerType()) {
+      S.Diag(AL.getLoc(), diag::err_ownership_takes_return_type) << AL;
+      return;
+    }
+  }
+
   IdentifierInfo *Module = AL.getArgAsIdent(0)->Ident;
 
   StringRef ModuleName = Module->getName();
diff --git a/clang/test/AST/attr-print-emit.cpp b/clang/test/AST/attr-print-emit.cpp
index 8c8a2b2080599..9c89764a3cac2 100644
--- a/clang/test/AST/attr-print-emit.cpp
+++ b/clang/test/AST/attr-print-emit.cpp
@@ -33,7 +33,7 @@ void ownt(int *, int *) __attribute__((ownership_takes(foo, 1, 2)));
 // CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
 void ownh(int *, int *) __attribute__((ownership_holds(foo, 1, 2)));
 // CHECK: void ownr(int) __attribute__((ownership_returns(foo, 1)));
-void ownr(int) __attribute__((ownership_returns(foo, 1)));
+void *ownr(int) __attribute__((ownership_returns(foo, 1)));
 
 // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
 void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 3, 2)));
@@ -66,7 +66,7 @@ class C {
   // CHECK: void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
   void ownh(int *, int *) __attribute__((ownership_holds(foo, 2, 3)));
   // CHECK: void ownr(int) __attribute__((ownership_returns(foo, 2)));
-  void ownr(int) __attribute__((ownership_returns(foo, 2)));
+  void *ownr(int) __attribute__((ownership_returns(foo, 2)));
 
   // CHECK: void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
   void awtt(int, int, ...) __attribute__((argument_with_type_tag(foo, 4, 3)));
diff --git a/clang/test/Sema/attr-ownership.c b/clang/test/Sema/attr-ownership.c
index 8157ba7145a24..28b7cf25a21d5 100644
--- a/clang/test/Sema/attr-ownership.c
+++ b/clang/test/Sema/attr-ownership.c
@@ -18,9 +18,12 @@ void *f12(float i, int k, int f, int *j) __attribute__((ownership_returns(foo, 4
 void f13(int *i, int *j) __attribute__((ownership_holds(foo, 1))) __attribute__((ownership_takes(foo, 2)));
 void f14(int i, int j, int *k) __attribute__((ownership_holds(foo, 3))) __attribute__((ownership_takes(foo, 3)));  // expected-error {{'ownership_takes' and 'ownership_holds' attributes are not compatible}}
 
-void f15(int, int)
+void *f15(int, int)
   __attribute__((ownership_returns(foo, 1)))  // expected-error {{'ownership_returns' attribute index does not match; here it is 1}}
   __attribute__((ownership_returns(foo, 2))); // expected-note {{declared with index 2 here}}
 void f16(int *i, int *j) __attribute__((ownership_holds(foo, 1))) __attribute__((ownership_holds(foo, 1))); // OK, same index
 void f17(void*) __attribute__((ownership_takes(__, 1)));
 void f18() __attribute__((ownership_takes(foo, 1)));  // expected-warning {{'ownership_takes' attribute only applies to non-K&R-style functions}}
+
+void f18(void) __attribute__((ownership_returns(foo)));  // expected-error {{'ownership_returns' attribute only applies to functions that return pointers}}
+int f19(void) __attribute__((ownership_returns(foo)));  // expected-error {{'ownership_returns' attribute only applies to functions that return pointers}}
diff --git a/clang/test/Sema/attr-ownership.cpp b/clang/test/Sema/attr-ownership.cpp
index 7381285e2da48..0626efa5aaf9a 100644
--- a/clang/test/Sema/attr-ownership.cpp
+++ b/clang/test/Sema/attr-ownership.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 %s -verify -fsyntax-only
 
 class C {
-  void f(int, int)
-      __attribute__((ownership_returns(foo, 2)))  // expected-error {{'ownership_returns' attribute index does not match; here it is 2}}
-      __attribute__((ownership_returns(foo, 3))); // expected-note {{declared with index 3 here}}
+  void *f(int, int)
+       __attribute__((ownership_returns(foo, 2)))  // expected-error {{'ownership_returns' attribute index does not match; here it is 2}}
+       __attribute__((ownership_returns(foo, 3))); // expected-note {{declared with index 3 here}}
 };

``````````

</details>


https://github.com/llvm/llvm-project/pull/99564