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

[Aaron Ballman via cfe-commits]

================
@@ -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));
----------------
AaronBallman wrote:

Sure, I can switch to that!

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