Revert "Diagnose problematic uses of constructor/destructor attribute (#67360)"

This reverts commit b443510. This caused too many disruptions in compiler-rt where reserved priorities were being used. Reverting to investigate appropriate solutions.

Author: AaronBallman

clang/docs/ReleaseNotes.rst

@@ -171,11 +171,6 @@ 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
 -----------------------------------

clang/include/clang/Basic/AttrDocs.td

@@ -7251,10 +7251,8 @@ 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 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 constructor or destructor function should not accept any arguments and its
+return type should be ``void``.
 
 The attributes accept an optional argument used to specify the priority order
 in which to execute constructor and destructor functions. The priority is

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -2864,8 +2864,6 @@ 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<
@@ -3178,11 +3176,6 @@ 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' 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<
  "%0 attribute cannot be applied to sizeless type %1">;
 def err_attribute_argument_n_type : Error<

clang/lib/Sema/SemaDeclAttr.cpp

@@ -2352,78 +2352,26 @@ static void handleUnusedAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
  D->addAttr(::new (S.Context) UnusedAttr(S.Context, 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. 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 << ReservedPriorityLower << ReservedPriorityUpper;
- A.setInvalid();
- return true;
- }
- return false;
-}
-
-template <typename CtorDtorAttr>
-static void handleCtorDtorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
- uint32_t Priority = CtorDtorAttr::DefaultPriority;
+static void handleConstructorAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
+ uint32_t priority = ConstructorAttr::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;
 
- // 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;
-
- // Ensure the priority is in a reasonable range.
- if (diagnoseInvalidPriority(S, Priority, AL,
- AL.getArgAsExpr(0)->getExprLoc()))
- return;
- }
+ D->addAttr(::new (S.Context) ConstructorAttr(S.Context, AL, priority));
+}
 
- // Ensure the function we're attaching to is something that is sensible to
- // 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);
- 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();
+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))
  return;
- } 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(CtorDtorAttr::Create(S.Context, Priority, AL));
+ D->addAttr(::new (S.Context) DestructorAttr(S.Context, AL, priority));
 }
 
 template <typename AttrTy>
@@ -3940,9 +3888,16 @@ static void handleInitPriorityAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
  return;
  }
 
- if (diagnoseInvalidPriority(S, prioritynum, AL, E->getExprLoc()))
+ // 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();
  return;
-
+ }
  D->addAttr(::new (S.Context) InitPriorityAttr(S.Context, AL, prioritynum));
 }
 
@@ -8975,13 +8930,13 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
  handlePassObjectSizeAttr(S, D, AL);
  break;
  case ParsedAttr::AT_Constructor:
- handleCtorDtorAttr<ConstructorAttr>(S, D, AL);
+  handleConstructorAttr(S, D, AL);
  break;
  case ParsedAttr::AT_Deprecated:
  handleDeprecatedAttr(S, D, AL);
  break;
  case ParsedAttr::AT_Destructor:
- handleCtorDtorAttr<DestructorAttr>(S, D, AL);
+  handleDestructorAttr(S, D, AL);
  break;
  case ParsedAttr::AT_EnableIf:
  handleEnableIfAttr(S, D, AL);

clang/test/CodeGen/PowerPC/aix-destructor-attribute.c

@@ -12,8 +12,9 @@
 // RUN: -fno-use-cxa-atexit -fregister-global-dtors-with-atexit < %s | \
 // RUN: FileCheck --check-prefix=REGISTER %s
 
-int bar(void) __attribute__((destructor(101)));
+int bar(void) __attribute__((destructor(100)));
 int bar2(void) __attribute__((destructor(65535)));
+int bar3(int) __attribute__((destructor(65535)));
 
 int bar(void) {
  return 1;
@@ -23,12 +24,16 @@ int bar2(void) {
  return 2;
 }
 
-// NO-REGISTER: @llvm.global_dtors = appending global [2 x { i32, ptr, ptr }] [{ i32, ptr, ptr } { i32 101, ptr @bar, ptr null }, { i32, ptr, ptr } { i32 65535, ptr @bar2, ptr null }]
+int bar3(int a) {
+ return a;
+}
+
+// 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 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 }]
+// 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_101() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_init_100() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER: %0 = call i32 @atexit(ptr @bar)
 // REGISTER: ret void
@@ -37,10 +42,11 @@ int bar2(void) {
 // REGISTER: define internal void @__GLOBAL_init_65535() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER: %0 = call i32 @atexit(ptr @bar2)
+// REGISTER: %1 = call i32 @atexit(ptr @bar3)
 // REGISTER: ret void
 // REGISTER: }
 
-// REGISTER: define internal void @__GLOBAL_cleanup_101() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_cleanup_100() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER: %0 = call i32 @unatexit(ptr @bar)
 // REGISTER: %needs_destruct = icmp eq i32 %0, 0
@@ -56,11 +62,20 @@ int bar2(void) {
 
 // REGISTER: define internal void @__GLOBAL_cleanup_65535() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
-// REGISTER: %0 = call i32 @unatexit(ptr @bar2)
+// REGISTER: %0 = call i32 @unatexit(ptr @bar3)
 // REGISTER: %needs_destruct = icmp eq i32 %0, 0
-// REGISTER: br i1 %needs_destruct, label %destruct.call, label %destruct.end
+// REGISTER: br i1 %needs_destruct, label %destruct.call, label %unatexit.call
 
 // REGISTER: destruct.call:
+// REGISTER: call void @bar3()
+// REGISTER: br label %unatexit.call
+
+// REGISTER: unatexit.call:
+// REGISTER: %1 = call i32 @unatexit(ptr @bar2)
+// REGISTER: %needs_destruct1 = icmp eq i32 %1, 0
+// REGISTER: br i1 %needs_destruct1, label %destruct.call2, label %destruct.end
+
+// REGISTER: destruct.call2:
 // REGISTER: call void @bar2()
 // REGISTER: br label %destruct.end
 

clang/test/CodeGenCXX/aix-destructor-attribute.cpp

@@ -17,8 +17,9 @@ struct test {
  ~test();
 } t;
 
-int bar() __attribute__((destructor(101)));
+int bar() __attribute__((destructor(100)));
 int bar2() __attribute__((destructor(65535)));
+int bar3(int) __attribute__((destructor(65535)));
 
 int bar() {
  return 1;
@@ -28,13 +29,17 @@ int bar2() {
  return 2;
 }
 
+int bar3(int a) {
+ return a;
+}
+
 // 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 [3 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 @_GLOBAL__D_a, 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 }]
 
-// 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: @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: define internal void @__GLOBAL_init_101() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_init_100() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER: %0 = call i32 @atexit(ptr @_Z3barv)
 // REGISTER: ret void
@@ -43,10 +48,11 @@ int bar2() {
 // 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: ret void
 // REGISTER: }
 
-// REGISTER: define internal void @__GLOBAL_cleanup_101() [[ATTR:#[0-9]+]] {
+// REGISTER: define internal void @__GLOBAL_cleanup_100() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
 // REGISTER: %0 = call i32 @unatexit(ptr @_Z3barv)
 // REGISTER: %needs_destruct = icmp eq i32 %0, 0
@@ -62,11 +68,20 @@ int bar2() {
 
 // REGISTER: define internal void @__GLOBAL_cleanup_65535() [[ATTR:#[0-9]+]] {
 // REGISTER: entry:
-// REGISTER: %0 = call i32 @unatexit(ptr @_Z4bar2v)
+// REGISTER: %0 = call i32 @unatexit(ptr @_Z4bar3i)
 // REGISTER: %needs_destruct = icmp eq i32 %0, 0
-// REGISTER: br i1 %needs_destruct, label %destruct.call, label %destruct.end
+// REGISTER: br i1 %needs_destruct, label %destruct.call, label %unatexit.call
 
 // REGISTER: destruct.call:
+// REGISTER: call void @_Z4bar3i()
+// REGISTER: br label %unatexit.call
+
+// REGISTER: unatexit.call:
+// REGISTER: %1 = call i32 @unatexit(ptr @_Z4bar2v)
+// REGISTER: %needs_destruct1 = icmp eq i32 %1, 0
+// REGISTER: br i1 %needs_destruct1, label %destruct.call2, label %destruct.end
+
+// REGISTER: destruct.call2:
 // REGISTER: call void @_Z4bar2v()
 // REGISTER: br label %destruct.end