Merge pull request #3392 from neobrain/feature_libfwd_fixed_size_ints

Library Forwarding: Handle cross-architecture differences of integer types

ThunkLibs/Generator/analysis.cpp

@@ -429,7 +429,17 @@ 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
+                                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;

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));
+}

ThunkLibs/Generator/data_layout.cpp

@@ -86,6 +86,8 @@ std::unordered_map<const clang::Type*, TypeInfo> ComputeDataLayout(const clang::
                 .member_name = field->getNameAsString(),
                 .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
@@ -149,8 +151,28 @@ ABI GetStableLayout(const clang::ASTContext& context, const std::unordered_map<c
 
     for (auto [type, type_info] : data_layout) {
         auto type_name = get_type_name(context, type);
-        auto [it, inserted] = stable_layout.insert(std::pair { type_name, type_info });
-        if (!inserted && it->second != 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 + "\"");
         }
     }
@@ -168,6 +190,19 @@ static std::array<uint8_t, 32> 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));
 
@@ -180,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);
     }
@@ -218,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);

ThunkLibs/Generator/data_layout.h

@@ -29,14 +29,18 @@ struct StructInfo : SimpleTypeInfo {
         std::string member_name;
         std::optional<uint64_t> 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 &&
                     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;
         }
     };
 
@@ -97,6 +101,17 @@ ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map<con
 // As a consequence, type information is indexed by type name instead of clang::Type.
 ABI GetStableLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, TypeInfo>& 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

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";
@@ -705,10 +720,24 @@ 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;
-            std::string mangled_name = clang::QualType { type, 0 }.getAsString();
-            auto info = LookupGuestFuncPtrInfo(host_funcptr_entry.first.c_str());
+            auto func_type = type->getAs<clang::FunctionProtoType>();
+            FuncPtrInfo info = { };
+
+            // TODO: Use GetTypeNameWithFixedSizeIntegers
+            info.result = func_type->getReturnType().getAsString();
+
+            // 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(GetTypeNameWithFixedSizeIntegers(context, arg));
+            }
 
             std::string annotations;
             for (int param_idx = 0; param_idx < info.args.size(); ++param_idx) {
@@ -725,8 +754,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";