[clang] Diagnose problematic uses of constructor/destructor attribute (PR #67360)

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 26 08:49:36 PDT 2023


https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/67360

>From 6e9bcfa7fc5eb3da4592909b9e199215064df700 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Mon, 25 Sep 2023 15:03:56 -0400
Subject: [PATCH 1/4] Diagnose problematic uses of constructor/destructor
 attribute

Functions with these attributes will be automatically called before
main() or after main() exits gracefully, which means the functions
should not accept arguments or have a returned value (nothing can
provide an argument to the call in these cases, and nothing can use
the returned value), nor should they be allowed on a non-static member
function in C++.

Additionally, these reuse the same priority logic as the init_priority
attribute which explicitly reserved priorty values <= 100 or > 65535.
So we now diagnose use of reserved priorities the same as we do for the
init_priority attribute.
---
 clang/docs/ReleaseNotes.rst                   |  5 ++
 .../clang/Basic/DiagnosticSemaKinds.td        |  5 ++
 clang/lib/Sema/SemaDeclAttr.cpp               | 74 +++++++++++++------
 clang/test/CodeGen/2008-03-03-CtorAttrType.c  |  4 +-
 .../PowerPC/aix-constructor-attribute.c       | 20 ++---
 .../PowerPC/aix-destructor-attribute.c        | 30 +++-----
 .../CodeGenCXX/aix-constructor-attribute.cpp  | 20 ++---
 .../CodeGenCXX/aix-destructor-attribute.cpp   | 36 ++++-----
 clang/test/Sema/constructor-attribute.c       | 60 ++++++++++++---
 9 files changed, 147 insertions(+), 107 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 294b77a3e260908..a9ef2e2c378c7e3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@ Attribute Changes in Clang
   automatic diagnostic to use parameters of types that the format style
   supports but that are never the result of default argument promotion, such as
   ``float``. (`#59824: <https://github.com/llvm/llvm-project/issues/59824>`_)
+- The ``constructor`` and ``destructor`` attributes now diagnose when:
+  - the priority is not between 101 and 65535, inclusive,
+  - the function it is applied to accepts arguments or has a non-void return
+    type, or
+  - the function it is applied to is a non-static member function (C++).
 
 Improvements to Clang's diagnostics
 -----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f4eb02fd9570c2f..1c522500fedcf6d 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3176,6 +3176,11 @@ def err_alignas_underaligned : Error<
   "requested alignment is less than minimum alignment of %1 for type %0">;
 def warn_aligned_attr_underaligned : Warning<err_alignas_underaligned.Summary>,
   InGroup<IgnoredAttributes>;
+def err_ctor_dtor_attr_on_non_void_func : Error<
+  "%0 attribute can only be applied to a function which accepts no arguments "
+  "and has a 'void' return type">;
+def err_ctor_dtor_member_func : Error<
+  "%0 attribute cannot be applied to a member function">;
 def err_attribute_sizeless_type : Error<
   "%0 attribute cannot be applied to sizeless type %1">;
 def err_attribute_argument_n_type : Error<
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 090a54eedaa07d0..79a73ba4618c85d 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2352,26 +2352,61 @@ static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   D->addAttr(::new (S.Context) UnusedAttr(S.Context, AL));
 }
 
-static void handleConstructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
-  uint32_t priority = ConstructorAttr::DefaultPriority;
+static bool diagnoseInvalidPriority(Sema &S, uint32_t Priority,
+                                    const ParsedAttr &A,
+                                    SourceLocation PriorityLoc) {
+  // Only perform the priority check if the attribute is outside of a system
+  // header. Values <= 100 are reserved for the implementation, and libc++
+  // benefits from being able to specify values in that range.
+  if ((Priority < 101 || Priority > 65535) &&
+      !S.getSourceManager().isInSystemHeader(A.getLoc())) {
+    S.Diag(A.getLoc(), diag::err_attribute_argument_out_of_range)
+        << PriorityLoc << A << 101 << 65535;
+    A.setInvalid();
+    return true;
+  }
+  return false;
+}
+
+template <typename CtorDtorAttr>
+static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+  uint32_t Priority = CtorDtorAttr::DefaultPriority;
   if (S.getLangOpts().HLSL && AL.getNumArgs()) {
     S.Diag(AL.getLoc(), diag::err_hlsl_init_priority_unsupported);
     return;
   }
-  if (AL.getNumArgs() &&
-      !checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority))
-    return;
 
-  D->addAttr(::new (S.Context) ConstructorAttr(S.Context, AL, priority));
-}
+  // If we're given an argument for the priority, check that it's valid.
+  if (AL.getNumArgs()) {
+    if (!checkUInt32Argument(S, AL, AL.getArgAsExpr(0), Priority))
+      return;
 
-static void handleDestructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
-  uint32_t priority = DestructorAttr::DefaultPriority;
-  if (AL.getNumArgs() &&
-      !checkUInt32Argument(S, AL, AL.getArgAsExpr(0), priority))
+    // Ensure the priority is in a reasonable range.
+    if (diagnoseInvalidPriority(S, Priority, AL,
+                                AL.getArgAsExpr(0)->getExprLoc()))
+      return;
+  }
+
+  // Ensure the function we're attaching to is something that is sensible to
+  // automatically call before or after main(); it should accept no arguments
+  // and return no value (but it is not an error because it is theoretically
+  // possible to call the function during normal program execution and pass it
+  // valid values). It also cannot be a member function. We allow K&R C
+  // functions because that's a difficult edge case where it depends on how the
+  // function is defined as to whether it does or does not expect arguments.
+  auto *FD = cast<FunctionDecl>(D);
+  if (!FD->getReturnType()->isVoidType() ||
+      (FD->hasPrototype() && FD->getNumParams() != 0)) {
+    S.Diag(AL.getLoc(), diag::err_ctor_dtor_attr_on_non_void_func)
+        << AL << FD->getSourceRange();
     return;
+  } else if (auto *MD = dyn_cast<CXXMethodDecl>(FD); MD && MD->isInstance()) {
+    S.Diag(AL.getLoc(), diag::err_ctor_dtor_member_func)
+        << AL << FD->getSourceRange();
+    return;
+  }
 
-  D->addAttr(::new (S.Context) DestructorAttr(S.Context, AL, priority));
+  D->addAttr(::new (S.Context) CtorDtorAttr(S.Context, AL, Priority));
 }
 
 template <typename AttrTy>
@@ -3888,16 +3923,9 @@ static void handleInitPriorityAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     return;
   }
 
-  // Only perform the priority check if the attribute is outside of a system
-  // header. Values <= 100 are reserved for the implementation, and libc++
-  // benefits from being able to specify values in that range.
-  if ((prioritynum < 101 || prioritynum > 65535) &&
-      !S.getSourceManager().isInSystemHeader(AL.getLoc())) {
-    S.Diag(AL.getLoc(), diag::err_attribute_argument_out_of_range)
-        << E->getSourceRange() << AL << 101 << 65535;
-    AL.setInvalid();
+  if (diagnoseInvalidPriority(S, prioritynum, AL, E->getExprLoc()))
     return;
-  }
+
   D->addAttr(::new (S.Context) InitPriorityAttr(S.Context, AL, prioritynum));
 }
 
@@ -8930,13 +8958,13 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
     handlePassObjectSizeAttr(S, D, AL);
     break;
   case ParsedAttr::AT_Constructor:
-      handleConstructorAttr(S, D, AL);
+    handleCtorDtorAttr<ConstructorAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_Deprecated:
     handleDeprecatedAttr(S, D, AL);
     break;
   case ParsedAttr::AT_Destructor:
-      handleDestructorAttr(S, D, AL);
+    handleCtorDtorAttr<DestructorAttr>(S, D, AL);
     break;
   case ParsedAttr::AT_EnableIf:
     handleEnableIfAttr(S, D, AL);
diff --git a/clang/test/CodeGen/2008-03-03-CtorAttrType.c b/clang/test/CodeGen/2008-03-03-CtorAttrType.c
index dbd7bc0a7270953..efc219bf85e6f4e 100644
--- a/clang/test/CodeGen/2008-03-03-CtorAttrType.c
+++ b/clang/test/CodeGen/2008-03-03-CtorAttrType.c
@@ -1,6 +1,4 @@
 // RUN: %clang_cc1 %s -emit-llvm -o - | grep llvm.global_ctors
-int __attribute__((constructor)) foo(void) {
-  return 0;
-}
+void __attribute__((constructor)) foo(void) {}
 void __attribute__((constructor)) bar(void) {}
 
diff --git a/clang/test/CodeGen/PowerPC/aix-constructor-attribute.c b/clang/test/CodeGen/PowerPC/aix-constructor-attribute.c
index 7cd72a3276936ff..031deeb3ab257a8 100644
--- a/clang/test/CodeGen/PowerPC/aix-constructor-attribute.c
+++ b/clang/test/CodeGen/PowerPC/aix-constructor-attribute.c
@@ -7,18 +7,10 @@
 
 // CHECK: @llvm.global_ctors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @foo3, ptr null }, { i32, ptr, ptr } { i32 180, ptr @foo2, ptr null }, { i32, ptr, ptr } { i32 180, ptr @foo, ptr null }]
 
-int foo(void) __attribute__((constructor(180)));
-int foo2(void) __attribute__((constructor(180)));
-int foo3(void) __attribute__((constructor(65535)));
+void foo(void) __attribute__((constructor(180)));
+void foo2(void) __attribute__((constructor(180)));
+void foo3(void) __attribute__((constructor(65535)));
 
-int foo3(void) {
-  return 3;
-}
-
-int foo2(void) {
-  return 2;
-}
-
-int foo(void) {
-  return 1;
-}
+void foo3(void) {}
+void foo2(void) {}
+void foo(void) {}
diff --git a/clang/test/CodeGen/PowerPC/aix-destructor-attribute.c b/clang/test/CodeGen/PowerPC/aix-destructor-attribute.c
index cfb1fd7b0171a41..6c49b0f638e760d 100644
--- a/clang/test/CodeGen/PowerPC/aix-destructor-attribute.c
+++ b/clang/test/CodeGen/PowerPC/aix-destructor-attribute.c
@@ -12,28 +12,20 @@
 // RUN:     -fno-use-cxa-atexit -fregister-global-dtors-with-atexit < %s | \
 // RUN:   FileCheck --check-prefix=REGISTER %s
 
-int bar(void) __attribute__((destructor(100)));
-int bar2(void) __attribute__((destructor(65535)));
-int bar3(int) __attribute__((destructor(65535)));
+void bar(void) __attribute__((destructor(101)));
+void bar2(void) __attribute__((destructor(65535)));
+void bar3(void) __attribute__((destructor(65535)));
 
-int bar(void) {
-  return 1;
-}
+void bar(void) {}
+void bar2(void) {}
+void bar3(void) {}
 
-int bar2(void) {
-  return 2;
-}
+// NO-REGISTER: @llvm.global_dtors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @bar, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @bar2, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @bar3, ptr null }]
 
-int bar3(int a) {
-  return a;
-}
+// REGISTER: @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @__GLOBAL_init_101, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_init_65535, ptr null }]
+// REGISTER: @llvm.global_dtors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @__GLOBAL_cleanup_101, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_cleanup_65535, ptr null }]
 
-// NO-REGISTER: @llvm.global_dtors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 100, ptr @bar, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @bar2, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @bar3, ptr null }]
-
-// REGISTER: @llvm.global_ctors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 100, ptr @__GLOBAL_init_100, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_init_65535, ptr null }]
-// REGISTER: @llvm.global_dtors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 100, ptr @__GLOBAL_cleanup_100, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_cleanup_65535, ptr null }]
-
-// REGISTER: define internal void @__GLOBAL_init_100() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_init_101() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER:   %0 = call i32 @atexit(ptr @bar)
 // REGISTER:   ret void
@@ -46,7 +38,7 @@ int bar3(int a) {
 // REGISTER:   ret void
 // REGISTER: }
 
-// REGISTER: define internal void @__GLOBAL_cleanup_100() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_cleanup_101() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER:   %0 = call i32 @unatexit(ptr @bar)
 // REGISTER:   %needs_destruct = icmp eq i32 %0, 0
diff --git a/clang/test/CodeGenCXX/aix-constructor-attribute.cpp b/clang/test/CodeGenCXX/aix-constructor-attribute.cpp
index 9f1191278fcd34c..84c2b6722628344 100644
--- a/clang/test/CodeGenCXX/aix-constructor-attribute.cpp
+++ b/clang/test/CodeGenCXX/aix-constructor-attribute.cpp
@@ -8,9 +8,9 @@
 // CHECK: @llvm.global_ctors = appending global [4 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_Z4foo3v, ptr null }, { i32, ptr, ptr } { i32 180, ptr @_Z4foo2v, ptr null }, { i32, ptr, ptr } { i32 180, ptr @_Z3foov, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I__, ptr null }]
 // CHECK: @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__D_a, ptr null }]
 
-int foo() __attribute__((constructor(180)));
-int foo2() __attribute__((constructor(180)));
-int foo3() __attribute__((constructor(65535)));
+void foo() __attribute__((constructor(180)));
+void foo2() __attribute__((constructor(180)));
+void foo3() __attribute__((constructor(65535)));
 
 struct Test {
 public:
@@ -18,14 +18,6 @@ struct Test {
   ~Test() {}
 } t;
 
-int foo3() {
-  return 3;
-}
-
-int foo2() {
-  return 2;
-}
-
-int foo() {
-  return 1;
-}
+void foo3() {}
+void foo2() {}
+void foo() {}
diff --git a/clang/test/CodeGenCXX/aix-destructor-attribute.cpp b/clang/test/CodeGenCXX/aix-destructor-attribute.cpp
index 5ebea7a997c9db1..2782c7ee74939e5 100644
--- a/clang/test/CodeGenCXX/aix-destructor-attribute.cpp
+++ b/clang/test/CodeGenCXX/aix-destructor-attribute.cpp
@@ -17,29 +17,21 @@ struct test {
   ~test();
 } t;
 
-int bar() __attribute__((destructor(100)));
-int bar2() __attribute__((destructor(65535)));
-int bar3(int) __attribute__((destructor(65535)));
+void bar() __attribute__((destructor(101)));
+void bar2() __attribute__((destructor(65535)));
+void bar3() __attribute__((destructor(65535)));
 
-int bar() {
-  return 1;
-}
-
-int bar2() {
-  return 2;
-}
-
-int bar3(int a) {
-  return a;
-}
+void bar() {}
+void bar2() {}
+void bar3() {}
 
 // NO-REGISTER: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I__, ptr null }]
-// NO-REGISTER: @llvm.global_dtors = appending global [4 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 100, ptr @_Z3barv, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_Z4bar2v, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_Z4bar3i, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__D_a, ptr null }]
+// NO-REGISTER: @llvm.global_dtors = appending global [4 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @_Z3barv, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_Z4bar2v, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_Z4bar3v, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__D_a, ptr null }]
 
-// REGISTER: @llvm.global_ctors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I__, ptr null }, { i32, ptr, ptr } { i32 100, ptr @__GLOBAL_init_100, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_init_65535, ptr null }]
-// REGISTER: @llvm.global_dtors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__D_a, ptr null }, { i32, ptr, ptr } { i32 100, ptr @__GLOBAL_cleanup_100, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_cleanup_65535, ptr null }]
+// REGISTER: @llvm.global_ctors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__sub_I__, ptr null }, { i32, ptr, ptr } { i32 101, ptr @__GLOBAL_init_101, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_init_65535, ptr null }]
+// REGISTER: @llvm.global_dtors = appending global [3 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 65535, ptr @_GLOBAL__D_a, ptr null }, { i32, ptr, ptr } { i32 101, ptr @__GLOBAL_cleanup_101, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @__GLOBAL_cleanup_65535, ptr null }]
 
-// REGISTER: define internal void @__GLOBAL_init_100() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_init_101() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER:   %0 = call i32 @atexit(ptr @_Z3barv)
 // REGISTER:   ret void
@@ -48,11 +40,11 @@ int bar3(int a) {
 // REGISTER: define internal void @__GLOBAL_init_65535() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER:   %0 = call i32 @atexit(ptr @_Z4bar2v)
-// REGISTER:   %1 = call i32 @atexit(ptr @_Z4bar3i)
+// REGISTER:   %1 = call i32 @atexit(ptr @_Z4bar3v)
 // REGISTER:   ret void
 // REGISTER: }
 
-// REGISTER: define internal void @__GLOBAL_cleanup_100() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_cleanup_101() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER:   %0 = call i32 @unatexit(ptr @_Z3barv)
 // REGISTER:   %needs_destruct = icmp eq i32 %0, 0
@@ -68,12 +60,12 @@ int bar3(int a) {
 
 // REGISTER: define internal void @__GLOBAL_cleanup_65535() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
-// REGISTER:   %0 = call i32 @unatexit(ptr @_Z4bar3i)
+// REGISTER:   %0 = call i32 @unatexit(ptr @_Z4bar3v)
 // REGISTER:   %needs_destruct = icmp eq i32 %0, 0
 // REGISTER:   br i1 %needs_destruct, label %destruct.call, label %unatexit.call
 
 // REGISTER: destruct.call:
-// REGISTER:   call void @_Z4bar3i()
+// REGISTER:   call void @_Z4bar3v()
 // REGISTER:   br label %unatexit.call
 
 // REGISTER: unatexit.call:
diff --git a/clang/test/Sema/constructor-attribute.c b/clang/test/Sema/constructor-attribute.c
index 2317c7735bda586..8df5ac42378bdd8 100644
--- a/clang/test/Sema/constructor-attribute.c
+++ b/clang/test/Sema/constructor-attribute.c
@@ -1,16 +1,52 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-strict-prototypes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
 
-int x __attribute__((constructor)); // expected-warning {{'constructor' attribute only applies to functions}}
-int f(void) __attribute__((constructor));
-int f(void) __attribute__((constructor(1)));
-int f(void) __attribute__((constructor(1,2))); // expected-error {{'constructor' attribute takes no more than 1 argument}}
-int f(void) __attribute__((constructor(1.0))); // expected-error {{'constructor' attribute requires an integer constant}}
-int f(void) __attribute__((constructor(0x100000000))); // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
+int x1 __attribute__((constructor)); // expected-warning {{'constructor' attribute only applies to functions}}
+void f(void) __attribute__((constructor));
+void f(void) __attribute__((constructor(1)));   // expected-error {{'constructor' attribute requires integer constant between 101 and 65535 inclusive}}
+void f(void) __attribute__((constructor(1,2))); // expected-error {{'constructor' attribute takes no more than 1 argument}}
+void f(void) __attribute__((constructor(1.0))); // expected-error {{'constructor' attribute requires an integer constant}}
+void f(void) __attribute__((constructor(0x100000000))); // expected-error {{integer constant expression evaluates to value 4294967296 that cannot be represented in a 32-bit unsigned integer type}}
+void f(void) __attribute__((constructor(101)));
 
-int x __attribute__((destructor)); // expected-warning {{'destructor' attribute only applies to functions}}
-int f(void) __attribute__((destructor));
-int f(void) __attribute__((destructor(1)));
-int f(void) __attribute__((destructor(1,2))); // expected-error {{'destructor' attribute takes no more than 1 argument}}
-int f(void) __attribute__((destructor(1.0))); // expected-error {{'destructor' attribute requires an integer constant}}
+int x2 __attribute__((destructor)); // expected-warning {{'destructor' attribute only applies to functions}}
+void f(void) __attribute__((destructor));
+void f(void) __attribute__((destructor(1)));   // expected-error {{'destructor' attribute requires integer constant between 101 and 65535 inclusive}}
+void f(void) __attribute__((destructor(1,2))); // expected-error {{'destructor' attribute takes no more than 1 argument}}
+void f(void) __attribute__((destructor(1.0))); // expected-error {{'destructor' attribute requires an integer constant}}
+void f(void) __attribute__((destructor(101)));
 
-void knr() __attribute__((constructor));
+void knr1() __attribute__((constructor));
+void knr2() __attribute__((destructor));
+
+// Require a void return type
+int g(void) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+int h(void) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+
+// Require no parameters
+void i(int v) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+void j(int v) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+
+#ifdef __cplusplus
+struct S {
+  // Not allowed on a nonstatic member function, but is allowed on a static
+  // member function so long as it has no args/void return type.
+  void mem1() __attribute__((constructor)); // expected-error {{'constructor' attribute cannot be applied to a member function}}
+  void mem2() __attribute__((destructor));  // expected-error {{'destructor' attribute cannot be applied to a member function}}
+
+  static void nonmem1() __attribute__((constructor));
+  static void nonmem2() __attribute__((destructor));
+
+  static int nonmem3() __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+  static int nonmem4() __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+
+  static void nonmem5(int) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+  static void nonmem6(int) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+};
+#endif // __cplusplus
+
+# 1 "source.c" 1 3
+// Can use reserved priorities within a system header
+void f(void) __attribute__((constructor(1)));
+void f(void) __attribute__((destructor(1)));
+# 1 "source.c" 2

>From 5c9509d2765f2cc803757b2d151a672f8571ab15 Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 26 Sep 2023 09:46:59 -0400
Subject: [PATCH 2/4] Update based on review feedback:

* Updated some comments
* Added some named constants
* Diagnose consteval functions
* Better const correctness
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 ++
 clang/lib/Sema/SemaDeclAttr.cpp               | 31 ++++++++++++-------
 clang/test/Sema/constructor-attribute.c       |  5 ++-
 3 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1c522500fedcf6d..5cd05c7567d0158 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2864,6 +2864,8 @@ def warn_cxx11_compat_constexpr_body_multiple_return : Warning<
   InGroup<CXXPre14Compat>, DefaultIgnore;
 def note_constexpr_body_previous_return : Note<
   "previous return statement is here">;
+def err_ctordtor_attr_consteval : Error<
+  "%0 attribute cannot be applied to a 'consteval' function">;
 
 // C++20 function try blocks in constexpr
 def ext_constexpr_function_try_block_cxx20 : ExtWarn<
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 79a73ba4618c85d..9a5de7eda641473 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2355,13 +2355,16 @@ static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 static bool diagnoseInvalidPriority(Sema &S, uint32_t Priority,
                                     const ParsedAttr &A,
                                     SourceLocation PriorityLoc) {
+  constexpr uint32_t ReservedPriorityLower = 101, ReservedPriorityUpper = 65535;
+
   // Only perform the priority check if the attribute is outside of a system
   // header. Values <= 100 are reserved for the implementation, and libc++
-  // benefits from being able to specify values in that range.
-  if ((Priority < 101 || Priority > 65535) &&
+  // benefits from being able to specify values in that range. Values > 65535
+  // are reserved for historical reasons.
+  if ((Priority < ReservedPriorityLower || Priority > ReservedPriorityUpper) &&
       !S.getSourceManager().isInSystemHeader(A.getLoc())) {
     S.Diag(A.getLoc(), diag::err_attribute_argument_out_of_range)
-        << PriorityLoc << A << 101 << 65535;
+        << PriorityLoc << A << ReservedPriorityLower << ReservedPriorityUpper;
     A.setInvalid();
     return true;
   }
@@ -2389,24 +2392,28 @@ static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
 
   // Ensure the function we're attaching to is something that is sensible to
   // automatically call before or after main(); it should accept no arguments
-  // and return no value (but it is not an error because it is theoretically
-  // possible to call the function during normal program execution and pass it
-  // valid values). It also cannot be a member function. We allow K&R C
-  // functions because that's a difficult edge case where it depends on how the
-  // function is defined as to whether it does or does not expect arguments.
-  auto *FD = cast<FunctionDecl>(D);
+  // and return no value; we treat it as an error because it's a form of type
+  // system incompatibility. It also cannot be a member function. We allow K&R
+  // C functions because that's a difficult edge case where it depends on how
+  // the function is defined as to whether it does or does not expect arguments.
+  const auto *FD = cast<FunctionDecl>(D);
   if (!FD->getReturnType()->isVoidType() ||
       (FD->hasPrototype() && FD->getNumParams() != 0)) {
     S.Diag(AL.getLoc(), diag::err_ctor_dtor_attr_on_non_void_func)
         << AL << FD->getSourceRange();
     return;
-  } else if (auto *MD = dyn_cast<CXXMethodDecl>(FD); MD && MD->isInstance()) {
+  } else if (const auto *MD = dyn_cast<CXXMethodDecl>(FD);
+             MD && MD->isInstance()) {
     S.Diag(AL.getLoc(), diag::err_ctor_dtor_member_func)
         << AL << FD->getSourceRange();
     return;
+  } else if (FD->isConsteval()) {
+    S.Diag(AL.getLoc(), diag::err_ctordtor_attr_consteval)
+        << AL << FD->getSourceRange();
+    return;
   }
-
-  D->addAttr(::new (S.Context) CtorDtorAttr(S.Context, AL, Priority));
+  
+  D->addAttr(CtorDtorAttr::Create(S.Context, Priority, AL));
 }
 
 template <typename AttrTy>
diff --git a/clang/test/Sema/constructor-attribute.c b/clang/test/Sema/constructor-attribute.c
index 8df5ac42378bdd8..39ff714218f3493 100644
--- a/clang/test/Sema/constructor-attribute.c
+++ b/clang/test/Sema/constructor-attribute.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wno-strict-prototypes %s
-// RUN: %clang_cc1 -fsyntax-only -verify -x c++ %s
+// RUN: %clang_cc1 -fsyntax-only -verify -x c++ -std=c++20 %s
 
 int x1 __attribute__((constructor)); // expected-warning {{'constructor' attribute only applies to functions}}
 void f(void) __attribute__((constructor));
@@ -43,6 +43,9 @@ struct S {
   static void nonmem5(int) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
   static void nonmem6(int) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
 };
+
+consteval void consteval_func1() __attribute__((constructor)); // expected-error {{'constructor' attribute cannot be applied to a 'consteval' function}}
+consteval void consteval_func2() __attribute__((destructor));  // expected-error {{'destructor' attribute cannot be applied to a 'consteval' function}}
 #endif // __cplusplus
 
 # 1 "source.c" 1 3

>From d3666f20483da0593535396c57088791fd7428fc Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 26 Sep 2023 10:13:31 -0400
Subject: [PATCH 3/4] Fix formatting; NFC

---
 clang/lib/Sema/SemaDeclAttr.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 9a5de7eda641473..f80e0d39ef658c6 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2412,7 +2412,7 @@ static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
         << AL << FD->getSourceRange();
     return;
   }
-  
+
   D->addAttr(CtorDtorAttr::Create(S.Context, Priority, AL));
 }
 

>From 327bb8555b298b9b791a00b71b08ff5dd14b726f Mon Sep 17 00:00:00 2001
From: Aaron Ballman <aaron at aaronballman.com>
Date: Tue, 26 Sep 2023 11:06:43 -0400
Subject: [PATCH 4/4] Update based on review feedback

Now accepts 'int' and 'unsigned int' as return types, in addition to
'void'. Updated the diagnostic, test cases, and documentation
accordingly.
---
 clang/include/clang/Basic/AttrDocs.td         |  6 +++--
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 +-
 clang/lib/Sema/SemaDeclAttr.cpp               | 22 +++++++++++-----
 clang/test/Sema/constructor-attribute.c       | 26 +++++++++++--------
 4 files changed, 36 insertions(+), 20 deletions(-)

diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 2f9d4d1b7907b20..7720580b2c34912 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -7251,8 +7251,10 @@ after returning from ``main()`` or when the ``exit()`` function has been
 called. Note, ``quick_exit()``, ``_Exit()``, and ``abort()`` prevent a function
 marked ``destructor`` from being called.
 
-The constructor or destructor function should not accept any arguments and its
-return type should be ``void``.
+The constructor or destructor function cannot accept any arguments and its
+return type should be ``void``, ``int``, or ``unsigned int``. The latter two
+types are supported for historical reasons. In C++ language modes, the function
+cannot be marked ``consteval``, nor can it be a non-static member function.
 
 The attributes accept an optional argument used to specify the priority order
 in which to execute constructor and destructor functions. The priority is
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5cd05c7567d0158..3f4624d1d09db6f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3180,7 +3180,7 @@ def warn_aligned_attr_underaligned : Warning<err_alignas_underaligned.Summary>,
   InGroup<IgnoredAttributes>;
 def err_ctor_dtor_attr_on_non_void_func : Error<
   "%0 attribute can only be applied to a function which accepts no arguments "
-  "and has a 'void' return type">;
+  "and has a 'void' or 'int' return type">;
 def err_ctor_dtor_member_func : Error<
   "%0 attribute cannot be applied to a member function">;
 def err_attribute_sizeless_type : Error<
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index f80e0d39ef658c6..a0000a2118e812a 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -2391,13 +2391,23 @@ static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   }
 
   // Ensure the function we're attaching to is something that is sensible to
-  // automatically call before or after main(); it should accept no arguments
-  // and return no value; we treat it as an error because it's a form of type
-  // system incompatibility. It also cannot be a member function. We allow K&R
-  // C functions because that's a difficult edge case where it depends on how
-  // the function is defined as to whether it does or does not expect arguments.
+  // automatically call before or after main(); it should accept no arguments.
+  // In theory, a void return type is the only truly safe return type (consider
+  // that calling conventions may place returned values in a hidden pointer
+  // argument passed to the function that will not be present when called
+  // automatically). However, there is a significant amount of existing code
+  // which uses an int return type. So we will accept void, int, and
+  // unsigned int return types. Any other return type, or a non-void parameter
+  // list is treated as an error because it's a form of type system
+  // incompatibility. The function also cannot be a member function. We allow
+  // K&R C functions because that's a difficult edge case where it depends on
+  // how the function is defined as to whether it does or does not expect
+  // arguments.
   const auto *FD = cast<FunctionDecl>(D);
-  if (!FD->getReturnType()->isVoidType() ||
+  QualType RetTy = FD->getReturnType();
+  if (!(RetTy->isVoidType() ||
+        RetTy->isSpecificBuiltinType(BuiltinType::UInt) ||
+        RetTy->isSpecificBuiltinType(BuiltinType::Int)) ||
       (FD->hasPrototype() && FD->getNumParams() != 0)) {
     S.Diag(AL.getLoc(), diag::err_ctor_dtor_attr_on_non_void_func)
         << AL << FD->getSourceRange();
diff --git a/clang/test/Sema/constructor-attribute.c b/clang/test/Sema/constructor-attribute.c
index 39ff714218f3493..4d91f84af103af2 100644
--- a/clang/test/Sema/constructor-attribute.c
+++ b/clang/test/Sema/constructor-attribute.c
@@ -19,13 +19,17 @@ void f(void) __attribute__((destructor(101)));
 void knr1() __attribute__((constructor));
 void knr2() __attribute__((destructor));
 
-// Require a void return type
-int g(void) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
-int h(void) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+// Require a void or (unsigned) int return type
+int g0(void) __attribute__((constructor));
+signed int g1(void) __attribute__((constructor));
+float g2(void) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
+int h0(void) __attribute__((destructor));
+unsigned int h1(void) __attribute__((destructor));
+float h2(void) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
 
 // Require no parameters
-void i(int v) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
-void j(int v) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+void i(int v) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
+void j(int v) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
 
 #ifdef __cplusplus
 struct S {
@@ -34,14 +38,14 @@ struct S {
   void mem1() __attribute__((constructor)); // expected-error {{'constructor' attribute cannot be applied to a member function}}
   void mem2() __attribute__((destructor));  // expected-error {{'destructor' attribute cannot be applied to a member function}}
 
-  static void nonmem1() __attribute__((constructor));
-  static void nonmem2() __attribute__((destructor));
+  static signed nonmem1() __attribute__((constructor));
+  static unsigned nonmem2() __attribute__((destructor));
 
-  static int nonmem3() __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
-  static int nonmem4() __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+  static _BitInt(32) nonmem3() __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
+  static char nonmem4() __attribute__((destructor));         // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
 
-  static void nonmem5(int) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
-  static void nonmem6(int) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' return type}}
+  static void nonmem5(int) __attribute__((constructor)); // expected-error {{'constructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
+  static void nonmem6(int) __attribute__((destructor));  // expected-error {{'destructor' attribute can only be applied to a function which accepts no arguments and has a 'void' or 'int' return type}}
 };
 
 consteval void consteval_func1() __attribute__((constructor)); // expected-error {{'constructor' attribute cannot be applied to a 'consteval' function}}



More information about the cfe-commits mailing list