From 26007168b01aad770e433e7bcca878ba97d899f9 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:37 +0100 Subject: [PATCH 1/9] Library Forwarding: Extend function pointer interface to take separate guest parameter lists Some types (notably size_t on 32-bit) have different sizes on the guest than on the host. This template function must be aware of these differences, so a second parameter list with fixed-size types must be provided to describe the guest types. Note that this information can't be queried through type traits: To a C++ compiler, size_t is indistuingishable from uint64_t. For this reason, the correct guest type must indeed be provided externally. --- ThunkLibs/Generator/gen.cpp | 19 ++++++++++++++++--- ThunkLibs/include/common/Host.h | 20 +++++++++++--------- unittests/ThunkLibs/generator.cpp | 2 +- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp index 3be1b127eb..85bfe6f377 100644 --- a/ThunkLibs/Generator/gen.cpp +++ b/ThunkLibs/Generator/gen.cpp @@ -705,10 +705,21 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { } // Endpoints for Guest->Host invocation of runtime host-function pointers + // NOTE: The function parameters may differ slightly between guest and host, + // e.g. due to differing sizes or due to data layout differences. + // Hence, two separate parameter lists are managed here. for (auto& host_funcptr_entry : thunked_funcptrs) { auto& [type, param_annotations] = host_funcptr_entry.second; + auto func_type = type->getAs(); std::string mangled_name = clang::QualType { type, 0 }.getAsString(); - auto info = LookupGuestFuncPtrInfo(host_funcptr_entry.first.c_str()); + FuncPtrInfo info = { }; + + info.result = func_type->getReturnType().getAsString(); + + // TODO: Use guest-sizes for integer types + for (auto arg : func_type->getParamTypes()) { + info.args.push_back(arg.getAsString()); + } std::string annotations; for (int param_idx = 0; param_idx < info.args.size(); ++param_idx) { @@ -725,8 +736,10 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { } annotations += "}"; } - fmt::print( file, " {{(uint8_t*)\"\\x{:02x}\", (void(*)(void *))&GuestWrapperForHostFunction<{}({})>::Call<{}>}}, // {}\n", - fmt::join(info.sha256, "\\x"), info.result, fmt::join(info.args, ", "), annotations, host_funcptr_entry.first); + auto guest_info = LookupGuestFuncPtrInfo(host_funcptr_entry.first.c_str()); + // TODO: Consider differences in guest/host return types + fmt::print( file, " {{(uint8_t*)\"\\x{:02x}\", (void(*)(void *))&GuestWrapperForHostFunction<{}({}){}{}>::Call<{}>}}, // {}\n", + fmt::join(guest_info.sha256, "\\x"), guest_info.result, fmt::join(info.args, ", "), guest_info.args.empty() ? "" : ", ", fmt::join(guest_info.args, ", "), annotations, host_funcptr_entry.first); } file << " { nullptr, nullptr }\n"; diff --git a/ThunkLibs/include/common/Host.h b/ThunkLibs/include/common/Host.h index 549c208283..96ccb47c86 100644 --- a/ThunkLibs/include/common/Host.h +++ b/ThunkLibs/include/common/Host.h @@ -371,8 +371,8 @@ struct CallbackUnpack { GuestcallInfo *guestcall; LOAD_INTERNAL_GUESTPTR_VIA_CUSTOM_ABI(guestcall); - PackedArguments packed_args = { - args... + PackedArguments...> packed_args = { + to_guest(to_host_layout(args))... }; guestcall->CallCallback(guestcall->GuestUnpacker, guestcall->GuestTarget, &packed_args); if constexpr (!std::is_void_v) { @@ -381,18 +381,20 @@ struct CallbackUnpack { } }; -template +template struct GuestWrapperForHostFunction; -template -struct GuestWrapperForHostFunction { +template +struct GuestWrapperForHostFunction { // Host functions called from Guest + // NOTE: GuestArgs typically matches up with Args, however there may be exceptions (e.g. size_t) template static void Call(void* argsv) { static_assert(sizeof...(Annotations) == sizeof...(Args)); + static_assert(sizeof...(GuestArgs) == sizeof...(Args)); - auto args = reinterpret_cast..., uintptr_t>*>(argsv); - constexpr auto CBIndex = sizeof...(Args); + auto args = reinterpret_cast..., uintptr_t>*>(argsv); + constexpr auto CBIndex = sizeof...(GuestArgs); uintptr_t cb; static_assert(CBIndex <= 18 || CBIndex == 23); if constexpr(CBIndex == 0) { @@ -439,9 +441,9 @@ struct GuestWrapperForHostFunction { // This is almost the same type as "Result func(Args..., uintptr_t)", but // individual parameters annotated as passthrough are replaced by guest_layout - auto callback = reinterpret_cast, Args>..., uintptr_t)>(cb); + auto callback = reinterpret_cast, Args>..., uintptr_t)>(cb); - auto f = [&callback](guest_layout... args, uintptr_t target) -> Result { + auto f = [&callback](guest_layout... args, uintptr_t target) -> Result { // Fold over each of Annotations, Args, and args. This will match up the elements in triplets. return callback(Projection(args)..., target); }; diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index 401bf88442..a5db7ae899 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -275,7 +275,7 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v " uintptr_t GuestTarget;\n" "};\n" "struct ParameterAnnotations {};\n" - "template\n" + "template\n" "struct GuestWrapperForHostFunction {\n" " template static void Call(void*);\n" "};\n" From 28ae84cf60d7d7d25bee8f348791c8d8ad61098f Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:37 +0100 Subject: [PATCH 2/9] Library Forwarding: Allow implicit conversions between various integer types Generally, implicit integer conversions are prohibited for data wrapped in guest_layout/host_layout, but a few types are exceptional: * char vs signed char vs unsigned char vs other 8-bit ints * wchar_t vs other 32-bit ints * size_t vs uint32_t (32-bit only) * long long vs other 64-bit ints * long vs long long (64-bit only) These combinations have the same data size, so conversions between them are explicitly allowed now. --- ThunkLibs/include/common/Host.h | 100 +++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 13 deletions(-) diff --git a/ThunkLibs/include/common/Host.h b/ThunkLibs/include/common/Host.h index 96ccb47c86..3783dab569 100644 --- a/ThunkLibs/include/common/Host.h +++ b/ThunkLibs/include/common/Host.h @@ -210,6 +210,14 @@ struct host_layout { explicit host_layout(const guest_layout& from) requires (std::is_enum_v) : data { static_cast(from.data) } { } + + // Allow conversion of integral types of smaller or equal size and same sign + // to each other. Zero-extension is applied if needed. + // Notably, this is useful for handling "long"/"long long" on 64-bit, as well + // as uint8_t/char. + template + explicit host_layout(const guest_layout& from) requires (std::is_integral_v && sizeof(U) <= sizeof(T) && std::is_convertible_v && std::is_signed_v == std::is_signed_v) : data { static_cast(from.data) } { + } }; // Explicitly turn a host type into its corresponding host_layout @@ -229,8 +237,29 @@ struct host_layout { explicit host_layout(const guest_layout& from) : data { (T*)(uintptr_t)from.data } { } - // TODO: Make this explicit? host_layout() = default; + + // Allow conversion of pointers to 64-bit integer types to "(un)signed long (long)*". + // This is useful for handling "long"/"long long" on 64-bit, which are distinct types + // but have equal data layout. + template + explicit host_layout(const guest_layout& from) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v) : data { (T*)(uintptr_t)from.data } { + } + template + explicit host_layout(const guest_layout& from) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v) : data { (T*)(uintptr_t)from.data } { + } + + // Allow conversion of pointers to 8-bit integer types to "char*". + // This is useful since "char"/"signed char"/"unsigned char"/"int8_t"/"uint8_t" + // may all be distinct types but have equal data layout + template + explicit host_layout(const guest_layout& from) requires (std::is_same_v, char> && std::is_integral_v && std::is_convertible_v && sizeof(U) == 1) : data { (T*)(uintptr_t)from.data } { + } + + // Allow conversion of pointers to 32-bit integer types to "wchar_t*". + template + explicit host_layout(const guest_layout& from) requires (std::is_same_v, wchar_t> && std::is_integral_v && std::is_convertible_v && sizeof(U) == sizeof(wchar_t)) : data { (T*)(uintptr_t)from.data } { + } }; template @@ -316,22 +345,67 @@ T* unwrap_host(repack_wrapper& val) { } 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) }; - } else { - guest_layout ret { .data = from.data }; +struct host_to_guest_convertible { + const host_layout from; + + // Conversion from host to guest layout for non-pointers + operator guest_layout() const 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) }; + } else { + guest_layout ret { .data = from.data }; + return ret; + } + } + + operator guest_layout() const requires(std::is_pointer_v) { + // TODO: Assert upper 32 bits are zero + guest_layout ret; + ret.data = reinterpret_cast(from.data); return ret; } -} + +#if IS_32BIT_THUNK + // Allow size_t -> uint32_t conversions, since they are so common on 32-bit + operator guest_layout() const requires(std::is_same_v) { + return { static_cast(from.data) }; + } +#endif + + // Make guest_layout of "long long" and "long" interoperable, since they are + // the same type as far as data layout is concerned. + operator guest_layout() const requires(std::is_same_v) { + return (guest_layout)reinterpret_cast&>(*this); + } + + // Make guest_layout of "char" and "uint8_t" interoperable + operator guest_layout() const requires(std::is_same_v) { + return (guest_layout)reinterpret_cast&>(*this); + } + + operator guest_layout() const requires(std::is_same_v) { + return (guest_layout)reinterpret_cast&>(*this); + } + + // Make guest_layout of "wchar_t" and "uint32_t" interoperable + operator guest_layout() const requires(std::is_same_v) { + return (guest_layout)reinterpret_cast&>(*this); + } + + static_assert(sizeof(wchar_t) == 4); + + // Allow conversion of integral types of same size and sign to each other. + // This is useful for handling "long"/"long long" on 64-bit, as well as uint8_t/char. + template + operator guest_layout() const requires (std::is_integral_v && sizeof(U) == sizeof(T) && std::is_convertible_v && std::is_signed_v == std::is_signed_v) { + return guest_layout { .data { static_cast(from.data) } }; + } +}; 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; +inline host_to_guest_convertible to_guest(const host_layout& from) { + return { from }; } template From 0ebf260ed04a7e6d5dc4af37c83d643f619b6c7c Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:37 +0100 Subject: [PATCH 3/9] Library Forwarding: Consider struct metadata equal if it only differs in integer member type names --- ThunkLibs/Generator/data_layout.cpp | 1 + ThunkLibs/Generator/data_layout.h | 7 +++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ThunkLibs/Generator/data_layout.cpp b/ThunkLibs/Generator/data_layout.cpp index 243ddfc287..6b5e760f4a 100644 --- a/ThunkLibs/Generator/data_layout.cpp +++ b/ThunkLibs/Generator/data_layout.cpp @@ -86,6 +86,7 @@ std::unordered_map ComputeDataLayout(const clang:: .member_name = field->getNameAsString(), .array_size = array_size, .is_function_pointer = field_type->isFunctionPointerType(), + .is_integral = field->getType()->isIntegerType(), }; // 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 1bcf76d5ec..07fd13330c 100644 --- a/ThunkLibs/Generator/data_layout.h +++ b/ThunkLibs/Generator/data_layout.h @@ -29,14 +29,17 @@ struct StructInfo : SimpleTypeInfo { std::string member_name; std::optional array_size; bool is_function_pointer; + bool is_integral; bool operator==(const MemberInfo& other) const { return size_bits == other.size_bits && offset_bits == other.offset_bits && - type_name == other.type_name && + // The type name may differ for integral types if all other parameters are equal + (type_name == other.type_name || (is_integral && other.is_integral)) && member_name == other.member_name && array_size == other.array_size && - is_function_pointer == other.is_function_pointer; + is_function_pointer == other.is_function_pointer && + is_integral == other.is_integral; } }; From d04c94fe802a09288a24ea68c7f951474bb0235a Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:38 +0100 Subject: [PATCH 4/9] Library Forwarding/gen: Map integers to fixed-size equivalents on guest --- ThunkLibs/Generator/analysis.cpp | 16 ++++++++++ ThunkLibs/Generator/analysis.h | 12 ++++++++ ThunkLibs/Generator/data_layout.cpp | 46 ++++++++++++++++++++++++++--- ThunkLibs/Generator/data_layout.h | 12 ++++++++ ThunkLibs/Generator/gen.cpp | 32 +++++++++++++++----- unittests/ThunkLibs/generator.cpp | 19 ++++++------ 6 files changed, 117 insertions(+), 20 deletions(-) diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp index be4c7bab49..b3c11c5681 100644 --- a/ThunkLibs/Generator/analysis.cpp +++ b/ThunkLibs/Generator/analysis.cpp @@ -430,6 +430,16 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) { types.emplace(context.getCanonicalType(param_type.getTypePtr()), RepackedType { }); } else if (param_type->isPointerType()) { auto pointee_type = param_type->getPointeeType()->getLocallyUnqualifiedSingleStepDesugaredType(); + + if (pointee_type->isIntegerType()) { + // Add builtin pointee type to type list + if (!pointee_type->isEnumeralType()) { + types.emplace(pointee_type.getTypePtr(), RepackedType { }); + } else { + types.emplace(context.getCanonicalType(pointee_type.getTypePtr()), RepackedType { }); + } + } + if (data.param_annotations[param_idx].assume_compatible) { // Nothing to do } else if (types.contains(context.getCanonicalType(pointee_type.getTypePtr())) && LookupType(context, pointee_type.getTypePtr()).assumed_compatible) { @@ -502,6 +512,12 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) { } void AnalysisAction::CoverReferencedTypes(clang::ASTContext& context) { + // Add common fixed-size integer types explicitly + for (unsigned size : { 8, 32, 64 }) { + types.emplace(context.getIntTypeForBitwidth(size, false).getTypePtr(), RepackedType {}); + types.emplace(context.getIntTypeForBitwidth(size, true).getTypePtr(), RepackedType {}); + } + // Repeat until no more children are appended for (bool changed = true; std::exchange(changed, false);) { for ( auto next_type_it = types.begin(), type_it = next_type_it; diff --git a/ThunkLibs/Generator/analysis.h b/ThunkLibs/Generator/analysis.h index 7a9d8a1f17..6b4d0f85d4 100644 --- a/ThunkLibs/Generator/analysis.h +++ b/ThunkLibs/Generator/analysis.h @@ -199,3 +199,15 @@ inline std::string get_type_name(const clang::ASTContext& context, const clang:: } return type_name; } + +inline std::string get_fixed_size_int_name(bool is_signed, int size) { + return (!is_signed ? "u" : "") + std::string { "int" } + std::to_string(size) + "_t"; +} + +inline std::string get_fixed_size_int_name(const clang::Type* type, int size) { + return get_fixed_size_int_name(type->isSignedIntegerType(), size); +} + +inline std::string get_fixed_size_int_name(const clang::Type* type, const clang::ASTContext& context) { + return get_fixed_size_int_name(type, context.getTypeSize(type)); +} diff --git a/ThunkLibs/Generator/data_layout.cpp b/ThunkLibs/Generator/data_layout.cpp index 6b5e760f4a..0076a69d74 100644 --- a/ThunkLibs/Generator/data_layout.cpp +++ b/ThunkLibs/Generator/data_layout.cpp @@ -87,6 +87,7 @@ std::unordered_map ComputeDataLayout(const clang:: .array_size = array_size, .is_function_pointer = field_type->isFunctionPointerType(), .is_integral = field->getType()->isIntegerType(), + .is_signed_integer = field->getType()->isSignedIntegerType(), }; // TODO: Process types in dependency-order. Currently we skip this @@ -150,8 +151,28 @@ ABI GetStableLayout(const clang::ASTContext& context, const std::unordered_mapsecond != type_info) { + if (auto struct_info = type_info.get_if_struct()) { + for (auto& member : struct_info->members) { + if (member.is_integral) { + // Map member types to fixed-size integers + auto alt_type_name = get_fixed_size_int_name(member.is_signed_integer, member.size_bits); + auto alt_type_info = SimpleTypeInfo { + .size_bits = member.size_bits, + .alignment_bits = context.getTypeAlign(context.getIntTypeForBitwidth(member.size_bits, member.is_signed_integer)), + }; + stable_layout.insert(std::pair { alt_type_name, alt_type_info }); + member.type_name = std::move(alt_type_name); + } + } + } + + auto [it, inserted] = stable_layout.insert(std::pair { type_name, std::move(type_info) }); + if (type->isIntegerType()) { + auto alt_type_name = get_fixed_size_int_name(type, context); + stable_layout.insert(std::pair { std::move(alt_type_name), type_info }); + } + + if (!inserted && it->second != type_info && !type->isIntegerType()) { throw std::runtime_error("Duplicate type information: Tried to re-register type \"" + type_name + "\""); } } @@ -169,6 +190,19 @@ static std::array GetSha256(const std::string& function_name) { return sha256; }; +std::string GetTypeNameWithFixedSizeIntegers(clang::ASTContext& context, clang::QualType type) { + if (type->isBuiltinType()) { + auto size = context.getTypeSize(type); + return fmt::format("uint{}_t", size); + } else if (type->isPointerType() && type->getPointeeType()->isBuiltinType() && context.getTypeSize(type->getPointeeType()) > 8) { + // TODO: Also apply this path to char-like types + auto size = context.getTypeSize(type->getPointeeType()); + return fmt::format("uint{}_t*", size); + } else { + return type.getAsString(); + } +} + void AnalyzeDataLayoutAction::OnAnalysisComplete(clang::ASTContext& context) { type_abi = GetStableLayout(context, ComputeDataLayout(context, types)); @@ -181,9 +215,11 @@ void AnalyzeDataLayoutAction::OnAnalysisComplete(clang::ASTContext& context) { auto cb_sha256 = GetSha256("fexcallback_" + mangled_name); FuncPtrInfo info = { cb_sha256 }; + // TODO: Also apply GetTypeNameWithFixedSizeIntegers here info.result = func_type->getReturnType().getAsString(); + for (auto arg : func_type->getParamTypes()) { - info.args.push_back(arg.getAsString()); + info.args.push_back(GetTypeNameWithFixedSizeIntegers(context, arg)); } type_abi.thunked_funcptrs[funcptr_id] = std::move(info); } @@ -219,7 +255,9 @@ TypeCompatibility DataLayoutCompareAction::GetTypeCompatibility( } auto type_name = get_type_name(context, type); - auto& guest_info = guest_abi.at(type_name); + // Look up the same type name in the guest map, + // unless it's an integer (which is mapped to fixed-size uintX_t types) + auto guest_info = guest_abi.at(!type->isIntegerType() ? type_name : get_fixed_size_int_name(type, context)); auto& host_info = host_abi.at(type->isBuiltinType() ? type : context.getCanonicalType(type)); const bool is_32bit = (guest_abi.pointer_size == 4); diff --git a/ThunkLibs/Generator/data_layout.h b/ThunkLibs/Generator/data_layout.h index 07fd13330c..c378806f83 100644 --- a/ThunkLibs/Generator/data_layout.h +++ b/ThunkLibs/Generator/data_layout.h @@ -30,6 +30,7 @@ struct StructInfo : SimpleTypeInfo { std::optional array_size; bool is_function_pointer; bool is_integral; + bool is_signed_integer; bool operator==(const MemberInfo& other) const { return size_bits == other.size_bits && @@ -100,6 +101,17 @@ ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map& data_layout); +/** + * Returns the type of the given name, but replaces any mentions of integer + * types with fixed-size equivalents. + * + * Examples: + * - int -> int32_t + * - unsigned long long* -> uint64_t* + * - MyStruct -> MyStruct (no change) + */ +std::string GetTypeNameWithFixedSizeIntegers(clang::ASTContext&, clang::QualType); + enum class TypeCompatibility { Full, // Type has matching data layout across architectures Repackable, // Type has different data layout but can be repacked automatically diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp index 85bfe6f377..e47e335bc1 100644 --- a/ThunkLibs/Generator/gen.cpp +++ b/ThunkLibs/Generator/gen.cpp @@ -189,7 +189,10 @@ void GenerateThunkLibsAction::EmitLayoutWrappers( fmt::print(file, " struct type {{\n"); // TODO: Insert any required padding bytes for (auto& member : guest_abi.at(struct_name).get_if_struct()->members) { - fmt::print(file, " guest_layout<{}> {};\n", member.type_name, member.member_name); + fmt::print( file, " guest_layout<{}{}> {};\n", + member.type_name, + member.array_size ? fmt::format("[{}]", member.array_size.value()) : "", + member.member_name); } fmt::print(file, " }};\n"); } @@ -546,6 +549,18 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { } } + auto get_guest_type_name = [this](clang::QualType type) { + if (type->isBuiltinType() && !type->isFloatingType()) { + auto size = guest_abi.at(type.getUnqualifiedType().getAsString()).get_if_simple_or_struct()->size_bits; + return get_fixed_size_int_name(type.getTypePtr(), size); + } else if (type->isPointerType() && type->getPointeeType()->isIntegerType() && !type->getPointeeType()->isEnumeralType() && !type->getPointeeType()->isVoidType()) { + auto size = guest_abi.at(type->getPointeeType().getUnqualifiedType().getAsString()).get_if_simple_or_struct()->size_bits; + return fmt::format("{}{}*", type->getPointeeType().isConstQualified() ? "const " : "", get_fixed_size_int_name(type->getPointeeType().getTypePtr(), size)); + } else { + return type.getUnqualifiedType().getAsString(); + } + }; + // Forward declarations for user-provided implementations if (thunk.custom_host_impl) { file << "static auto fexfn_impl_" << libname << "_" << function_name << "("; @@ -555,7 +570,7 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { file << (idx == 0 ? "" : ", "); if (thunk.param_annotations[idx].is_passthrough) { - fmt::print(file, "guest_layout<{}> a_{}", type.getAsString(), idx); + fmt::print(file, "guest_layout<{}> a_{}", get_guest_type_name(type), idx); } else { file << format_decl(type, fmt::format("a_{}", idx)); } @@ -588,10 +603,10 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { file << "struct " << struct_name << " {\n"; for (std::size_t idx = 0; idx < thunk.param_types.size(); ++idx) { - fmt::print(file, " guest_layout<{}> a_{};\n", get_type_name(context, thunk.param_types[idx].getTypePtr()), idx); + fmt::print(file, " guest_layout<{}> a_{};\n", get_guest_type_name(thunk.param_types[idx]), idx); } if (!thunk.return_type->isVoidType()) { - fmt::print(file, " guest_layout<{}> rv;\n", get_type_name(context, thunk.return_type.getTypePtr())); + fmt::print(file, " guest_layout<{}> rv;\n", get_guest_type_name(thunk.return_type)); } else if (thunk.param_types.size() == 0) { // Avoid "empty struct has size 0 in C, size 1 in C++" warning file << " char force_nonempty;\n"; @@ -711,14 +726,17 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) { for (auto& host_funcptr_entry : thunked_funcptrs) { auto& [type, param_annotations] = host_funcptr_entry.second; auto func_type = type->getAs(); - std::string mangled_name = clang::QualType { type, 0 }.getAsString(); FuncPtrInfo info = { }; + // TODO: Use GetTypeNameWithFixedSizeIntegers info.result = func_type->getReturnType().getAsString(); - // TODO: Use guest-sizes for integer types + // NOTE: In guest contexts, integer types must be mapped to + // fixed-size equivalents. Since this is a host context, this + // isn't strictly necessary here, but it makes matching up + // guest_layout/host_layout constructors easier. for (auto arg : func_type->getParamTypes()) { - info.args.push_back(arg.getAsString()); + info.args.push_back(GetTypeNameWithFixedSizeIntegers(context, arg)); } std::string annotations; diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index a5db7ae899..cc5298c275 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -301,7 +301,8 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v "struct host_layout {\n" " T data;\n" "\n" - " host_layout(const guest_layout& from);\n" + " template\n" + " host_layout(const guest_layout& from) requires(!std::is_integral_v || sizeof(T) == sizeof(U));\n" "};\n" "\n" "template\n" @@ -426,7 +427,7 @@ TEST_CASE_METHOD(Fixture, "FunctionPointerViaType") { hasInitializer(hasDescendant(declRefExpr(to(cxxMethodDecl(hasName("Call"), ofClass(hasName("GuestWrapperForHostFunction"))).bind("funcptr"))))) )).check_binding("funcptr", +[](const clang::CXXMethodDecl* decl) { auto parent = llvm::cast(decl->getParent()); - return parent->getTemplateArgs().get(0).getAsType().getAsString() == "int (char, char)"; + return parent->getTemplateArgs().get(0).getAsType().getAsString() == "int (unsigned char, unsigned char)"; })); } @@ -505,9 +506,9 @@ TEST_CASE_METHOD(Fixture, "MultipleParameters") { parameterCountIs(1), hasParameter(0, hasType(pointerType(pointee(hasUnqualifiedDesugaredType( recordType(hasDeclaration(decl( - has(fieldDecl(hasType(asString("guest_layout")))), - has(fieldDecl(hasType(asString("guest_layout")))), - has(fieldDecl(hasType(asString("guest_layout")))), + has(fieldDecl(hasType(asString("guest_layout")))), + has(fieldDecl(hasType(asString("guest_layout")))), + has(fieldDecl(hasType(asString("guest_layout")))), has(fieldDecl(hasType(asString("guest_layout<" CLANG_STRUCT_PREFIX "TestStruct>")))) )))))))) ))); @@ -622,9 +623,9 @@ TEST_CASE_METHOD(Fixture, "LayoutWrappers") { hasAnyTemplateArgument(refersToType(asString("struct A"))), // The member "data" exists and is defined to a struct... has(fieldDecl(hasName("data"), hasType(hasCanonicalType(hasDeclaration(decl( - // ... the members of which also use guest_layout - has(fieldDecl(hasName("a"), hasType(asString("guest_layout")))), - has(fieldDecl(hasName("b"), hasType(asString("guest_layout")))) + // ... the members of which also use guest_layout (with fixed-size integers) + has(fieldDecl(hasName("a"), hasType(asString("guest_layout")))), + has(fieldDecl(hasName("b"), hasType(asString("guest_layout")))) )))))) ))); CHECK_THAT(output, guest_converter_defined); @@ -671,7 +672,7 @@ TEST_CASE_METHOD(Fixture, "LayoutWrappers") { has(fieldDecl(hasName("data"), hasType(hasCanonicalType(hasDeclaration(decl( // ... the members of which also use guest_layout has(fieldDecl(hasName("a"), hasType(asString("guest_layout<" CLANG_STRUCT_PREFIX "B *>")))), - has(fieldDecl(hasName("b"), hasType(asString("guest_layout")))) + has(fieldDecl(hasName("b"), hasType(asString("guest_layout")))) )))))) ))); CHECK_THAT(output, guest_converter_defined); From 3ff3dc8769c56407beb0e669d7e574f986d7c3b9 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:38 +0100 Subject: [PATCH 5/9] Library Forwarding: Add unit test for diverging parameter type sizes --- ThunkLibs/libfex_thunk_test/api.h | 10 ++++++++++ ThunkLibs/libfex_thunk_test/lib.cpp | 4 ++++ .../libfex_thunk_test/libfex_thunk_test_interface.cpp | 2 ++ unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp | 6 ++++++ 4 files changed, 22 insertions(+) diff --git a/ThunkLibs/libfex_thunk_test/api.h b/ThunkLibs/libfex_thunk_test/api.h index 9d36762909..1503857d3c 100644 --- a/ThunkLibs/libfex_thunk_test/api.h +++ b/ThunkLibs/libfex_thunk_test/api.h @@ -64,4 +64,14 @@ struct CustomRepackedType { // Should return true if the custom repacker set "custom_repack_invoked" to true int RanCustomRepack(CustomRepackedType*); +/// Interface used to check that function arguments with different integer size +/// get forwarded correctly + +#if !defined(GUEST_THUNK_LIBRARY) +enum DivType : uint8_t {}; +#else +enum DivType : uint32_t {}; +#endif +int FunctionWithDivergentSignature(DivType, DivType, DivType, DivType); + } diff --git a/ThunkLibs/libfex_thunk_test/lib.cpp b/ThunkLibs/libfex_thunk_test/lib.cpp index dac27eae1b..0fbe9a1c10 100644 --- a/ThunkLibs/libfex_thunk_test/lib.cpp +++ b/ThunkLibs/libfex_thunk_test/lib.cpp @@ -55,4 +55,8 @@ int RanCustomRepack(CustomRepackedType* data) { return data->custom_repack_invoked; } +int FunctionWithDivergentSignature(DivType a, DivType b, DivType c, DivType d) { + return ((uint8_t)a << 24) | ((uint8_t)b << 16) | ((uint8_t)c << 8) | (uint8_t)d; +} + } // extern "C" diff --git a/ThunkLibs/libfex_thunk_test/libfex_thunk_test_interface.cpp b/ThunkLibs/libfex_thunk_test/libfex_thunk_test_interface.cpp index 1e4293ca07..8923722f36 100644 --- a/ThunkLibs/libfex_thunk_test/libfex_thunk_test_interface.cpp +++ b/ThunkLibs/libfex_thunk_test/libfex_thunk_test_interface.cpp @@ -33,3 +33,5 @@ template<> struct fex_gen_param : fexgen::ptr template<> struct fex_gen_config<&CustomRepackedType::data> : fexgen::custom_repack {}; template<> struct fex_gen_config {}; + +template<> struct fex_gen_config {}; diff --git a/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp b/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp index 183a5c31dc..2ba4fd1111 100644 --- a/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp +++ b/unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp @@ -34,6 +34,8 @@ struct Fixture { GET_SYMBOL(QueryOffsetOf); GET_SYMBOL(RanCustomRepack); + + GET_SYMBOL(FunctionWithDivergentSignature); }; TEST_CASE_METHOD(Fixture, "Trivial") { @@ -83,3 +85,7 @@ TEST_CASE_METHOD(Fixture, "Assisted struct repacking") { CustomRepackedType data {}; CHECK(RanCustomRepack(&data) == 1); } + +TEST_CASE_METHOD(Fixture, "Function signature with differing parameter sizes") { + CHECK(FunctionWithDivergentSignature(DivType{1}, DivType{2}, DivType{3}, DivType{4}) == 0x01020304); +} From 0114c874a787769f94ab070e99aa87257173914d Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:38 +0100 Subject: [PATCH 6/9] unittests/Library Forwarding: Respect silent flag when compiling SourceWithAST --- unittests/ThunkLibs/generator.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index cc5298c275..85d468bf6c 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -23,7 +23,7 @@ struct SourceWithAST { std::string code; std::unique_ptr ast; - SourceWithAST(std::string_view input); + SourceWithAST(std::string_view input, bool silent_compile = false); }; std::ostream& operator<<(std::ostream& os, const SourceWithAST& ast) { @@ -177,7 +177,7 @@ class DefinesPublicFunction : public HasASTMatching { } }; -SourceWithAST::SourceWithAST(std::string_view input) : code(input) { +SourceWithAST::SourceWithAST(std::string_view input, bool silent_compile) : code(input) { // Call run_tool with a ToolAction that assigns this->ast struct ToolAction : clang::tooling::ToolAction { @@ -195,7 +195,7 @@ SourceWithAST::SourceWithAST(std::string_view input) : code(input) { } } tool_action { ast }; - run_tool(tool_action, code); + run_tool(tool_action, code, silent_compile); } /** @@ -335,7 +335,7 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v result.replace(pos, 6, " "); // Replace "static" with 6 spaces (avoiding reallocation) } } - return SourceWithAST { std::string { prelude } + result }; + return SourceWithAST { std::string { prelude } + result, silent }; } Fixture::GenOutput Fixture::run_thunkgen(std::string_view prelude, std::string_view code, bool silent) { From b40d8f5679ec67c09edc96d050e9b9b78ee2e834 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 17:17:38 +0100 Subject: [PATCH 7/9] unittests/Library Forwarding: Add tests for fixed-size integer mapping --- unittests/ThunkLibs/generator.cpp | 117 +++++++++++++++++++++++++++++- 1 file changed, 113 insertions(+), 4 deletions(-) diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index 85d468bf6c..d32bf53ff4 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -301,16 +301,37 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v "struct host_layout {\n" " T data;\n" "\n" - " template\n" - " host_layout(const guest_layout& from) requires(!std::is_integral_v || sizeof(T) == sizeof(U));\n" + " explicit host_layout(const guest_layout&);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && sizeof(U) <= sizeof(T) && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + "};\n" + "\n" + "template\n" + "struct host_layout {\n" + " T* data;\n" + " explicit host_layout(const guest_layout&);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_same_v, char> && std::is_integral_v && std::is_convertible_v && sizeof(U) == 1);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_same_v, wchar_t> && std::is_integral_v && std::is_convertible_v && sizeof(U) == sizeof(wchar_t));\n" + "};\n" + "\n" + "template struct host_to_guest_convertible {\n" + " operator guest_layout();\n" + " operator guest_layout() const requires(std::is_same_v);\n" + " operator guest_layout() const requires(std::is_same_v);\n" + " operator guest_layout() const requires(std::is_same_v);\n" + " operator guest_layout() const requires(std::is_same_v);\n" + " template operator guest_layout() const requires (std::is_integral_v && sizeof(U) == sizeof(T) && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + "#if IS_32BIT_THUNK\n" + " operator guest_layout() const requires(std::is_same_v);\n" + "#endif\n" "};\n" "\n" "template\n" "struct repack_wrapper {};\n" "template\n" "repack_wrapper make_repack_wrapper(guest_layout& orig_arg);\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 host_to_guest_convertible to_guest(const host_layout& from);\n" "template void FinalizeHostTrampolineForGuestFunction(F*);\n" "template void FinalizeHostTrampolineForGuestFunction(guest_layout);\n" "template T& unwrap_host(host_layout&);\n" @@ -681,6 +702,94 @@ TEST_CASE_METHOD(Fixture, "LayoutWrappers") { } } +// Some integer types are differently sized on the guest than on the host. +// All integer types are mapped to fixed-size equivalents when mentioned in a +// guest context hence. This test ensures the mapping is done correctly and +// the resulting guest_layout instantiations are convertible to host_layout. +TEST_CASE_METHOD(Fixture, "Mapping guest integers to fixed-size") { + auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64); + INFO(guest_abi); + + // Run each test a second time to ensure fixed-size integer mapping is + // applied to pointees as well + const std::string ptr = GENERATE("", " *"); + + // These types are differently sized on 32-bit guests + SECTION("(u)intptr_t / size_t / long") { + const std::string type = GENERATE("long", "unsigned long", "uintptr_t", "intptr_t", "size_t"); + INFO(type + ptr); + const auto code = + "#include \n" + "#include \n" + "#include \n" + "void func(" + type + ptr + ");\n" + "template<> struct fex_gen_config {};\n"; + if (!ptr.empty() && guest_abi == GuestABI::X86_32) { + // Guest points to a 32-bit integer, but the host to a 64-bit one. + // This should be detected as a failure. + CHECK_THROWS_WITH(run_thunkgen_host("", code, guest_abi, true), Catch::Contains("initialization of 'host_layout", Catch::CaseSensitive::No)); + } else { + const auto output = run_thunkgen_host("", code, guest_abi); + std::string expected_type = "guest_layout<"; + if (type == "size_t" || type.starts_with("u")) { + expected_type += "u"; + } + expected_type += (guest_abi == GuestABI::X86_32 ? + "int32_t" : "int64_t"); + expected_type += ptr + ">"; + CHECK_THAT(output, + matches(functionDecl( + hasName("fexfn_unpack_libtest_func"), + // Packed argument struct should contain all parameters + parameterCountIs(1), + hasParameter(0, hasType(pointerType(pointee(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(decl( + has(fieldDecl(hasType(asString(expected_type)))) + )))))))) + ))); + } + } + + // Most integer types are uniquely defined by specifying their size and + // their signedness. (w)char-types and long-types are special: + // * "char", "signed char", and "unsigned char" are different types + // * "wchar_t" is mapped to "guest_layout", but uint32_t + // itself is a type alias for "int" + // * "long long" is mapped to "guest_layout", but uint64_t + // itself is a type alias for "long" on 64-bit (which is a different + // type than "long long") + // + // This test section ensures that the correct fixed-size integers are used + // *and* that they can be converted to host_layout. + SECTION("Special integer types") { + const std::string type = GENERATE("long long", "unsigned long long", "char", "unsigned char", "signed char", "wchar_t"); + std::string fixed_size_type = ((type.starts_with("unsigned") || type == "char" || type == "wchar_t") ? "u" : ""); + if (type.ends_with("long long")) { + fixed_size_type += "int64_t"; + } else if (type.ends_with("wchar_t")) { + fixed_size_type += "int32_t"; + } else { + fixed_size_type += "int8_t"; + } + INFO(type + ptr + ", expecting " + fixed_size_type + ptr); + const auto code = + "#include \n" + "#include \n" + "void func(" + type + ptr + ");\n" + "template<> struct fex_gen_config {};\n"; + const auto output = run_thunkgen_host("", code, guest_abi); + CHECK_THAT(output, + matches(functionDecl( + hasName("fexfn_unpack_libtest_func"), + // Packed argument struct should contain all parameters + parameterCountIs(1), + hasParameter(0, hasType(pointerType(pointee(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(decl( + has(fieldDecl(hasType(asString("guest_layout<" + fixed_size_type + ptr + ">")))) + )))))))) + ))); + } +} + TEST_CASE_METHOD(Fixture, "StructRepacking") { auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64); INFO(guest_abi); From a4c1aaa6bcf3cdcb0e983d2fdd70f56b61b12127 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 30 Jan 2024 20:18:27 +0100 Subject: [PATCH 8/9] Library Forwarding: Fix build for clang < 16 --- ThunkLibs/include/common/Host.h | 17 +++++++++++++---- unittests/ThunkLibs/generator.cpp | 6 ++++-- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/ThunkLibs/include/common/Host.h b/ThunkLibs/include/common/Host.h index 3783dab569..d69cfd9f48 100644 --- a/ThunkLibs/include/common/Host.h +++ b/ThunkLibs/include/common/Host.h @@ -227,6 +227,13 @@ const host_layout& to_host_layout(const T& t) { return reinterpret_cast&>(t); } +template +constexpr bool is_long_or_longlong = + std::is_same_v || + std::is_same_v || + std::is_same_v || + std::is_same_v; + template struct host_layout { T* data; @@ -243,10 +250,12 @@ struct host_layout { // This is useful for handling "long"/"long long" on 64-bit, which are distinct types // but have equal data layout. template - explicit host_layout(const guest_layout& from) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v) : data { (T*)(uintptr_t)from.data } { - } - template - explicit host_layout(const guest_layout& from) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v) : data { (T*)(uintptr_t)from.data } { + explicit host_layout(const guest_layout& from) requires (is_long_or_longlong> && std::is_integral_v && std::is_convertible_v && std::is_signed_v == std::is_signed_v +#if __clang_major__ >= 16 + // Old clang versions don't support using sizeof on incomplete types when evaluating requires() + && sizeof(T) == sizeof(U) +#endif + ) : data { (T*)(uintptr_t)from.data } { } // Allow conversion of pointers to 8-bit integer types to "char*". diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp index d32bf53ff4..b2f6411085 100644 --- a/unittests/ThunkLibs/generator.cpp +++ b/unittests/ThunkLibs/generator.cpp @@ -305,12 +305,14 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && sizeof(U) <= sizeof(T) && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" "};\n" "\n" + "template constexpr bool is_long = std::is_same_v || std::is_same_v;\n" + "template constexpr bool is_longlong = std::is_same_v || std::is_same_v;\n" "template\n" "struct host_layout {\n" " T* data;\n" " explicit host_layout(const guest_layout&);\n" - " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" - " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && std::is_integral_v && sizeof(T) == sizeof(U) && std::is_same_v>, long long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && sizeof(U) == sizeof(long) && sizeof(long) == 8 && is_long> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" + " template explicit host_layout(const guest_layout&) requires (std::is_integral_v && sizeof(U) == sizeof(long long) && is_longlong> && std::is_convertible_v && std::is_signed_v == std::is_signed_v);\n" " template explicit host_layout(const guest_layout&) requires (std::is_same_v, char> && std::is_integral_v && std::is_convertible_v && sizeof(U) == 1);\n" " template explicit host_layout(const guest_layout&) requires (std::is_same_v, wchar_t> && std::is_integral_v && std::is_convertible_v && sizeof(U) == sizeof(wchar_t));\n" "};\n" From 8e1aaa055911a4b5d3c7a8f0d71fc93294f78c89 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Wed, 31 Jan 2024 18:28:22 +0100 Subject: [PATCH 9/9] Library Forwarding: Avoid de-sugaring pointee types Doing so would accidentally resolve typedefs before. This code isn't needed now that integers are mapped to fixed-size equivalents anyway. --- ThunkLibs/Generator/analysis.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp index b3c11c5681..802ef573ac 100644 --- a/ThunkLibs/Generator/analysis.cpp +++ b/ThunkLibs/Generator/analysis.cpp @@ -429,7 +429,7 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) { check_struct_type(param_type.getTypePtr()); types.emplace(context.getCanonicalType(param_type.getTypePtr()), RepackedType { }); } else if (param_type->isPointerType()) { - auto pointee_type = param_type->getPointeeType()->getLocallyUnqualifiedSingleStepDesugaredType(); + auto pointee_type = param_type->getPointeeType(); if (pointee_type->isIntegerType()) { // Add builtin pointee type to type list