[clang][Sema] Handle target_clones redeclarations that omit the attribute (#169259)

This patch adds a case to `CheckMultiVersionAdditionalDecl()` that
detects redeclarations of `target_clones` functions which omit the
attribute, and makes sure they are marked as redeclarations. It also
updates the comment at the call site of
`CheckMultiVersionAdditionalDecl()` to reflect this.

Previously, `target_clones` multiversioned functions that omitted the
attribute from subsequent declarations would cause Clang to hit an
`llvm_unreachable` and crash. In the following example, the second
declaration (the function definition) should inherit the `target_clones`
attribute from the first declaration (the forward declaration):

```
__attribute__((target_clones("arch=atom", "default")))
void foo(void);

void foo(void) { /* ... */ }
```

However, `CheckMultiVersionAdditionalDecl()` was not recognizing the
function definition as a redeclaration of the forward declaration, which
prevented `Sema::MergeFunctionDecl()` from automatically inheriting the
attribute.

A side effect of this fix is that Clang now catches redeclarations of
`target_clones` functions that have conflicting types, which previously
caused Clang to crash by hitting that same `llvm_unreachable`. The
`bad_overload1` case in `clang/test/Sema/attr-target-clones.c` has been
updated to reflect this.

Fixes #165517
Fixes #129483

Author: henrybw

clang/docs/ReleaseNotes.rst

@@ -491,6 +491,8 @@ Bug Fixes in This Version
 - Accept empty enumerations in MSVC-compatible C mode. (#GH114402)
 - Fix a bug leading to incorrect code generation with complex number compound assignment and bitfield values, which also caused a crash with UBsan. (#GH166798)
 - Fixed false-positive shadow diagnostics for lambdas in explicit object member functions. (#GH163731)
+- Fix an assertion failure when a ``target_clones`` attribute is only on the
+  forward declaration of a multiversioned function. (#GH165517) (#GH129483)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/Sema/SemaDecl.cpp

@@ -11996,6 +11996,16 @@ static bool CheckMultiVersionAdditionalDecl(
     }
   }
 
+  // Redeclarations of a target_clones function may omit the attribute, in which
+  // case it will be inherited during declaration merging.
+  if (NewMVKind == MultiVersionKind::None &&
+      OldMVKind == MultiVersionKind::TargetClones) {
+    NewFD->setIsMultiVersion();
+    Redeclaration = true;
+    OldDecl = OldFD;
+    return false;
+  }
+
   // Else, this is simply a non-redecl case.  Checking the 'value' is only
   // necessary in the Target case, since The CPUSpecific/Dispatch cases are
   // handled in the attribute adding step.
@@ -12119,8 +12129,9 @@ static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD,
   }
 
   // At this point, we have a multiversion function decl (in OldFD) AND an
-  // appropriate attribute in the current function decl.  Resolve that these are
-  // still compatible with previous declarations.
+  // appropriate attribute in the current function decl (unless it's allowed to
+  // omit the attribute).  Resolve that these are still compatible with previous
+  // declarations.
   return CheckMultiVersionAdditionalDecl(S, OldFD, NewFD, NewCPUDisp,
                                          NewCPUSpec, NewClones, Redeclaration,
                                          OldDecl, Previous);

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

@@ -125,6 +125,35 @@ void __attribute__((target_clones("default, arch=ivybridge"))) unused(void) {}
 // WINDOWS: musttail call void @unused.arch_ivybridge.0
 // WINDOWS: musttail call void @unused.default.1
 
+int __attribute__((target_clones("sse4.2, default"))) inherited(void);
+int inherited(void) { return 0; }
+// LINUX: define {{.*}}i32 @inherited.sse4.2.0()
+// LINUX: define {{.*}}i32 @inherited.default.1()
+// LINUX: define weak_odr ptr @inherited.resolver() #[[ATTR_RESOLVER]] comdat
+// LINUX: ret ptr @inherited.sse4.2.0
+// LINUX: ret ptr @inherited.default.1
+
+// DARWIN: define {{.*}}i32 @inherited.sse4.2.0()
+// DARWIN: define {{.*}}i32 @inherited.default.1()
+// DARWIN: define weak_odr ptr @inherited.resolver() #[[ATTR_RESOLVER]] {
+// DARWIN: ret ptr @inherited.sse4.2.0
+// DARWIN: ret ptr @inherited.default.1
+
+// WINDOWS: define dso_local i32 @inherited.sse4.2.0()
+// WINDOWS: define dso_local i32 @inherited.default.1()
+// WINDOWS: define weak_odr dso_local i32 @inherited() #[[ATTR_RESOLVER]] comdat
+// WINDOWS: musttail call i32 @inherited.sse4.2.0
+// WINDOWS: musttail call i32 @inherited.default.1
+
+int test_inherited(void) {
+  // LINUX: define {{.*}}i32 @test_inherited() #[[DEF:[0-9]+]]
+  // DARWIN: define {{.*}}i32 @test_inherited() #[[DEF:[0-9]+]]
+  // WINDOWS: define dso_local i32 @test_inherited() #[[DEF:[0-9]+]]
+  return inherited();
+  // LINUX: call i32 @inherited()
+  // DARWIN: call i32 @inherited()
+  // WINDOWS: call i32 @inherited()
+}
 
 inline int __attribute__((target_clones("arch=sandybridge,default,sse4.2")))
 foo_inline(void) { return 0; }

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

@@ -28,6 +28,17 @@ int __attribute__((target_clones("sse4.2", "arch=atom", "default"))) redecl4(voi
 int __attribute__((target_clones("sse4.2", "arch=sandybridge", "default")))
 redecl4(void) { return 1; }
 
+int __attribute__((target_clones("sse4.2", "default"))) redecl5(void);
+int redecl5(void) { return 1; }
+
+int redecl6(void);
+int __attribute__((target_clones("sse4.2", "default"))) redecl6(void) { return 1; }
+
+int __attribute__((target_clones("sse4.2", "default"))) redecl7(void);
+// expected-error@+2 {{multiversioning attributes cannot be combined}}
+// expected-note@-2 {{previous declaration is here}}
+int __attribute__((target("sse4.2"))) redecl7(void) { return 1; }
+
 int __attribute__((target("sse4.2"))) redef2(void) { return 1; }
 // expected-error@+2 {{multiversioning attributes cannot be combined}}
 // expected-note@-2 {{previous declaration is here}}
@@ -87,6 +98,8 @@ int useage(void) {
 int __attribute__((target_clones("sse4.2", "default"))) mv_after_use(void) { return 1; }
 
 void bad_overload1(void) __attribute__((target_clones("mmx", "sse4.2", "default")));
+// expected-error@+2 {{conflicting types for 'bad_overload1'}}
+// expected-note@-2 {{previous declaration is here}}
 void bad_overload1(int p) {}
 
 void bad_overload2(int p) {}