ilinpv created this revision. ilinpv added reviewers: srhines, danielkiss, enh, MaskRay, rprichard. Herald added subscribers: Enna1, kristof.beyls, krytarowski. Herald added a project: All. ilinpv requested review of this revision. Herald added projects: clang, Sanitizers. Herald added subscribers: Sanitizers, cfe-commits.
The patch tries to fix Function Multi Versioning features detection in ifunc resolver on Android API levels < 30. Ifunc hwcaps parameters are not supported <https://android.googlesource.com/platform/bionic/+/master/libc/include/sys/ifunc.h#69> on Android API levels 23-29, so all CPU features are set unsupported if they were not initialized before ifunc resolver call. There is no support for ifunc on Android API levels < 23, so Function Multi Versioning is disabled in this case. However, applying this patch to android-ndk-r26-beta2 and build using it a simple app with FMV and feature detection lead to crash when FMV ifunc resolver called. I suspect issue could be in Android API runtime check called from ifunc resolver: ifunc_resolver -> init_cpu_features_resolver -> android_get_device_api_level(__system_property_get). Part of crash stack trace ( thanks to Lingkai.Dong ) : 08-23 15:37:16.824 4641 4641 F DEBUG : backtrace: 08-23 15:37:16.824 4641 4641 F DEBUG : #00 pc 0000000000001af0 <unknown> 08-23 15:37:16.824 4641 4641 F DEBUG : #01 pc 00000000000015b8 /data/app/~~dMaXGzJksZog7lGF_CEMHQ==/com.arm.v9.demo-C74mlVTkH7NumAtbKGo36A==/base.apk!libdemo.so (offset 0x3e9000) (BuildId: 91106fcefd2e1eb3f7567f0c9b297d48ebfdf452) 08-23 15:37:16.824 4641 4641 F DEBUG : #02 pc 00000000000010e8 /data/app/~~dMaXGzJksZog7lGF_CEMHQ==/com.arm.v9.demo-C74mlVTkH7NumAtbKGo36A==/base.apk!libdemo.so (offset 0x3e9000) (BuildId: 91106fcefd2e1eb3f7567f0c9b297d48ebfdf452) 08-23 15:37:16.824 4641 4641 F DEBUG : #03 pc 000000000004c2f0 /apex/com.android.runtime/bin/linker64 (__dl__Z19call_ifunc_resolvery+36) (BuildId: 87cff915f050a1eab12b7d6dd7c25a63) 08-23 15:37:16.824 4641 4641 F DEBUG : #04 pc 000000000005dbd8 /apex/com.android.runtime/bin/linker64 (__dl__ZL26process_relocation_generalR9RelocatorRK10elf64_rela.__uniq.153370809355997480299804515629147722701+1500) (BuildId: 87cff915f050a1eab12b7d6dd7c25a63) I would appreciate any ideas what could be wrong in the patch, further confirmation if android_get_device_api_level should work from ifunc_resolver, or tips for ifunc_resolver debugging on Android :) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D158641 Files: clang/lib/Driver/ToolChains/Clang.cpp clang/test/Driver/aarch64-features.c compiler-rt/lib/builtins/cpu_model.c Index: compiler-rt/lib/builtins/cpu_model.c =================================================================== --- compiler-rt/lib/builtins/cpu_model.c +++ compiler-rt/lib/builtins/cpu_model.c @@ -894,6 +894,7 @@ #include <asm/hwcap.h> #if defined(__ANDROID__) +#include <android/api-level.h> #include <string.h> #include <sys/system_properties.h> #elif defined(__Fuchsia__) @@ -1186,7 +1187,8 @@ // As features grows new fields could be added } __aarch64_cpu_features __attribute__((visibility("hidden"), nocommon)); -void init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t *arg) { +static void init_cpu_features_constructor(unsigned long hwcap, + const __ifunc_arg_t *arg) { #define setCPUFeature(F) __aarch64_cpu_features.features |= 1ULL << F #define getCPUFeature(id, ftr) __asm__("mrs %0, " #id : "=r"(ftr)) #define extractBits(val, start, number) \ @@ -1374,6 +1376,21 @@ setCPUFeature(FEAT_MAX); } +void init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t *arg) { + if (__aarch64_cpu_features.features) + return; +#if defined(__ANDROID__) + // ifunc resolvers don't have hwcaps in arguments on Android API lower + // than 30. In this case set detection done and keep all CPU features + // unsupported (zeros). + if (android_get_device_api_level() < 30) { + setCPUFeature(FEAT_MAX); + return; + } +#endif // defined(__ANDROID__) + init_cpu_features_constructor(hwcap, arg); +} + void CONSTRUCTOR_ATTRIBUTE init_cpu_features(void) { unsigned long hwcap; unsigned long hwcap2; @@ -1399,7 +1416,7 @@ arg._size = sizeof(__ifunc_arg_t); arg._hwcap = hwcap; arg._hwcap2 = hwcap2; - init_cpu_features_resolver(hwcap | _IFUNC_ARG_HWCAP, &arg); + init_cpu_features_constructor(hwcap | _IFUNC_ARG_HWCAP, &arg); #undef extractBits #undef getCPUFeature #undef setCPUFeature Index: clang/test/Driver/aarch64-features.c =================================================================== --- clang/test/Driver/aarch64-features.c +++ clang/test/Driver/aarch64-features.c @@ -7,12 +7,18 @@ // CHECK: fno-signed-char // Check Function Multi Versioning option and rtlib dependency. -// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \ +// RUN: %clang --target=aarch64-linux-android23 -rtlib=compiler-rt \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV %s +// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \ +// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s + // RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt -mno-fmv \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s +// RUN: %clang --target=aarch64-linux-android22 -rtlib=compiler-rt \ +// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s + // RUN: %clang --target=aarch64-linux-gnu -rtlib=libgcc \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s Index: clang/lib/Driver/ToolChains/Clang.cpp =================================================================== --- clang/lib/Driver/ToolChains/Clang.cpp +++ clang/lib/Driver/ToolChains/Clang.cpp @@ -7497,6 +7497,7 @@ if (Triple.isAArch64() && (Args.hasArg(options::OPT_mno_fmv) || + (Triple.isAndroid() && Triple.isAndroidVersionLT(23)) || getToolChain().GetRuntimeLibType(Args) != ToolChain::RLT_CompilerRT)) { // Disable Function Multiversioning on AArch64 target. CmdArgs.push_back("-target-feature");
Index: compiler-rt/lib/builtins/cpu_model.c =================================================================== --- compiler-rt/lib/builtins/cpu_model.c +++ compiler-rt/lib/builtins/cpu_model.c @@ -894,6 +894,7 @@ #include <asm/hwcap.h> #if defined(__ANDROID__) +#include <android/api-level.h> #include <string.h> #include <sys/system_properties.h> #elif defined(__Fuchsia__) @@ -1186,7 +1187,8 @@ // As features grows new fields could be added } __aarch64_cpu_features __attribute__((visibility("hidden"), nocommon)); -void init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t *arg) { +static void init_cpu_features_constructor(unsigned long hwcap, + const __ifunc_arg_t *arg) { #define setCPUFeature(F) __aarch64_cpu_features.features |= 1ULL << F #define getCPUFeature(id, ftr) __asm__("mrs %0, " #id : "=r"(ftr)) #define extractBits(val, start, number) \ @@ -1374,6 +1376,21 @@ setCPUFeature(FEAT_MAX); } +void init_cpu_features_resolver(unsigned long hwcap, const __ifunc_arg_t *arg) { + if (__aarch64_cpu_features.features) + return; +#if defined(__ANDROID__) + // ifunc resolvers don't have hwcaps in arguments on Android API lower + // than 30. In this case set detection done and keep all CPU features + // unsupported (zeros). + if (android_get_device_api_level() < 30) { + setCPUFeature(FEAT_MAX); + return; + } +#endif // defined(__ANDROID__) + init_cpu_features_constructor(hwcap, arg); +} + void CONSTRUCTOR_ATTRIBUTE init_cpu_features(void) { unsigned long hwcap; unsigned long hwcap2; @@ -1399,7 +1416,7 @@ arg._size = sizeof(__ifunc_arg_t); arg._hwcap = hwcap; arg._hwcap2 = hwcap2; - init_cpu_features_resolver(hwcap | _IFUNC_ARG_HWCAP, &arg); + init_cpu_features_constructor(hwcap | _IFUNC_ARG_HWCAP, &arg); #undef extractBits #undef getCPUFeature #undef setCPUFeature Index: clang/test/Driver/aarch64-features.c =================================================================== --- clang/test/Driver/aarch64-features.c +++ clang/test/Driver/aarch64-features.c @@ -7,12 +7,18 @@ // CHECK: fno-signed-char // Check Function Multi Versioning option and rtlib dependency. -// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \ +// RUN: %clang --target=aarch64-linux-android23 -rtlib=compiler-rt \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV %s +// RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt \ +// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s + // RUN: %clang --target=aarch64-linux-android -rtlib=compiler-rt -mno-fmv \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s +// RUN: %clang --target=aarch64-linux-android22 -rtlib=compiler-rt \ +// RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s + // RUN: %clang --target=aarch64-linux-gnu -rtlib=libgcc \ // RUN: -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-FMV-OFF %s Index: clang/lib/Driver/ToolChains/Clang.cpp =================================================================== --- clang/lib/Driver/ToolChains/Clang.cpp +++ clang/lib/Driver/ToolChains/Clang.cpp @@ -7497,6 +7497,7 @@ if (Triple.isAArch64() && (Args.hasArg(options::OPT_mno_fmv) || + (Triple.isAndroid() && Triple.isAndroidVersionLT(23)) || getToolChain().GetRuntimeLibType(Args) != ToolChain::RLT_CompilerRT)) { // Disable Function Multiversioning on AArch64 target. CmdArgs.push_back("-target-feature");
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits