Merge pull request FEX-Emu#3373 from neobrain/feature_thunk_custom_re…

…packing

Library Forwarding: Implement assisted struct repacking

ThunkLibs/Generator/analysis.cpp

@@ -300,6 +300,24 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
 
                 if (auto emitted_function = llvm::dyn_cast<clang::FunctionDecl>(template_args[0].getAsDecl())) {
                     // Process later
+                } else if (auto annotated_member = llvm::dyn_cast<clang::FieldDecl>(template_args[0].getAsDecl())) {
+                    {
+                        if (decl->getNumBases() != 1 || decl->bases_begin()->getType().getAsString() != "fexgen::custom_repack") {
+                            throw report_error(template_arg_loc, "Unsupported member annotation(s)");
+                        }
+
+                        if (!annotated_member->getType()->isPointerType() && !annotated_member->getType()->isArrayType()) {
+                            throw report_error(template_arg_loc, "custom_repack annotation requires pointer member");
+                        }
+                    }
+
+                    // Get or add parent type to list of structure types
+                    auto repack_info_it = types.emplace(context.getCanonicalType(annotated_member->getParent()->getTypeForDecl()), RepackedType {}).first;
+                    if (repack_info_it->second.assumed_compatible) {
+                        throw report_error(template_arg_loc, "May not annotate members of opaque types");
+                    }
+                    // Add member to its list of members
+                    repack_info_it->second.custom_repacked_members.insert(annotated_member->getNameAsString());
                 } else {
                     throw report_error(template_arg_loc, "Cannot annotate this kind of symbol");
                 }
@@ -356,14 +374,17 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
                             }
 
                             for (auto* member : type->getAsStructureType()->getDecl()->fields()) {
-                                  /*if (!member->getType()->isPointerType())*/ {
-                                      // TODO: Perform more elaborate validation for non-pointers to ensure ABI compatibility
-                                      continue;
-                                  }
-
-                                  throw report_error(member->getBeginLoc(), "Unannotated pointer member")
-                                        .addNote(report_error(param_loc, "in struct type", clang::DiagnosticsEngine::Note))
-                                        .addNote(report_error(template_arg_loc, "used in annotation here", clang::DiagnosticsEngine::Note));
+                                auto annotated_type = types.find(type->getCanonicalTypeUnqualified().getTypePtr());
+                                if (annotated_type == types.end() || !annotated_type->second.UsesCustomRepackFor(member)) {
+                                    /*if (!member->getType()->isPointerType())*/ {
+                                        // TODO: Perform more elaborate validation for non-pointers to ensure ABI compatibility
+                                        continue;
+                                    }
+
+                                    throw report_error(member->getBeginLoc(), "Unannotated pointer member")
+                                          .addNote(report_error(param_loc, "in struct type", clang::DiagnosticsEngine::Note))
+                                          .addNote(report_error(template_arg_loc, "used in annotation here", clang::DiagnosticsEngine::Note));
+                                }
                             }
                         };
 
@@ -518,7 +539,7 @@ void AnalysisAction::CoverReferencedTypes(clang::ASTContext& context) {
                     }
                     continue;
                 }
-                if (member_type->isUnionType() && !types.contains(member_type)) {
+                if (member_type->isUnionType() && !types.contains(member_type) && !type_repack_info.UsesCustomRepackFor(member)) {
                     throw std::runtime_error(fmt::format("\"{}\" has unannotated member \"{}\" of union type \"{}\"",
                                                          clang::QualType { type, 0 }.getAsString(),
                                                          member->getNameAsString(),

ThunkLibs/Generator/analysis.h

@@ -122,6 +122,16 @@ class AnalysisAction : public clang::ASTFrontendAction {
 
         // If true, emit guest_layout/host_layout definitions even if the type is non-repackable
         bool emit_layout_wrappers = false;
+
+        // Set of members (identified by their field name) with custom repacking
+        std::unordered_set<std::string> custom_repacked_members;
+
+        bool UsesCustomRepackFor(const clang::FieldDecl* member) const {
+            return custom_repacked_members.contains(member->getNameAsString());
+        }
+        bool UsesCustomRepackFor(const std::string& member_name) const {
+            return custom_repacked_members.contains(member_name);
+        }
     };
 
 protected:

ThunkLibs/Generator/data_layout.cpp

@@ -272,7 +272,9 @@ TypeCompatibility DataLayoutCompareAction::GetTypeCompatibility(
                 // * Pointer member is annotated
                 // TODO: Don't restrict this to structure types. it applies to pointers to builtin types too!
                 auto host_member_pointee_type = context.getCanonicalType(host_member_type->getPointeeType().getTypePtr());
-                if (types.contains(host_member_pointee_type) && types.at(host_member_pointee_type).assumed_compatible) {
+                if (types.at(type).UsesCustomRepackFor(host_member_field)) {
+                    member_compat.push_back(TypeCompatibility::Repackable);
+                } else if (types.contains(host_member_pointee_type) && types.at(host_member_pointee_type).assumed_compatible) {
                     // Pointee doesn't need repacking, but pointer needs extending on 32-bit
                     member_compat.push_back(is_32bit ? TypeCompatibility::Repackable : TypeCompatibility::Full);
                 } else if (host_member_pointee_type->isPointerType()) {

ThunkLibs/Generator/gen.cpp

@@ -234,11 +234,19 @@ void GenerateThunkLibsAction::EmitLayoutWrappers(
             };
             // Prefer initialization via the constructor's initializer list if possible (to detect unintended narrowing), otherwise initialize in the body
             for (auto* member : type->getAsStructureType()->getDecl()->fields()) {
-                map_field(member, true);
+                if (!type_repack_info.UsesCustomRepackFor(member)) {
+                    map_field(member, true);
+                } else {
+                  // Leave field uninitialized
+                }
             }
             fmt::print(file, "    }} {{\n");
             for (auto* member : type->getAsStructureType()->getDecl()->fields()) {
-                map_field(member, false);
+                if (!type_repack_info.UsesCustomRepackFor(member)) {
+                    map_field(member, false);
+                } else {
+                  // Leave field uninitialized
+                }
             }
         }
         fmt::print(file, "  }}\n");
@@ -274,16 +282,46 @@ void GenerateThunkLibsAction::EmitLayoutWrappers(
 
             // Prefer initialization via the constructor's initializer list if possible (to detect unintended narrowing), otherwise initialize in the body
             for (auto& member : guest_abi.at(struct_name).get_if_struct()->members) {
-                map_field2(member, true);
+                if (!type_repack_info.UsesCustomRepackFor(member.member_name)) {
+                    map_field2(member, true);
+                } else {
+                  // Leave field uninitialized
+                }
             }
             fmt::print(file, "  }} }};\n");
             for (auto& member : guest_abi.at(struct_name).get_if_struct()->members) {
-                map_field2(member, false);
+                if (!type_repack_info.UsesCustomRepackFor(member.member_name)) {
+                    map_field2(member, false);
+                } else {
+                  // Leave field uninitialized
+                }
             }
         }
         fmt::print(file, "  return ret;\n");
         fmt::print(file, "}}\n\n");
 
+        // Forward-declare user-provided repacking functions
+        if (type_repack_info.custom_repacked_members.empty()) {
+            fmt::print(file, "void fex_apply_custom_repacking_entry(host_layout<{}>& source, const guest_layout<{}>& from) {{\n", struct_name, struct_name);
+            fmt::print(file, "}}\n");
+            fmt::print(file, "bool fex_apply_custom_repacking_exit(guest_layout<{}>& into, host_layout<{}>& from) {{\n", struct_name, struct_name);
+            fmt::print(file, "  return false;\n");
+            fmt::print(file, "}}\n");
+        } else {
+            fmt::print(file, "void fex_custom_repack_entry(host_layout<{}>& into, const guest_layout<{}>& from);\n",
+                       struct_name, struct_name);
+            fmt::print(file, "bool fex_custom_repack_exit(guest_layout<{}>& into, const host_layout<{}>& from);\n\n",
+                       struct_name, struct_name);
+
+            fmt::print(file, "void fex_apply_custom_repacking_entry(host_layout<{}>& source, const guest_layout<{}>& from) {{\n", struct_name, struct_name);
+            fmt::print(file, "  fex_custom_repack_entry(source, from);\n");
+            fmt::print(file, "}}\n");
+
+            fmt::print(file, "bool fex_apply_custom_repacking_exit(guest_layout<{}>& into, host_layout<{}>& from) {{\n", struct_name, struct_name);
+            fmt::print(file, "  return fex_custom_repack_exit(into, from);\n");
+            fmt::print(file, "}}\n");
+        }
+
         fmt::print(file, "template<> inline constexpr bool has_compatible_data_layout<{}> = {};\n",
                    struct_name, (type_compat.at(type) == TypeCompatibility::Full));
     }
@@ -603,6 +641,7 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
                     continue;
                 }
 
+                // Layout repacking happens here
                 if (!param_type->isPointerType() || (is_assumed_compatible || pointee_compat == TypeCompatibility::Full) ||
                     param_type->getPointeeType()->isBuiltinType() /* TODO: handle size_t. Actually, properly check for data layout compatibility */) {
                     // Fully compatible

ThunkLibs/include/common/GeneratorInterface.h

@@ -12,6 +12,22 @@ struct callback_annotation_base {
 };
 struct callback_stub : callback_annotation_base {};
 
+// Member annotation to mark members handled by custom repacking. This enables
+// automatic struct repacking of structs with non-trivial members (pointers,
+// unions, ...). Repacking logic is auto-generated as usual, with the
+// difference that an external function is called to manually repack the
+// annotated members.
+//
+// Two functions must be implemented for the parent struct type:
+// * fex_custom_repack_entry, called after automatic repacking of the other members
+// * fex_custom_repack_exit, called on exit but before automatic exit-repacking
+//     of the other members. Non-trivial implementations must perform host->guest
+//     repacking manually and return the boolean value true.
+//
+// If multiple members of the same struct are annotated as custom_repack,
+// they must be handled in the same fex_custom_repack_entry/exit functions.
+struct custom_repack {};
+
 // Type annotation to indicate that guest_layout/host_layout definitions should
 // be emitted even if the type is non-repackable. Pointer members will be
 // copied (or zero-extended) without regard for the referred data.

ThunkLibs/include/common/Host.h

@@ -262,17 +262,31 @@ struct repack_wrapper {
   repack_wrapper(guest_layout<GuestT>& orig_arg_) : orig_arg(orig_arg_) {
     if (orig_arg.get_pointer()) {
       data = { *orig_arg_.get_pointer() };
+
+      if constexpr (!std::is_enum_v<T>) {
+        constexpr bool is_compatible = has_compatible_data_layout<T> && std::is_same_v<T, GuestT>;
+        if constexpr (!is_compatible && std::is_class_v<std::remove_pointer_t<T>>) {
+          fex_apply_custom_repacking_entry(*data, *orig_arg_.get_pointer());
+        }
+      }
     }
   }
 
   ~repack_wrapper() {
     // TODO: Properly detect opaque types
     if constexpr (requires(guest_layout<T> t, decltype(data) h) { t.get_pointer(); (bool)h; *data; }) {
-      if constexpr (!std::is_const_v<std::remove_pointer_t<T>>) { // Skip exit-repacking for const pointees
-        if (data) {
-          constexpr bool is_compatible = has_compatible_data_layout<T> && std::is_same_v<T, GuestT>;
-          if constexpr (!is_compatible && std::is_class_v<std::remove_pointer_t<T>>) {
-            *orig_arg.get_pointer() = to_guest(*data); // TODO: Only if annotated as out-parameter
+      // NOTE: It's assumed that the native host library didn't modify any
+      //       const-pointees, so we skip automatic exit repacking for them.
+      //       However, *custom* repacking must still be applied since it might
+      //       have unrelated side effects (such as deallocation of memory
+      //       reserved on entry)
+      if (!fex_apply_custom_repacking_exit(*orig_arg.get_pointer(), *data)) {
+        if constexpr (!std::is_const_v<std::remove_pointer_t<T>>) { // Skip exit-repacking for const pointees
+          if (data) {
+            constexpr bool is_compatible = has_compatible_data_layout<T> && std::is_same_v<T, GuestT>;
+            if constexpr (!is_compatible && std::is_class_v<std::remove_pointer_t<T>>) {
+              *orig_arg.get_pointer() = to_guest(*data); // TODO: Only if annotated as out-parameter
+            }
           }
         }
       }

ThunkLibs/libfex_thunk_test/Host.cpp

@@ -21,4 +21,12 @@ static uint32_t fexfn_impl_libfex_thunk_test_QueryOffsetOf(guest_layout<Reorderi
     }
 }
 
+void fex_custom_repack_entry(host_layout<CustomRepackedType>& to, guest_layout<CustomRepackedType> const& from) {
+  to.data.custom_repack_invoked = 1;
+}
+
+bool fex_custom_repack_exit(guest_layout<CustomRepackedType>& to, host_layout<CustomRepackedType> const& from) {
+  return false;
+}
+
 EXPORTS(libfex_thunk_test)

ThunkLibs/libfex_thunk_test/api.h

@@ -49,4 +49,17 @@ uint32_t GetReorderingTypeMember(ReorderingType*, int index);
 void ModifyReorderingTypeMembers(ReorderingType* data);
 uint32_t QueryOffsetOf(ReorderingType*, int index);
 
+
+/// Interfaces used to test assisted struct repacking
+
+// We enable custom repacking on the "data" member, with repacking code that
+// sets the first bit of "custom_repack_invoked" to 1 on entry.
+struct CustomRepackedType {
+    ReorderingType* data;
+    int custom_repack_invoked;
+};
+
+// Should return true if the custom repacker set "custom_repack_invoked" to true
+int RanCustomRepack(CustomRepackedType*);
+
 }

ThunkLibs/libfex_thunk_test/lib.cpp

@@ -47,4 +47,8 @@ void ModifyReorderingTypeMembers(ReorderingType* data) {
   data->b += 2;
 }
 
+int RanCustomRepack(CustomRepackedType* data) {
+  return data->custom_repack_invoked;
+}
+
 } // extern "C"

ThunkLibs/libfex_thunk_test/libfex_thunk_test_interface.cpp

@@ -28,3 +28,6 @@ template<> struct fex_gen_config<ModifyReorderingTypeMembers> {};
 
 template<> struct fex_gen_config<QueryOffsetOf> : fexgen::custom_host_impl {};
 template<> struct fex_gen_param<QueryOffsetOf, 0, ReorderingType*> : fexgen::ptr_passthrough {};
+
+template<> struct fex_gen_config<&CustomRepackedType::data> : fexgen::custom_repack {};
+template<> struct fex_gen_config<RanCustomRepack> {};

unittests/FEXLinuxTests/tests/thunks/thunk_testlib.cpp

@@ -29,6 +29,8 @@ struct Fixture {
   GET_SYMBOL(GetReorderingTypeMember);
   GET_SYMBOL(ModifyReorderingTypeMembers);
   GET_SYMBOL(QueryOffsetOf);
+
+  GET_SYMBOL(RanCustomRepack);
 };
 
 TEST_CASE_METHOD(Fixture, "Trivial") {
@@ -69,3 +71,8 @@ TEST_CASE_METHOD(Fixture, "Automatic struct repacking") {
     CHECK(GetReorderingTypeMember(&test_struct, 1) == 0x567a);
   };
 }
+
+TEST_CASE_METHOD(Fixture, "Assisted struct repacking") {
+  CustomRepackedType data {};
+  CHECK(RanCustomRepack(&data) == 1);
+}