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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits