From 27309114bef0eefd7b947702c99aba18b6f14269 Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Wed, 14 Sep 2022 14:58:23 -0700 Subject: [PATCH] FHU: Convert to a interface target Noticed recently that `FEXServer -w` was broken and couldn't understand why. Turns out that FHU syscall handling was /always/ falling down the `#else` path in the handlers since cmake `add_definitions` follows folder scoping rules. This means it was always returning -1, which was causing FEXServer's pidfd_open usage to always receive -1, which meant the sendmsg with FD was always failing, which meant the `FEXServer -w` would forever wait for a message that was never sent. Converting the utility over to a target not only fixes definition scoping problems, but also makes the other paths actually work. This found some compiling bugs and instead lets us define SYS_pidfd_open if it doesn't exist. Letting the kernel return the ENOSYS if it doesn't exist on that platform. Main thing, fixes FEXServer -w hanging forever. --- CMakeLists.txt | 2 - External/FEXCore/Source/CMakeLists.txt | 8 +-- FEXHeaderUtils/CMakeLists.txt | 76 ++++++++++-------------- FEXHeaderUtils/FEXHeaderUtils/Syscalls.h | 17 +++--- Source/Common/CMakeLists.txt | 2 +- 5 files changed, 47 insertions(+), 58 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 110070cfd..8f9e8e6e6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -364,8 +364,6 @@ if (BUILD_TESTS) endif() add_subdirectory(FEXHeaderUtils/) -include_directories(FEXHeaderUtils/) - add_subdirectory(External/FEXCore) # Binfmt_misc files must be installed prior to Source/ installs diff --git a/External/FEXCore/Source/CMakeLists.txt b/External/FEXCore/Source/CMakeLists.txt index a3d49d01f..316489f8d 100644 --- a/External/FEXCore/Source/CMakeLists.txt +++ b/External/FEXCore/Source/CMakeLists.txt @@ -218,7 +218,7 @@ if (ENABLE_JIT_ARM64) ) endif() -set (LIBS vixl dl xxhash tiny-json) +set (LIBS fmt::fmt vixl dl xxhash tiny-json FEXHeaderUtils) if (ENABLE_JEMALLOC) list (APPEND LIBS FEX_jemalloc) endif() @@ -364,14 +364,14 @@ endfunction() # Build FEXCore_Config static library add_library(FEXCore_Base STATIC ${FEXCORE_BASE_SRCS}) -target_link_libraries(FEXCore_Base fmt::fmt tiny-json) +target_link_libraries(FEXCore_Base ${LIBS}) AddDefaultOptionsToTarget(FEXCore_Base) function(AddObject Name Type) add_library(${Name} ${Type} ${SRCS}) add_dependencies(${Name} IR_INC) - target_link_libraries(${Name} FEXCore_Base ${LIBS}) + target_link_libraries(${Name} FEXCore_Base) AddDefaultOptionsToTarget(${Name}) set_target_properties(${Name} PROPERTIES OUTPUT_NAME FEXCore) @@ -379,7 +379,7 @@ endfunction() function(AddLibrary Name Type) add_library(${Name} ${Type} $) - target_link_libraries(${Name} FEXCore_Base ${LIBS}) + target_link_libraries(${Name} FEXCore_Base) set_target_properties(${Name} PROPERTIES OUTPUT_NAME FEXCore) AddDefaultOptionsToTarget(${Name}) diff --git a/FEXHeaderUtils/CMakeLists.txt b/FEXHeaderUtils/CMakeLists.txt index 835266826..264229bce 100644 --- a/FEXHeaderUtils/CMakeLists.txt +++ b/FEXHeaderUtils/CMakeLists.txt @@ -1,74 +1,64 @@ +add_library(FEXHeaderUtils INTERFACE) + # Check for syscall support here check_cxx_source_compiles( - " - #include - int main() { - return ::getcpu(nullptr, nullptr); - }" + " + #include + int main() { + return ::getcpu(nullptr, nullptr); + }" compiles) if (compiles) message(STATUS "Has getcpu helper") - add_definitions(-DHAS_SYSCALL_GETCPU=1) + target_compile_definitions(FEXHeaderUtils INTERFACE HAS_SYSCALL_GETCPU=1) endif () check_cxx_source_compiles( - " - #include - int main() { - return ::gettid(); - }" + " + #include + int main() { + return ::gettid(); + }" compiles) if (compiles) message(STATUS "Has gettid helper") - add_definitions(-DHAS_SYSCALL_GETTID=1) + target_compile_definitions(FEXHeaderUtils INTERFACE HAS_SYSCALL_GETTID=1) endif () check_cxx_source_compiles( - " - #include - int main() { - return ::tgkill(0, 0, 0); - }" + " + #include + int main() { + return ::tgkill(0, 0, 0); + }" compiles) if (compiles) message(STATUS "Has tgkill helper") - add_definitions(-DHAS_SYSCALL_TGKILL=1) + target_compile_definitions(FEXHeaderUtils INTERFACE HAS_SYSCALL_TGKILL=1) endif () check_cxx_source_compiles( - " - #include - int main() { - return ::statx(0, nullptr, 0, 0, nullptr); - }" + " + #include + int main() { + return ::statx(0, nullptr, 0, 0, nullptr); + }" compiles) if (compiles) message(STATUS "Has statx helper") - add_definitions(-DHAS_SYSCALL_STATX=1) + target_compile_definitions(FEXHeaderUtils INTERFACE HAS_SYSCALL_STATX=1) endif () check_cxx_source_compiles( - " - #include - int main() { - return ::renameat2(0, nullptr, 0, nullptr, 0); - }" + " + #include + int main() { + return ::renameat2(0, nullptr, 0, nullptr, 0); + }" compiles) if (compiles) message(STATUS "Has renameat2 helper") - add_definitions(-DHAS_SYSCALL_RENAMEAT2=1) -endif () - -check_cxx_source_compiles( - " - #include - #include - int main() { - return ::syscall(SYS_pidfd_open, ::getpid(), 0); - }" - compiles) -if (compiles) - message(STATUS "Has pidfd_open helper") - add_definitions(-DHAS_SYSCALL_PIDFD_OPEN=1) + target_compile_definitions(FEXHeaderUtils INTERFACE HAS_SYSCALL_RENAMEAT2=1) endif () +target_include_directories(FEXHeaderUtils INTERFACE .) diff --git a/FEXHeaderUtils/FEXHeaderUtils/Syscalls.h b/FEXHeaderUtils/FEXHeaderUtils/Syscalls.h index 997418204..2918a097d 100644 --- a/FEXHeaderUtils/FEXHeaderUtils/Syscalls.h +++ b/FEXHeaderUtils/FEXHeaderUtils/Syscalls.h @@ -38,10 +38,15 @@ namespace FHU::Syscalls { #endif #endif +// Common syscall numbers +#ifndef SYS_pidfd_open +#define SYS_pidfd_open 434 +#endif + inline int32_t getcpu(uint32_t *cpu, uint32_t *node) { // Third argument is unused #if defined(HAS_SYSCALL_GETCPU) && HAS_SYSCALL_GETCPU - return ::getcpu(cpu, node, nullptr); + return ::getcpu(cpu, node); #else return ::syscall(SYS_getcpu, cpu, node, nullptr); #endif @@ -57,7 +62,7 @@ inline int32_t gettid() { inline int32_t tgkill(pid_t tgid, pid_t tid, int sig) { #if defined(HAS_SYSCALL_GETTID) && HAS_SYSCALL_GETTID - return ::tgkill(tggid, tid, sig); + return ::tgkill(tgid, tid, sig); #else return ::syscall(SYS_tgkill, tgid, tid, sig); #endif @@ -65,7 +70,7 @@ inline int32_t tgkill(pid_t tgid, pid_t tid, int sig) { inline int32_t statx(int dirfd, const char *pathname, int32_t flags, uint32_t mask, void *statxbuf) { #if defined(HAS_SYSCALL_STATX) && HAS_SYSCALL_STATX - return ::statx(dirfd, pathname, flags, mask, statxbuf); + return ::statx(dirfd, pathname, flags, mask, reinterpret_cast(statxbuf)); #else return ::syscall(SYS_statx, dirfd, pathname, flags, mask, statxbuf); #endif @@ -80,11 +85,7 @@ inline int32_t renameat2(int olddirfd, const char *oldpath, int newdirfd, const } inline int32_t pidfd_open(pid_t pid, unsigned int flags) { -#if defined(DHAS_SYSCALL_PIDFD_OPEN) && DHAS_SYSCALL_PIDFD_OPEN - return ::syscall(SYS_pidfd_open, pid_t pid, unsigned int flags); -#else - return -1; -#endif + return ::syscall(SYS_pidfd_open, pid, flags); } } diff --git a/Source/Common/CMakeLists.txt b/Source/Common/CMakeLists.txt index 01e9598ba..adae65422 100644 --- a/Source/Common/CMakeLists.txt +++ b/Source/Common/CMakeLists.txt @@ -8,6 +8,6 @@ set(SRCS StringUtil.cpp) add_library(${NAME} STATIC ${SRCS}) -target_link_libraries(${NAME} FEXCore_Base cpp-optparse json-maker) +target_link_libraries(${NAME} FEXCore_Base cpp-optparse json-maker FEXHeaderUtils) target_include_directories(${NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/External/cpp-optparse/) target_include_directories(${NAME} PRIVATE ${CMAKE_BINARY_DIR}/generated)