diff --git a/ThunkLibs/Generator/data_layout.cpp b/ThunkLibs/Generator/data_layout.cpp index 65a09cad88..1ed3496b43 100644 --- a/ThunkLibs/Generator/data_layout.cpp +++ b/ThunkLibs/Generator/data_layout.cpp @@ -85,6 +85,7 @@ std::unordered_map ComputeDataLayout(const clang:: .type_name = get_type_name(context, field_type), .member_name = field->getNameAsString(), .array_size = array_size, + .is_function_pointer = field_type->isFunctionPointerType(), }; // TODO: Process types in dependency-order. Currently we skip this diff --git a/ThunkLibs/Generator/data_layout.h b/ThunkLibs/Generator/data_layout.h index 0c13e9980b..1bcf76d5ec 100644 --- a/ThunkLibs/Generator/data_layout.h +++ b/ThunkLibs/Generator/data_layout.h @@ -28,13 +28,15 @@ struct StructInfo : SimpleTypeInfo { std::string type_name; std::string member_name; std::optional array_size; + bool is_function_pointer; bool operator==(const MemberInfo& other) const { return size_bits == other.size_bits && offset_bits == other.offset_bits && type_name == other.type_name && member_name == other.member_name && - array_size == other.array_size; + array_size == other.array_size && + is_function_pointer == other.is_function_pointer; } }; diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp index eac5109bfa..48a823c5ab 100644 --- a/ThunkLibs/Generator/gen.cpp +++ b/ThunkLibs/Generator/gen.cpp @@ -216,7 +216,12 @@ void GenerateThunkLibsAction::EmitLayoutWrappers( auto type_name = member->getType().getAsString(); auto array_type = llvm::dyn_cast(member->getType()); if (!array_type && skip_arrays) { - fmt::print(file, " .{} = host_layout<{}> {{ from.data.{} }}.data,\n", decl_name, type_name, decl_name); + if (member->getType()->isFunctionPointerType()) { + // Function pointers must be handled manually, so zero them out by default + fmt::print(file, " .{} {{ }},\n", decl_name); + } else { + fmt::print(file, " .{} = host_layout<{}> {{ from.data.{} }}.data,\n", decl_name, type_name, decl_name); + } } else if (array_type && !skip_arrays) { // Copy element-wise below fmt::print(file, " for (size_t i = 0; i < {}; ++i) {{\n", array_type->getSize().getZExtValue()); @@ -250,7 +255,12 @@ void GenerateThunkLibsAction::EmitLayoutWrappers( auto& decl_name = member.member_name; auto& array_size = member.array_size; if (!array_size && skip_arrays) { - fmt::print(file, " .{} = to_guest(to_host_layout(from.data.{})),\n", decl_name, decl_name); + if (member.is_function_pointer) { + // Function pointers must be handled manually, so zero them out by default + fmt::print(file, " .{} {{ }},\n", decl_name); + } else { + fmt::print(file, " .{} = to_guest(to_host_layout(from.data.{})),\n", decl_name, decl_name); + } } else if (array_size && !skip_arrays) { // Copy element-wise below fmt::print(file, " for (size_t i = 0; i < {}; ++i) {{\n", array_size.value()); @@ -589,7 +599,10 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { } if (!thunk.return_type->isVoidType()) { - fmt::print(file, " args->rv = to_guest(to_host_layout<{}>(", thunk.return_type.getAsString()); + fmt::print(file, " args->rv = "); + if (!thunk.return_type->isFunctionPointerType()) { + fmt::print(file, "to_guest(to_host_layout<{}>(", thunk.return_type.getAsString()); + } } fmt::print(file, "{}(", function_to_call); { @@ -600,9 +613,14 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { if (cb != thunk.callbacks.end() && cb->second.is_stub) { return "fexfn_unpack_" + get_callback_name(function_name, cb->first) + "_stub"; } else if (cb != thunk.callbacks.end()) { - auto arg_name = fmt::format("args->a_{}.data", idx); + auto arg_name = fmt::format("args->a_{}", idx); // Use parameter directly // Use comma operator to inject a function call before returning the argument - return "(FinalizeHostTrampolineForGuestFunction(" + arg_name + "), " + arg_name + ")"; + // TODO: Avoid casting away the guest_layout + if (thunk.custom_host_impl) { + return fmt::format("(FinalizeHostTrampolineForGuestFunction({}), {})", arg_name, arg_name); + } else { + return fmt::format("(FinalizeHostTrampolineForGuestFunction({}), ({})(uint64_t {{ {}.data }}))", arg_name, get_type_name(context, thunk.param_types[idx].getTypePtr()), arg_name); + } } else if (thunk.param_annotations[idx].is_passthrough) { // Pass raw guest_layout return fmt::format("args->a_{}", idx); @@ -611,9 +629,9 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { } }; - file << format_function_args(thunk, format_param); + fmt::print(file, "{}", format_function_args(thunk, format_param)); } - if (!thunk.return_type->isVoidType()) { + if (!thunk.return_type->isVoidType() && !thunk.return_type->isFunctionPointerType()) { fmt::print(file, "))"); } fmt::print(file, ");\n"); diff --git a/ThunkLibs/include/common/Host.h b/ThunkLibs/include/common/Host.h index 02db74a012..9c2d00e0cc 100644 --- a/ThunkLibs/include/common/Host.h +++ b/ThunkLibs/include/common/Host.h @@ -97,8 +97,63 @@ struct guest_layout { static_assert(!std::is_enum_v, "No guest layout defined for this enum type. This is a bug in the thunk generator."); static_assert(!std::is_void_v, "Attempted to get guest layout of void. Missing annotation for void pointer?"); - using type = T; + using type = std::enable_if_t, T>; type data; + + guest_layout& operator=(const T from) { + data = from; + return *this; + } +}; + +template +struct guest_layout { +#ifdef IS_32BIT_THUNK + using type = uint32_t; +#else + using type = uint64_t; +#endif + type data; + + // Allow implicit conversion for function pointers, since they disallow use of host_layout + guest_layout& operator=(const T* from) requires (std::is_function_v) { + // TODO: Assert upper 32 bits are zero + data = reinterpret_cast(from); + return *this; + } + + guest_layout* get_pointer() { + return reinterpret_cast*>(uintptr_t { data }); + } + + const guest_layout* get_pointer() const { + return reinterpret_cast*>(uintptr_t { data }); + } +}; + +template +struct guest_layout { +#ifdef IS_32BIT_THUNK + using type = uint32_t; +#else + using type = uint64_t; +#endif + type data; + + // Allow implicit conversion for function pointers, since they disallow use of host_layout + guest_layout& operator=(const T* from) requires (std::is_function_v) { + // TODO: Assert upper 32 bits are zero + data = reinterpret_cast(from); + return *this; + } + + guest_layout* get_pointer() { + return reinterpret_cast*>(uintptr_t { data }); + } + + const guest_layout* get_pointer() const { + return reinterpret_cast*>(uintptr_t { data }); + } }; template @@ -133,7 +188,32 @@ const host_layout& to_host_layout(const T& t) { } template -inline guest_layout to_guest(const host_layout& from) { +struct host_layout { + T* data; + + static_assert(!std::is_function_v, "Function types must be handled separately"); + + // Assume underlying data is compatible and just convert the guest-sized pointer to 64-bit + explicit host_layout(const guest_layout& from) : data { (T*)(uintptr_t)from.data } { + } + + // TODO: Make this explicit? + host_layout() = default; +}; + +template +struct host_layout { + T* data; + + static_assert(!std::is_function_v, "Function types must be handled separately"); + + // Assume underlying data is compatible and just convert the guest-sized pointer to 64-bit + explicit host_layout(const guest_layout& from) : data { (T*)(uintptr_t)from.data } { + } +}; + +template +inline guest_layout to_guest(const host_layout& from) requires(!std::is_pointer_v) { if constexpr (std::is_enum_v) { // enums are represented by fixed-size integers in guest_layout, so explicitly cast them return guest_layout { static_cast>(from.data) }; @@ -143,6 +223,14 @@ inline guest_layout to_guest(const host_layout& from) { } } +template +inline guest_layout to_guest(const host_layout& from) { + // TODO: Assert upper 32 bits are zero + guest_layout ret; + ret.data = reinterpret_cast(from.data); + return ret; +} + template struct CallbackUnpack; @@ -254,6 +342,13 @@ void FinalizeHostTrampolineForGuestFunction(F* PreallocatedTrampolineForGuestFun (void*)&CallbackUnpack::CallGuestPtr); } +template +void FinalizeHostTrampolineForGuestFunction(guest_layout PreallocatedTrampolineForGuestFunction) { + FEXCore::FinalizeHostTrampolineForGuestFunction( + (FEXCore::HostToGuestTrampolinePtr*)PreallocatedTrampolineForGuestFunction.data, + (void*)&CallbackUnpack::CallGuestPtr); +} + // In the case of the thunk host_loader being the default, FEX need to use dlsym with RTLD_DEFAULT. // If FEX queried the symbol object directly then it wouldn't follow symbol overriding rules. // diff --git a/ThunkLibs/libvulkan/Host.cpp b/ThunkLibs/libvulkan/Host.cpp index 48bd5157e3..1cfaa42275 100644 --- a/ThunkLibs/libvulkan/Host.cpp +++ b/ThunkLibs/libvulkan/Host.cpp @@ -77,11 +77,21 @@ static VkResult FEXFN_IMPL(vkCreateInstance)(const VkInstanceCreateInfo* a_0, co } } - return LDR_PTR(vkCreateInstance)(vk_struct_base, nullptr, a_2.data); + VkInstance out; + auto ret = LDR_PTR(vkCreateInstance)(vk_struct_base, nullptr, &out); + if (ret == VK_SUCCESS) { + *a_2.get_pointer() = to_guest(to_host_layout(out)); + } + return ret; } static VkResult FEXFN_IMPL(vkCreateDevice)(VkPhysicalDevice a_0, const VkDeviceCreateInfo* a_1, const VkAllocationCallbacks* a_2, guest_layout a_3){ - return LDR_PTR(vkCreateDevice)(a_0, a_1, nullptr, a_3.data); + VkDevice out; + auto ret = LDR_PTR(vkCreateDevice)(a_0, a_1, nullptr, &out); + if (ret == VK_SUCCESS) { + *a_3.get_pointer() = to_guest(to_host_layout(out)); + } + return ret; } static VkResult FEXFN_IMPL(vkAllocateMemory)(VkDevice a_0, const VkMemoryAllocateInfo* a_1, const VkAllocationCallbacks* a_2, VkDeviceMemory* a_3){ @@ -95,7 +105,7 @@ static void FEXFN_IMPL(vkFreeMemory)(VkDevice a_0, VkDeviceMemory a_1, const VkA } static VkResult FEXFN_IMPL(vkCreateDebugReportCallbackEXT)(VkInstance a_0, guest_layout a_1, const VkAllocationCallbacks* a_2, VkDebugReportCallbackEXT* a_3) { - VkDebugReportCallbackCreateInfoEXT overridden_callback = *a_1.data; + auto overridden_callback = host_layout { *a_1.get_pointer() }.data; overridden_callback.pfnCallback = DummyVkDebugReportCallback; (void*&)LDR_PTR(vkCreateDebugReportCallbackEXT) = (void*)LDR_PTR(vkGetInstanceProcAddr)(a_0, "vkCreateDebugReportCallbackEXT"); return LDR_PTR(vkCreateDebugReportCallbackEXT)(a_0, &overridden_callback, nullptr, a_3); @@ -115,7 +125,7 @@ extern "C" VkBool32 DummyVkDebugUtilsMessengerCallback( static VkResult FEXFN_IMPL(vkCreateDebugUtilsMessengerEXT)( VkInstance_T* a_0, guest_layout a_1, const VkAllocationCallbacks* a_2, VkDebugUtilsMessengerEXT* a_3) { - VkDebugUtilsMessengerCreateInfoEXT overridden_callback = *a_1.data; + auto overridden_callback = host_layout { *a_1.get_pointer() }.data; overridden_callback.pfnUserCallback = DummyVkDebugUtilsMessengerCallback; (void*&)LDR_PTR(vkCreateDebugUtilsMessengerEXT) = (void*)LDR_PTR(vkGetInstanceProcAddr)(a_0, "vkCreateDebugUtilsMessengerEXT"); return LDR_PTR(vkCreateDebugUtilsMessengerEXT)(a_0, &overridden_callback, nullptr, a_3); diff --git a/ThunkLibs/libvulkan/libvulkan_interface.cpp b/ThunkLibs/libvulkan/libvulkan_interface.cpp index ba50a18429..1e4e0ec679 100644 --- a/ThunkLibs/libvulkan/libvulkan_interface.cpp +++ b/ThunkLibs/libvulkan/libvulkan_interface.cpp @@ -52,6 +52,11 @@ template<> struct fex_gen_type : fexgen::assume_c template<> struct fex_gen_type : fexgen::assume_compatible_data_layout {}; #endif +// Structures that contain function pointers +// TODO: Use custom repacking for these instead +template<> struct fex_gen_type : fexgen::emit_layout_wrappers {}; +template<> struct fex_gen_type : fexgen::emit_layout_wrappers {}; + #ifndef IS_32BIT_THUNK // The pNext member of this is a pointer to another VkBaseOutStructure, so data layout compatibility can't be inferred automatically template<> struct fex_gen_type : fexgen::assume_compatible_data_layout {}; diff --git a/ThunkLibs/libwayland-client/Host.cpp b/ThunkLibs/libwayland-client/Host.cpp index 00a29ef5cf..0aaffec7c7 100644 --- a/ThunkLibs/libwayland-client/Host.cpp +++ b/ThunkLibs/libwayland-client/Host.cpp @@ -60,7 +60,7 @@ extern "C" int fexfn_impl_libwayland_client_wl_proxy_add_listener(struct wl_prox // ? just indicates that the argument may be null, so it doesn't change the signature signature.erase(std::remove(signature.begin(), signature.end(), '?'), signature.end()); - auto callback = callback_raw.data[i]; + auto callback = reinterpret_cast(uintptr_t { callback_raw.get_pointer()[i].data }); if (signature == "") { // E.g. xdg_toplevel::close @@ -162,7 +162,9 @@ extern "C" int fexfn_impl_libwayland_client_wl_proxy_add_listener(struct wl_prox } // Pass the original function pointer table to the host wayland library. This ensures the table is valid until the listener is unregistered. - return fexldr_ptr_libwayland_client_wl_proxy_add_listener(proxy, callback_raw.data, data); + return fexldr_ptr_libwayland_client_wl_proxy_add_listener(proxy, + reinterpret_cast(callback_raw.get_pointer()), + data); } wl_interface* fexfn_impl_libwayland_client_fex_wl_exchange_interface_pointer(wl_interface* guest_interface, char const* name) { diff --git a/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp b/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp index 910c8a5fa3..7c17ad7556 100644 --- a/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp +++ b/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp @@ -30,8 +30,6 @@ TEST_CASE_METHOD(Fixture, "Trivial") { CHECK(GetDoubledValue(10) == 20); } -// Passing pointers across architecture boundaries is not yet supported on 32-bit -#if __SIZEOF_POINTER__ != 4 TEST_CASE_METHOD(Fixture, "Opaque data types") { { auto data = MakeOpaqueType(0x1234); @@ -44,4 +42,3 @@ TEST_CASE_METHOD(Fixture, "Opaque data types") { CHECK(GetUnionTypeA(&data) == 0x04030201); } } -#endif diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index 16a37cf8ee..d1ad7cda4c 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -287,14 +287,26 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v "};\n" "\n" "template\n" + "struct guest_layout {\n" + "#ifdef IS_32BIT_THUNK\n" + " using type = uint32_t;\n" + "#else\n" + " using type = uint64_t;\n" + "#endif\n" + " type data;\n" + "};\n" + "\n" + "template\n" "struct host_layout {\n" " T data;\n" "\n" " host_layout(const guest_layout& from);\n" "};\n" "\n" - "template guest_layout to_guest(const host_layout& from);\n" + "template guest_layout to_guest(const host_layout& from) requires(!std::is_pointer_v);\n" + "template guest_layout to_guest(const host_layout& from);\n" "template void FinalizeHostTrampolineForGuestFunction(F*);\n" + "template void FinalizeHostTrampolineForGuestFunction(guest_layout);\n" "template const host_layout& to_host_layout(const T& t);\n"; auto& filename = output_filenames.host; @@ -434,7 +446,7 @@ TEST_CASE_METHOD(Fixture, "FunctionPointerParameter") { hasDescendant(callExpr(callee(functionDecl(hasName("FinalizeHostTrampolineForGuestFunction"))), hasArgument(0, expr().bind("funcptr")))) )).check_binding("funcptr", +[](const clang::Expr* funcptr) { // Check that the argument type matches the function pointer - return funcptr->getType().getAsString() == "int (*)(char, char)"; + return funcptr->getType().getAsString() == "guest_layout"; })); // Host should export the unpacking function for function pointer arguments