https://github.com/pskrgag updated https://github.com/llvm/llvm-project/pull/99564
>From 6b7ec7c95df16de5eb0fecf2d69befb5461d98a5 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin <paskrip...@gmail.com> Date: Thu, 18 Jul 2024 18:48:47 +0300 Subject: [PATCH 1/4] clang/sema: disallow ownership_returns for functions that return non-pointers ownership_takes expects an argument to a pointer and clang reports an error if it is not the case. Since pointers consumed by ownership_takes are produced by functions with ownership_returns attribute, it make sence to report an error if function does not return a pointer type. --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 ++ clang/lib/Sema/SemaDeclAttr.cpp | 11 +++++++++++ clang/test/AST/attr-print-emit.cpp | 4 ++-- clang/test/Sema/attr-ownership.c | 5 ++++- clang/test/Sema/attr-ownership.cpp | 6 +++--- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 810abe4f23e31..9e6ea80ac4e40 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3330,6 +3330,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 39675422e3f9f..810f0ddfa06fd 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 084624353315c..5fb28a1b4e66f 100644 --- a/clang/test/Sema/attr-ownership.c +++ b/clang/test/Sema/attr-ownership.c @@ -18,7 +18,7 @@ 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 @@ -28,3 +28,6 @@ void f18() __attribute__((ownership_takes(foo, 1))); // expected-warning {{'own int f19(void *) __attribute__((ownership_takes(foo, 1))) // expected-error {{'ownership_takes' attribute class does not match; here it is 'foo'}} __attribute__((ownership_takes(foo1, 1))); // expected-note {{declared with class 'foo1' here}} + +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}} }; >From 6daba0637bfca9f7c43a3e9153bc6c3e68cb44b0 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin <paskrip...@gmail.com> Date: Thu, 18 Jul 2024 22:28:15 +0300 Subject: [PATCH 2/4] add release note --- clang/docs/ReleaseNotes.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 88f4d09308e8e..49b7d67066682 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -231,6 +231,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 ^^^^^^^^^^^^^^ >From d82befab4849fa023e2f9feb3dbe7ac88fb41f34 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin <paskrip...@gmail.com> Date: Fri, 19 Jul 2024 20:02:17 +0300 Subject: [PATCH 3/4] Fixed wrong comment and fixed release note wording --- clang/docs/ReleaseNotes.rst | 6 +++--- clang/lib/Sema/SemaDeclAttr.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 49b7d67066682..816b358df8327 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -231,9 +231,9 @@ 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 +- Improved the handling of the `ownership_returns` attribute. Now, Clang reports an + error if the attribute is attached to a function that returns a non-pointer value. + Fixes (#GH99501) Moved checkers ^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 810f0ddfa06fd..a9c2b2696fcd2 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -1481,7 +1481,7 @@ static void handleOwnershipAttr(Sema &S, Decl *D, const ParsedAttr &AL) { break; } - // Allow only pointers to be return type for functions with ownership_takes + // Allow only pointers to be return type for functions with ownership_returns // attribute. This matches with current OwnershipAttr::Takes semantics if (K == OwnershipAttr::Returns) { QualType RetType = getFunctionOrMethodResultType(D); >From e685e6f8bfa610aed1ea1eae59c73270065beb73 Mon Sep 17 00:00:00 2001 From: Pavel Skripkin <paskrip...@gmail.com> Date: Fri, 19 Jul 2024 20:02:47 +0300 Subject: [PATCH 4/4] Fix attr-print-emit test after fixing ownership_returns semantics --- clang/test/AST/attr-print-emit.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/AST/attr-print-emit.cpp b/clang/test/AST/attr-print-emit.cpp index 9c89764a3cac2..d8e62ed5f6cd1 100644 --- a/clang/test/AST/attr-print-emit.cpp +++ b/clang/test/AST/attr-print-emit.cpp @@ -32,7 +32,7 @@ int *aa(int i) __attribute__((alloc_align(1))); 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))); +// CHECK: 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))); @@ -65,7 +65,7 @@ class C { void ownt(int *, int *) __attribute__((ownership_takes(foo, 2, 3))); // 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))); + // CHECK: 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))); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits