[clang] [CodeGen] Set attributes on resolvers emitted after ifuncs (PR #98832)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang

Author: Fangrui Song (MaskRay)

<details>
<summary>Changes</summary>

When an ifunc is emitted before its resolver, the resolver is created by
`GetOrCreateLLVMFunction` without ever calling `SetFunctionAttributes`.
The causes missing `!kcfi_type` with -fsanitize=kcfi.

```
extern void ifunc0(void) __attribute__ ((ifunc("resolver0")));
void *resolver0(void) { return 0; } // SetFunctionAttributes not called

extern void ifunc1(void) __attribute__ ((ifunc("resolver1")));
static void *resolver1(void) { return 0; } // SetFunctionAttributes not called

extern void ifunc2(void) __attribute__ ((ifunc("resolver2")));
static void *resolver2(void*) { return 0; }
```

This is because `GetOrCreateLLVMFunction`, while might also be called
with `ForDefinition`, does not continue after `(Entry->getValueType() == Ty)`.

To ensure that `SetFunctionAttributes` is called, call
`GetOrCreateLLVMFunction` with a dummy non-function type. Now the
`F->takeName(Entry)` code path may be taken, the
`DisableSanitizerInstrumentation` code
(https://reviews.llvm.org/D150262) should be moved to `checkAliases`,
when the resolver function is finalized.


---
Full diff: https://github.com/llvm/llvm-project/pull/98832.diff


3 Files Affected:

- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+12-7) 
- (modified) clang/test/CodeGen/ifunc.c (+4-5) 
- (modified) clang/test/CodeGen/kcfi.c (+10-10) 


``````````diff
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 08cfa694cfb81..250041d380cb1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -721,6 +721,11 @@ void CodeGenModule::checkAliases() {
           cast<llvm::GlobalAlias>(Alias)->setAliasee(Aliasee);
       }
     }
+    // ifunc resolvers are usually implemented to run before sanitizer
+    // initialization. Disable instrumentation to prevent the ordering issue.
+    if (IsIFunc)
+      cast<llvm::Function>(Aliasee)->addFnAttr(
+          llvm::Attribute::DisableSanitizerInstrumentation);
   }
   if (!Error)
     return;
@@ -6106,11 +6111,14 @@ void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
 
   Aliases.push_back(GD);
 
+  // The resolver might not be visited yet. Specify a dummy non-function type to
+  // indicate IsIncompleteFunction. Either the type is ignored (if the resolver
+  // was emitted or will be eagerly emitted) or the whole function will replaced
+  // (if the resolver will be inserted into DeferredDeclsToEmit).
+  llvm::Constant *Resolver = GetOrCreateLLVMFunction(
+      IFA->getResolver(), llvm::Type::getVoidTy(getLLVMContext()), {},
+      /*ForVTable=*/false);
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
-  llvm::Type *ResolverTy = llvm::GlobalIFunc::getResolverFunctionType(DeclTy);
-  llvm::Constant *Resolver =
-      GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
-                              /*ForVTable=*/false);
   llvm::GlobalIFunc *GIF =
       llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
                                 "", Resolver, &getModule());
@@ -6134,9 +6142,6 @@ void CodeGenModule::emitIFuncDefinition(GlobalDecl GD) {
     Entry->eraseFromParent();
   } else
     GIF->setName(MangledName);
-  if (auto *F = dyn_cast<llvm::Function>(Resolver)) {
-    F->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
-  }
   SetCommonAttributes(GD, GIF);
 }
 
diff --git a/clang/test/CodeGen/ifunc.c b/clang/test/CodeGen/ifunc.c
index b049739daf2aa..58a00ada687cb 100644
--- a/clang/test/CodeGen/ifunc.c
+++ b/clang/test/CodeGen/ifunc.c
@@ -58,12 +58,11 @@ extern void hoo(int) __attribute__ ((ifunc("hoo_ifunc")));
 // CHECK: call i32 @foo(i32
 // CHECK: call void @goo()
 
-// SAN: define internal nonnull {{(noundef )?}}ptr @foo_ifunc() #[[#FOO_IFUNC:]] {
-
 // SAN: define {{(dso_local )?}}noalias {{(noundef )?}}ptr @goo_ifunc() #[[#GOO_IFUNC:]] {
 
-// SAN: define {{(dso_local )?}}noalias {{(noundef )?}}ptr @hoo_ifunc() #[[#HOO_IFUNC:]] {
+// SAN: define {{(dso_local )?}}noalias {{(noundef )?}}ptr @hoo_ifunc() #[[#GOO_IFUNC]] {
+
+// SAN: define internal {{(noundef )?}}nonnull ptr @foo_ifunc() #[[#FOO_IFUNC:]] {
 
-// SAN-DAG: attributes #[[#FOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
 // SAN-DAG: attributes #[[#GOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
-// SAN-DAG: attributes #[[#HOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
+// SAN-DAG: attributes #[[#FOO_IFUNC]] = {{{.*}} disable_sanitizer_instrumentation {{.*}}
diff --git a/clang/test/CodeGen/kcfi.c b/clang/test/CodeGen/kcfi.c
index c29429f644ba1..622843cedba50 100644
--- a/clang/test/CodeGen/kcfi.c
+++ b/clang/test/CodeGen/kcfi.c
@@ -33,16 +33,6 @@ int call(fn_t f) {
   return f();
 }
 
-#ifndef __cplusplus
-// C: define internal ptr @resolver1() #[[#]] {
-int ifunc1(int) __attribute__((ifunc("resolver1")));
-static void *resolver1(void) { return 0; }
-
-// C: define internal ptr @resolver2() #[[#]] {
-static void *resolver2(void) { return 0; }
-long ifunc2(long) __attribute__((ifunc("resolver2")));
-#endif
-
 // CHECK-DAG: define internal{{.*}} i32 @{{f3|_ZL2f3v}}(){{.*}} !kcfi_type ![[#TYPE]]
 static int f3(void) { return 1; }
 
@@ -58,6 +48,16 @@ static int f5(void) { return 2; }
 // CHECK-DAG: declare !kcfi_type ![[#TYPE]]{{.*}} i32 @{{f6|_Z2f6v}}()
 extern int f6(void);
 
+#ifndef __cplusplus
+// C: define internal ptr @resolver1() #[[#]] !kcfi_type ![[#]] {
+int ifunc1(int) __attribute__((ifunc("resolver1")));
+static void *resolver1(void) { return 0; }
+
+// C: define internal ptr @resolver2() #[[#]] !kcfi_type ![[#]] {
+static void *resolver2(void) { return 0; }
+long ifunc2(long) __attribute__((ifunc("resolver2")));
+#endif
+
 int test(void) {
   return call(f1) +
          __call((fn_t)f2) +

``````````

</details>


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