[Clang] Use valid source loc for empty target_clones diagnostic (#173747)

For X86 and RISCV checking of target_clones attribute arguments
attempted to use the location of the first argument for diagnosing a
missing default argument.

However, if the argument list is empty, then this location doesn't exist
and causes an assertion.

This commit passes the location of the attribute itself to the
target-specific validation function in the case of X86 and RISCV in
order to provide a usable location for this diagnostic.

Fixes #173684

---

I am not sure whether this is intentional, but for AArch64 the
validation does not emit a diagnostic for missing `"default"` argument.
Therefore the issue did not appear there and I did not make any changes
to it. This is the only other target besides X86 and RISCV that supports
`target_clones`.

clang/docs/ReleaseNotes.rst

@@ -570,6 +570,7 @@ Bug Fixes to Attribute Support
 - Fix ``cleanup`` attribute by delaying type checks until after the type is deduced. (#GH129631)
 - Fix a crash when instantiating a function template with ``constructor`` or ``destructor``
   attributes without a priority argument. (#GH169072)
+- Fix an assertion when using ``target_clones`` attribute with empty argument list. (#GH173684)
 
 Bug Fixes to C++ Support
 ^^^^^^^^^^^^^^^^^^^^^^^^

clang/include/clang/Sema/SemaRISCV.h

@@ -58,9 +58,10 @@ public:
 
   bool checkTargetVersionAttr(const StringRef Param, const SourceLocation Loc,
                               SmallString<64> &NewParam);
-  bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params,
-                             SmallVectorImpl<SourceLocation> &Locs,
-                             SmallVectorImpl<SmallString<64>> &NewParams);
+  bool checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params,
+                             const SmallVectorImpl<SourceLocation> &Locs,
+                             SmallVectorImpl<SmallString<64>> &NewParams,
+                             SourceLocation AttrLoc);
 };
 
 std::unique_ptr<sema::RISCVIntrinsicManager>

clang/include/clang/Sema/SemaX86.h

@@ -38,9 +38,10 @@ public:
   void handleAnyInterruptAttr(Decl *D, const ParsedAttr &AL);
   void handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL);
 
-  bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params,
-                             SmallVectorImpl<SourceLocation> &Locs,
-                             SmallVectorImpl<SmallString<64>> &NewParams);
+  bool checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params,
+                             const SmallVectorImpl<SourceLocation> &Locs,
+                             SmallVectorImpl<SmallString<64>> &NewParams,
+                             SourceLocation AttrLoc);
 };
 } // namespace clang
 

clang/lib/Sema/SemaDeclAttr.cpp

@@ -3510,10 +3510,12 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
     if (S.ARM().checkTargetClonesAttr(Params, Locations, NewParams))
       return;
   } else if (S.Context.getTargetInfo().getTriple().isRISCV()) {
-    if (S.RISCV().checkTargetClonesAttr(Params, Locations, NewParams))
+    if (S.RISCV().checkTargetClonesAttr(Params, Locations, NewParams,
+                                        AL.getLoc()))
       return;
   } else if (S.Context.getTargetInfo().getTriple().isX86()) {
-    if (S.X86().checkTargetClonesAttr(Params, Locations, NewParams))
+    if (S.X86().checkTargetClonesAttr(Params, Locations, NewParams,
+                                      AL.getLoc()))
       return;
   }
   Params.clear();

clang/lib/Sema/SemaRISCV.cpp

@@ -1801,8 +1801,9 @@ bool SemaRISCV::checkTargetVersionAttr(const StringRef Param,
 }
 
 bool SemaRISCV::checkTargetClonesAttr(
-    SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs,
-    SmallVectorImpl<SmallString<64>> &NewParams) {
+    const SmallVectorImpl<StringRef> &Params,
+    const SmallVectorImpl<SourceLocation> &Locs,
+    SmallVectorImpl<SmallString<64>> &NewParams, SourceLocation AttrLoc) {
   using namespace DiagAttrParams;
 
   assert(Params.size() == Locs.size() &&
@@ -1855,7 +1856,7 @@ bool SemaRISCV::checkTargetClonesAttr(
     NewParams.push_back(Param);
   }
   if (!HasDefault)
-    return Diag(Locs[0], diag::err_target_clone_must_have_default);
+    return Diag(AttrLoc, diag::err_target_clone_must_have_default);
 
   return false;
 }

clang/lib/Sema/SemaX86.cpp

@@ -1053,9 +1053,10 @@ void SemaX86::handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL) {
                  X86ForceAlignArgPointerAttr(getASTContext(), AL));
 }
 
-bool SemaX86::checkTargetClonesAttr(
-    SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs,
-    SmallVectorImpl<SmallString<64>> &NewParams) {
+bool SemaX86::checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params,
+                                    const SmallVectorImpl<SourceLocation> &Locs,
+                                    SmallVectorImpl<SmallString<64>> &NewParams,
+                                    SourceLocation AttrLoc) {
   using namespace DiagAttrParams;
 
   assert(Params.size() == Locs.size() &&
@@ -1105,7 +1106,7 @@ bool SemaX86::checkTargetClonesAttr(
     Diag(Locs[0], diag::warn_target_clone_mixed_values);
 
   if (!HasDefault)
-    return Diag(Locs[0], diag::err_target_clone_must_have_default);
+    return Diag(AttrLoc, diag::err_target_clone_must_have_default);
 
   return false;
 }

clang/test/Sema/attr-target-clones.c

@@ -138,3 +138,11 @@ void bad_isa_level(int) __attribute__((target_clones("default", "arch=x86-64-v5"
 
 // expected-warning@+1 {{unsupported 'sha' in the 'target_clones' attribute string; 'target_clones' attribute ignored}}
 void bad_feature(void) __attribute__((target_clones("default", "sse4.2", "sha")));
+
+// expected-error@+1 {{'target_clones' multiversioning requires a default target}}
+void __attribute__((target_clones()))
+gh173684_empty_attribute_args(void);
+
+// expected-error@+1 {{'target_clones' multiversioning requires a default target}}
+void __attribute__((target_clones))
+gh173684_empty_attribute_args_2(void);

clang/test/SemaCXX/attr-target-clones-riscv.cpp

@@ -51,3 +51,10 @@ void lambda() {
   auto y = []() __attribute__((target_clones("arch=+v", "default"))){};
   y();
 }
+
+namespace GH173684 {
+  // expected-error@+1 {{'target_clones' multiversioning requires a default target}}
+  void __attribute__((target_clones())) withoutDefault() {}
+  // expected-error@+1 {{'target_clones' multiversioning requires a default target}}
+  void __attribute__((target_clones)) withoutDefault2() {}
+}