From 7791e0090d5044a9d3418be9ab8fa724b389b87f Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:02 +0200
Subject: [PATCH 1/9] Thunks: Disable 32-bit host thunks

These are not supported yet.
---
 ThunkLibs/HostLibs/CMakeLists.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ThunkLibs/HostLibs/CMakeLists.txt b/ThunkLibs/HostLibs/CMakeLists.txt
index 799bd53df5..bdc9d4e592 100644
--- a/ThunkLibs/HostLibs/CMakeLists.txt
+++ b/ThunkLibs/HostLibs/CMakeLists.txt
@@ -91,7 +91,7 @@ function(add_host_lib NAME GUEST_BITNESS)
   endif()
 endfunction()
 
-set (BITNESS_LIST "32;64")
+set (BITNESS_LIST "64")
 foreach(GUEST_BITNESS IN LISTS BITNESS_LIST)
   #add_host_lib(fex_malloc_symbols ${GUEST_BITNESS})
 
@@ -189,7 +189,10 @@ foreach(GUEST_BITNESS IN LISTS BITNESS_LIST)
   target_include_directories(libdrm-${GUEST_BITNESS}-deps INTERFACE /usr/include/drm/)
   target_include_directories(libdrm-${GUEST_BITNESS}-deps INTERFACE /usr/include/libdrm/)
   add_host_lib(drm ${GUEST_BITNESS})
+endforeach()
 
+set (BITNESS_LIST "32;64")
+foreach(GUEST_BITNESS IN LISTS BITNESS_LIST)
   generate(libfex_thunk_test ${CMAKE_CURRENT_SOURCE_DIR}/../libfex_thunk_test/libfex_thunk_test_interface.cpp ${GUEST_BITNESS})
   add_host_lib(fex_thunk_test ${GUEST_BITNESS})
 endforeach()

From d65d29903b55d6b8ad810978623e3e765586795d Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:02 +0200
Subject: [PATCH 2/9] Thunks/gen: Rename EmitOutput to OnAnalysisComplete

---
 ThunkLibs/Generator/analysis.cpp | 2 +-
 ThunkLibs/Generator/analysis.h   | 2 +-
 ThunkLibs/Generator/gen.cpp      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp
index 39f02577c9..64357ee13a 100644
--- a/ThunkLibs/Generator/analysis.cpp
+++ b/ThunkLibs/Generator/analysis.cpp
@@ -128,7 +128,7 @@ void AnalysisAction::ExecuteAction() {
 
     try {
         ParseInterface(context);
-        EmitOutput(context);
+        OnAnalysisComplete(context);
     } catch (ClangDiagnosticAsException& exception) {
         exception.Report(context.getDiagnostics());
     }
diff --git a/ThunkLibs/Generator/analysis.h b/ThunkLibs/Generator/analysis.h
index 7aeaa9d77a..00b0dc8c30 100644
--- a/ThunkLibs/Generator/analysis.h
+++ b/ThunkLibs/Generator/analysis.h
@@ -110,7 +110,7 @@ class AnalysisAction : public clang::ASTFrontendAction {
     void ParseInterface(clang::ASTContext&);
 
     // Called from ExecuteAction() after parsing is complete
-    virtual void EmitOutput(clang::ASTContext&) {};
+    virtual void OnAnalysisComplete(clang::ASTContext&) {};
 
     std::vector<clang::DeclContext*> decl_contexts;
 
diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp
index bef050da23..1a93d93720 100644
--- a/ThunkLibs/Generator/gen.cpp
+++ b/ThunkLibs/Generator/gen.cpp
@@ -20,7 +20,7 @@ class GenerateThunkLibsAction : public AnalysisAction {
 
 private:
     // Generate helper code for thunk libraries and write them to the output file
-    void EmitOutput(clang::ASTContext&) override;
+    void OnAnalysisComplete(clang::ASTContext&) override;
 
     const std::string& libfilename;
     std::string libname; // sanitized filename, usable as part of emitted function names
@@ -47,7 +47,7 @@ static std::string format_function_args(const FunctionParams& params, Fn&& forma
     return ret;
 };
 
-void GenerateThunkLibsAction::EmitOutput(clang::ASTContext& context) {
+void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
     static auto format_decl = [](clang::QualType type, const std::string_view& name) {
         clang::QualType innermostPointee = type;
         while (innermostPointee->isPointerType()) {

From 371bf50c765682e05f4ddf07e96fb0ecac66573a Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:02 +0200
Subject: [PATCH 3/9] Thunks/gen: Track data types passed across architecture
 boundaries

The set of these types is tracked in AnalysisAction, to which extensive
verification logic is added to detect potential incompatibilities and to
enforce use of annotatations where needed.
---
 ThunkLibs/Generator/analysis.cpp | 289 ++++++++++++++++++++++++-------
 ThunkLibs/Generator/analysis.h   |  29 ++++
 2 files changed, 253 insertions(+), 65 deletions(-)

diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp
index 64357ee13a..6e395479e2 100644
--- a/ThunkLibs/Generator/analysis.cpp
+++ b/ThunkLibs/Generator/analysis.cpp
@@ -128,6 +128,9 @@ void AnalysisAction::ExecuteAction() {
 
     try {
         ParseInterface(context);
+        if (StrictModeEnabled(context)) {
+            CoverReferencedTypes(context);
+        }
         OnAnalysisComplete(context);
     } catch (ClangDiagnosticAsException& exception) {
         exception.Report(context.getDiagnostics());
@@ -149,9 +152,25 @@ FindClassTemplateDeclByName(clang::DeclContext& decl_context, std::string_view s
     }
 }
 
+static ParameterAnnotations GetParameterAnnotations(clang::ASTContext& context, clang::CXXRecordDecl* decl) {
+    if (!decl->hasDefinition()) {
+        return {};
+    }
+
+    ErrorReporter report_error { context };
+    ParameterAnnotations ret;
+
+    for (const clang::CXXBaseSpecifier& base : decl->bases()) {
+        throw report_error(base.getSourceRange().getBegin(), "Unknown parameter annotation");
+    }
+
+    return ret;
+}
+
 void AnalysisAction::ParseInterface(clang::ASTContext& context) {
     ErrorReporter report_error { context };
 
+    // TODO: Assert fex_gen_type is not declared at non-global namespaces
     if (auto template_decl = FindClassTemplateDeclByName(*context.getTranslationUnitDecl(), "fex_gen_type")) {
         for (auto* decl : template_decl->specializations()) {
             const auto& template_args = decl->getTemplateArgs();
@@ -161,8 +180,42 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
             //       named types (e.g. GLuint/GLenum) are represented by
             //       different Type instances. The canonical type they refer
             //       to is unique, however.
-            auto type = context.getCanonicalType(template_args[0].getAsType()).getTypePtr();
-            funcptr_types.insert(type);
+            clang::QualType type = context.getCanonicalType(template_args[0].getAsType());
+            type = type->getLocallyUnqualifiedSingleStepDesugaredType();
+
+            if (type->isFunctionPointerType() || type->isFunctionType()) {
+                funcptr_types.insert(type.getTypePtr());
+            } else {
+                [[maybe_unused]] auto [it, inserted] = types.emplace(context.getCanonicalType(type.getTypePtr()), RepackedType { });
+                assert(inserted);
+            }
+        }
+    }
+
+    // Process function parameter annotations
+    std::unordered_map<const clang::FunctionDecl*, std::unordered_map<unsigned, ParameterAnnotations>> param_annotations;
+    for (auto& decl_context : decl_contexts) {
+        if (auto template_decl = FindClassTemplateDeclByName(*decl_context, "fex_gen_param")) {
+            for (auto* decl : template_decl->specializations()) {
+                const auto& template_args = decl->getTemplateArgs();
+                assert(template_args.size() == 3);
+
+                auto function = llvm::dyn_cast<clang::FunctionDecl>(template_args[0].getAsDecl());
+                auto param_idx = template_args[1].getAsIntegral().getZExtValue();
+                clang::QualType type = context.getCanonicalType(template_args[2].getAsType());
+                type = type->getLocallyUnqualifiedSingleStepDesugaredType();
+
+                if (param_idx >= function->getNumParams()) {
+                    throw report_error(decl->getTypeAsWritten()->getTypeLoc().getAs<clang::TemplateSpecializationTypeLoc>().getArgLoc(1).getLocation(), "Out-of-bounds parameter index passed to fex_gen_param");
+                }
+
+                if (!type->isVoidType() && !context.hasSameType(type, function->getParamDecl(param_idx)->getType())) {
+                    throw report_error(decl->getTypeAsWritten()->getTypeLoc().getAs<clang::TemplateSpecializationTypeLoc>().getArgLoc(2).getLocation(), "Type passed to fex_gen_param doesn't match the function signature")
+                              .addNote(report_error(function->getParamDecl(param_idx)->getTypeSourceInfo()->getTypeLoc().getBeginLoc(), "Expected this type instead"));
+                }
+
+                param_annotations[function][param_idx] = GetParameterAnnotations(context, decl);
+            }
         }
     }
 
@@ -220,94 +273,200 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
 
                 const auto template_arg_loc = decl->getTypeAsWritten()->getTypeLoc().castAs<clang::TemplateSpecializationTypeLoc>().getArgLoc(0).getLocation();
 
-                auto emitted_function = llvm::dyn_cast<clang::FunctionDecl>(template_args[0].getAsDecl());
-                assert(emitted_function && "Argument is not a function");
-                auto return_type = emitted_function->getReturnType();
+                if (auto emitted_function = llvm::dyn_cast<clang::FunctionDecl>(template_args[0].getAsDecl())) {
+                    auto return_type = emitted_function->getReturnType();
 
-                const auto annotations = GetAnnotations(context, decl);
-                if (return_type->isFunctionPointerType() && !annotations.returns_guest_pointer) {
-                    throw report_error( template_arg_loc,
-                                        "Function pointer return types require explicit annotation\n");
-                }
+                    const auto annotations = GetAnnotations(context, decl);
+                    if (return_type->isFunctionPointerType() && !annotations.returns_guest_pointer) {
+                        throw report_error( template_arg_loc,
+                                            "Function pointer return types require explicit annotation\n");
+                    }
+
+                    // TODO: Use the types as written in the signature instead?
+                    ThunkedFunction data;
+                    data.function_name = emitted_function->getName().str();
+                    data.return_type = return_type;
+                    data.is_variadic = emitted_function->isVariadic();
 
-                // TODO: Use the types as written in the signature instead?
-                ThunkedFunction data;
-                data.function_name = emitted_function->getName().str();
-                data.return_type = return_type;
-                data.is_variadic = emitted_function->isVariadic();
+                    data.decl = emitted_function;
 
-                data.decl = emitted_function;
+                    data.custom_host_impl = annotations.custom_host_impl;
 
-                data.custom_host_impl = annotations.custom_host_impl;
+                    data.param_annotations = param_annotations[emitted_function];
 
-                for (std::size_t param_idx = 0; param_idx < emitted_function->param_size(); ++param_idx) {
-                    auto* param = emitted_function->getParamDecl(param_idx);
-                    data.param_types.push_back(param->getType());
+                    const int retval_index = -1;
+                    for (int param_idx = retval_index; param_idx < (int)emitted_function->param_size(); ++param_idx) {
+                        auto param_type = param_idx == retval_index ? emitted_function->getReturnType() : emitted_function->getParamDecl(param_idx)->getType();
+                        auto param_loc = param_idx == retval_index ? emitted_function->getReturnTypeSourceRange().getBegin() : emitted_function->getParamDecl(param_idx)->getBeginLoc();
 
-                    if (param->getType()->isFunctionPointerType()) {
-                        auto funcptr = param->getFunctionType()->getAs<clang::FunctionProtoType>();
-                        ThunkedCallback callback;
-                        callback.return_type = funcptr->getReturnType();
-                        for (auto& cb_param : funcptr->getParamTypes()) {
-                            callback.param_types.push_back(cb_param);
+                        if (param_idx != retval_index) {
+                            data.param_types.push_back(param_type);
+                        } else if (param_type->isVoidType()) {
+                            continue;
                         }
-                        callback.is_stub = annotations.callback_strategy == CallbackStrategy::Stub;
-                        callback.is_guest = annotations.callback_strategy == CallbackStrategy::Guest;
-                        callback.is_variadic = funcptr->isVariadic();
 
-                        if (callback.is_guest && !data.custom_host_impl) {
-                            throw report_error(template_arg_loc, "callback_guest can only be used with custom_host_impl");
+                        auto check_struct_type = [&](const clang::Type* type) {
+                            if (type->isIncompleteType()) {
+                                if (!StrictModeEnabled(context)) {
+                                    return;
+                                }
+                                throw report_error(type->getAsTagDecl()->getBeginLoc(), "Unannotated pointer with incomplete struct type; consider using an opaque_type annotation")
+                                      .addNote(report_error(emitted_function->getNameInfo().getLoc(), "in function", clang::DiagnosticsEngine::Note))
+                                      .addNote(report_error(template_arg_loc, "used in annotation here", clang::DiagnosticsEngine::Note));
+                            }
+
+                            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));
+                            }
+                        };
+
+                        if (param_type->isFunctionPointerType()) {
+                            if (param_idx == retval_index) {
+                                // TODO: We already rely on this in a few places...
+//                                throw report_error(template_arg_loc, "Support for returning function pointers is not implemented");
+                                continue;
+                            }
+                            auto funcptr = emitted_function->getParamDecl(param_idx)->getFunctionType()->getAs<clang::FunctionProtoType>();
+                            ThunkedCallback callback;
+                            callback.return_type = funcptr->getReturnType();
+                            for (auto& cb_param : funcptr->getParamTypes()) {
+                                callback.param_types.push_back(cb_param);
+                            }
+                            callback.is_stub = annotations.callback_strategy == CallbackStrategy::Stub;
+                            callback.is_guest = annotations.callback_strategy == CallbackStrategy::Guest;
+                            callback.is_variadic = funcptr->isVariadic();
+
+                            if (callback.is_guest && !data.custom_host_impl) {
+                                throw report_error(template_arg_loc, "callback_guest can only be used with custom_host_impl");
+                            }
+
+                            data.callbacks.emplace(param_idx, callback);
+                            if (!callback.is_stub && !callback.is_guest && !data.custom_host_impl) {
+                                funcptr_types.insert(context.getCanonicalType(funcptr));
+                            }
+
+                            if (data.callbacks.size() != 1) {
+                                throw report_error(template_arg_loc, "Support for more than one callback is untested");
+                            }
+                            if (funcptr->isVariadic() && !callback.is_stub) {
+                                throw report_error(template_arg_loc, "Variadic callbacks are not supported");
+                            }
+                        } else if (param_type->isBuiltinType()) {
+                            // NOTE: Intentionally not using getCanonicalType here since that would turn e.g. size_t into platform-specific types
+                            // TODO: Still, we may want to de-duplicate some of these...
+                            types.emplace(param_type.getTypePtr(), RepackedType { });
+                        } else if (param_type->isEnumeralType()) {
+                            types.emplace(context.getCanonicalType(param_type.getTypePtr()), RepackedType { });
+                        } else if ( param_type->isStructureType()) {
+                            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();
+                            if ( pointee_type->isStructureType()) {
+                                check_struct_type(pointee_type.getTypePtr());
+                                types.emplace(context.getCanonicalType(pointee_type.getTypePtr()), RepackedType { });
+                            } else if (false /* TODO: Can't check if this is unsupported until data layout analysis is complete */) {
+                                throw report_error(param_loc, "Unsupported parameter type")
+                                              .addNote(report_error(emitted_function->getNameInfo().getLoc(), "in function", clang::DiagnosticsEngine::Note))
+                                              .addNote(report_error(template_arg_loc, "used in definition here", clang::DiagnosticsEngine::Note));
+                            }
+                        } else {
+                            // TODO: For non-pointer parameters, perform more elaborate validation to ensure ABI compatibility
                         }
+                    }
+
+                    thunked_api.push_back(ThunkedAPIFunction { (const FunctionParams&)data, data.function_name, data.return_type,
+                                                                namespace_info.host_loader.empty() ? "dlsym_default" : namespace_info.host_loader,
+                                                                data.is_variadic || annotations.custom_guest_entrypoint,
+                                                                data.is_variadic,
+                                                                std::nullopt });
+                    if (namespace_info.generate_guest_symtable) {
+                        thunked_api.back().symtable_namespace = namespace_idx;
+                    }
 
-                        data.callbacks.emplace(param_idx, callback);
-                        if (!callback.is_stub && !callback.is_guest) {
-                            funcptr_types.insert(context.getCanonicalType(funcptr));
+                    if (data.is_variadic) {
+                        if (!annotations.uniform_va_type) {
+                            throw report_error(decl->getBeginLoc(), "Variadic functions must be annotated with parameter type using uniform_va_type");
                         }
 
-                        if (data.callbacks.size() != 1) {
-                            throw report_error(template_arg_loc, "Support for more than one callback is untested");
+                        // Convert variadic argument list into a count + pointer pair
+                        data.param_types.push_back(context.getSizeType());
+                        data.param_types.push_back(context.getPointerType(*annotations.uniform_va_type));
+                        types.emplace(context.getSizeType()->getTypePtr(), RepackedType { });
+                        if (!annotations.uniform_va_type.value()->isVoidPointerType()) {
+                            types.emplace(annotations.uniform_va_type->getTypePtr(), RepackedType { });
                         }
-                        if (funcptr->isVariadic() && !callback.is_stub) {
-                            throw report_error(template_arg_loc, "Variadic callbacks are not supported");
+                    }
+
+                    if (data.is_variadic) {
+                        // This function is thunked through an "_internal" symbol since its signature
+                        // is different from the one in the native host/guest libraries.
+                        data.function_name = data.function_name + "_internal";
+                        if (data.custom_host_impl) {
+                            throw report_error(decl->getBeginLoc(), "Custom host impl requested but this is implied by the function signature already");
                         }
+                        data.custom_host_impl = true;
                     }
-                }
 
-                thunked_api.push_back(ThunkedAPIFunction { (const FunctionParams&)data, data.function_name, data.return_type,
-                                                            namespace_info.host_loader.empty() ? "dlsym_default" : namespace_info.host_loader,
-                                                            data.is_variadic || annotations.custom_guest_entrypoint,
-                                                            data.is_variadic,
-                                                            std::nullopt });
-                if (namespace_info.generate_guest_symtable) {
-                    thunked_api.back().symtable_namespace = namespace_idx;
+                    // For indirect calls, register the function signature as a function pointer type
+                    if (namespace_info.indirect_guest_calls) {
+                        funcptr_types.insert(context.getCanonicalType(emitted_function->getFunctionType()));
+                    }
+
+                    thunks.push_back(std::move(data));
                 }
+            }
+        }
+    }
+}
 
-                if (data.is_variadic) {
-                    if (!annotations.uniform_va_type) {
-                        throw report_error(decl->getBeginLoc(), "Variadic functions must be annotated with parameter type using uniform_va_type");
-                    }
+void AnalysisAction::CoverReferencedTypes(clang::ASTContext& context) {
+    // 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;
+              type_it != types.end();
+              type_it = next_type_it) {
+            ++next_type_it;
+            const auto& [type, type_repack_info] = *type_it;
+            if (!type->isStructureType()) {
+                continue;
+            }
 
-                    // Convert variadic argument list into a count + pointer pair
-                    data.param_types.push_back(context.getSizeType());
-                    data.param_types.push_back(context.getPointerType(*annotations.uniform_va_type));
+            for (auto* member : type->getAsStructureType()->getDecl()->fields()) {
+                auto member_type = member->getType().getTypePtr();
+                while (member_type->isArrayType()) {
+                    member_type = member_type->getArrayElementTypeNoTypeQual();
+                }
+                while (member_type->isPointerType()) {
+                    member_type = member_type->getPointeeType().getTypePtr();
                 }
 
-                if (data.is_variadic) {
-                    // This function is thunked through an "_internal" symbol since its signature
-                    // is different from the one in the native host/guest libraries.
-                    data.function_name = data.function_name + "_internal";
-                    if (data.custom_host_impl) {
-                        throw report_error(decl->getBeginLoc(), "Custom host impl requested but this is implied by the function signature already");
-                    }
-                    data.custom_host_impl = true;
+                if (!member_type->isBuiltinType()) {
+                    member_type = context.getCanonicalType(member_type);
+                }
+                if (member_type->isUnionType() && !types.contains(member_type)) {
+                    throw std::runtime_error(fmt::format("\"{}\" has unannotated member \"{}\" of union type \"{}\"",
+                                                         clang::QualType { type, 0 }.getAsString(),
+                                                         member->getNameAsString(),
+                                                         clang::QualType { member_type, 0 }.getAsString()));
                 }
 
-                // For indirect calls, register the function signature as a function pointer type
-                if (namespace_info.indirect_guest_calls) {
-                    funcptr_types.insert(context.getCanonicalType(emitted_function->getFunctionType()));
+                if (!member_type->isStructureType() && !(member_type->isBuiltinType() && !member_type->isVoidType()) && !member_type->isEnumeralType()) {
+                    continue;
                 }
 
-                thunks.push_back(std::move(data));
+                auto [new_type_it, inserted] = types.emplace(member_type, RepackedType { });
+                if (inserted) {
+                    changed = true;
+                    next_type_it = new_type_it;
+                }
             }
         }
     }
diff --git a/ThunkLibs/Generator/analysis.h b/ThunkLibs/Generator/analysis.h
index 00b0dc8c30..f6538fc04d 100644
--- a/ThunkLibs/Generator/analysis.h
+++ b/ThunkLibs/Generator/analysis.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <clang/Basic/FileEntry.h>
 #include <clang/Frontend/FrontendAction.h>
 
 #include <memory>
@@ -21,6 +22,10 @@ struct ThunkedCallback : FunctionParams {
     bool is_variadic = false;
 };
 
+struct ParameterAnnotations {
+    bool operator==(const ParameterAnnotations&) const = default;
+};
+
 /**
  * Guest<->Host transition point.
  *
@@ -52,6 +57,10 @@ struct ThunkedFunction : FunctionParams {
     // Maps parameter index to ThunkedCallback
     std::unordered_map<unsigned, ThunkedCallback> callbacks;
 
+    // Maps parameter index to ParameterAnnotations
+    // TODO: Use index -1 for the return value?
+    std::unordered_map<unsigned, ParameterAnnotations> param_annotations;
+
     clang::FunctionDecl* decl;
 };
 
@@ -105,10 +114,16 @@ class AnalysisAction : public clang::ASTFrontendAction {
 
     std::unique_ptr<clang::ASTConsumer> CreateASTConsumer(clang::CompilerInstance&, clang::StringRef /*file*/) override;
 
+    struct RepackedType {
+    };
+
 protected:
     // Build the internal API representation by processing fex_gen_config and other annotated entities
     void ParseInterface(clang::ASTContext&);
 
+    // Recursively extend the type set to include types of struct members
+    void CoverReferencedTypes(clang::ASTContext&);
+
     // Called from ExecuteAction() after parsing is complete
     virtual void OnAnalysisComplete(clang::ASTContext&) {};
 
@@ -116,8 +131,22 @@ class AnalysisAction : public clang::ASTFrontendAction {
 
     std::vector<ThunkedFunction> thunks;
     std::vector<ThunkedAPIFunction> thunked_api;
+
     std::unordered_set<const clang::Type*> funcptr_types;
 
+    std::unordered_map<const clang::Type*, RepackedType> types;
     std::optional<unsigned> lib_version;
     std::vector<NamespaceInfo> namespaces;
+
+    RepackedType& LookupType(clang::ASTContext& context, const clang::Type* type) {
+      return types.at(context.getCanonicalType(type));
+    }
 };
+
+// Analysis can't process interfaces of real libraries, yet. This function
+// defines a "strict mode" to use for tests, only. Real libraries will switch
+// to strict mode once analysis is more feature-complete.
+inline bool StrictModeEnabled(clang::ASTContext& context) {
+    auto filename = context.getSourceManager().getFileEntryForID(context.getSourceManager().getMainFileID())->getName();
+    return filename.endswith("libfex_thunk_test_interface.cpp") || filename.endswith("gen_input.cpp");
+}

From 070fa9f924a86ee41b92d3984d43a860a90f4a09 Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:03 +0200
Subject: [PATCH 4/9] Thunks/gen: Add data layout analysis

This adds a ComputeDataLayout function that maps a set of clang::Types
to an internal representation of their data layout (size, member list, ...).
---
 ThunkLibs/Generator/CMakeLists.txt  |   2 +-
 ThunkLibs/Generator/analysis.h      |  37 +++++++++
 ThunkLibs/Generator/data_layout.cpp | 123 ++++++++++++++++++++++++++++
 ThunkLibs/Generator/data_layout.h   |  82 +++++++++++++++++++
 ThunkLibs/Generator/gen.cpp         |   6 ++
 5 files changed, 249 insertions(+), 1 deletion(-)
 create mode 100644 ThunkLibs/Generator/data_layout.cpp
 create mode 100644 ThunkLibs/Generator/data_layout.h

diff --git a/ThunkLibs/Generator/CMakeLists.txt b/ThunkLibs/Generator/CMakeLists.txt
index 55f27216b8..f628db6874 100644
--- a/ThunkLibs/Generator/CMakeLists.txt
+++ b/ThunkLibs/Generator/CMakeLists.txt
@@ -1,7 +1,7 @@
 find_package(Clang REQUIRED CONFIG)
 find_package(OpenSSL REQUIRED COMPONENTS Crypto)
 
-add_library(thunkgenlib analysis.cpp gen.cpp)
+add_library(thunkgenlib analysis.cpp data_layout.cpp gen.cpp)
 target_include_directories(thunkgenlib INTERFACE ${CMAKE_CURRENT_SOURCE_DIR})
 target_include_directories(thunkgenlib SYSTEM PUBLIC ${CLANG_INCLUDE_DIRS})
 target_link_libraries(thunkgenlib PUBLIC clang-cpp LLVM)
diff --git a/ThunkLibs/Generator/analysis.h b/ThunkLibs/Generator/analysis.h
index f6538fc04d..3255444ce5 100644
--- a/ThunkLibs/Generator/analysis.h
+++ b/ThunkLibs/Generator/analysis.h
@@ -143,6 +143,43 @@ class AnalysisAction : public clang::ASTFrontendAction {
     }
 };
 
+inline std::string get_type_name(const clang::ASTContext& context, const clang::Type* type) {
+    if (type->isBuiltinType()) {
+        // Skip canonicalization
+        return clang::QualType { type, 0 }.getAsString();
+    }
+
+    if (auto decl = type->getAsTagDecl()) {
+        // Replace unnamed types with a placeholder. This will fail to compile if referenced
+        // anywhere in generated code, but at least it will point to a useful location.
+        //
+        // A notable exception are C-style struct declarations like "typedef struct (unnamed) { ... } MyStruct;".
+        // A typedef name is associated with these for linking purposes, so
+        // getAsString() will produce a usable identifier.
+        // TODO: Consider turning this into a hard error instead of replacing the name
+        if (!decl->getDeclName() && !decl->getTypedefNameForAnonDecl()) {
+            auto loc = context.getSourceManager().getPresumedLoc(decl->getLocation());
+            std::string filename = loc.getFilename();
+            filename = std::move(filename).substr(filename.rfind("/"));
+            filename = std::move(filename).substr(1);
+            std::replace(filename.begin(), filename.end(), '.', '_');
+            return "unnamed_type_" + filename + "_" + std::to_string(loc.getLine());
+        }
+    }
+
+    auto type_name = clang::QualType { context.getCanonicalType(type), 0 }.getAsString();
+    if (type_name.starts_with("struct ")) {
+        type_name = type_name.substr(7);
+    }
+    if (type_name.starts_with("class ") || type_name.starts_with("union ")) {
+        type_name = type_name.substr(6);
+    }
+    if (type_name.starts_with("enum ")) {
+        type_name = type_name.substr(5);
+    }
+    return type_name;
+}
+
 // Analysis can't process interfaces of real libraries, yet. This function
 // defines a "strict mode" to use for tests, only. Real libraries will switch
 // to strict mode once analysis is more feature-complete.
diff --git a/ThunkLibs/Generator/data_layout.cpp b/ThunkLibs/Generator/data_layout.cpp
new file mode 100644
index 0000000000..ebefd20574
--- /dev/null
+++ b/ThunkLibs/Generator/data_layout.cpp
@@ -0,0 +1,123 @@
+#include "analysis.h"
+#include "data_layout.h"
+#include "interface.h"
+
+#include <fmt/format.h>
+
+#include <openssl/sha.h>
+
+constexpr bool enable_debug_output = false;
+
+std::unordered_map<const clang::Type*, TypeInfo> ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, AnalysisAction::RepackedType>& types) {
+    std::unordered_map<const clang::Type*, TypeInfo> layout;
+
+    // First, add all types directly used in function signatures of the library API to the meta set
+    for (const auto& [type, type_repack_info] : types) {
+        if (type->isIncompleteType()) {
+            throw std::runtime_error("Cannot compute data layout of incomplete type \"" + clang::QualType { type, 0 }.getAsString() + "\". Did you forget any annotations?");
+        }
+
+        if (type->isStructureType()) {
+            StructInfo info;
+            info.size_bits = context.getTypeSize(type);
+            info.alignment_bits = context.getTypeAlign(type);
+
+            auto [_, inserted] = layout.insert(std::pair { context.getCanonicalType(type), info });
+            if (!inserted) {
+                throw std::runtime_error("Failed to gather type metadata: Type \"" + clang::QualType { type, 0 }.getAsString() + "\" already registered");
+            }
+        } else if (type->isBuiltinType() || type->isEnumeralType()) {
+            SimpleTypeInfo info;
+            info.size_bits = context.getTypeSize(type);
+            info.alignment_bits = context.getTypeAlign(type);
+
+            // NOTE: Non-enum types are intentionally not canonicalized since that would turn e.g. size_t into platform-specific types
+            auto [_, inserted] = layout.insert(std::pair { type->isEnumeralType() ? context.getCanonicalType(type) : type, info });
+            if (!inserted) {
+                throw std::runtime_error("Failed to gather type metadata: Type \"" + clang::QualType { type, 0 }.getAsString() + "\" already registered");
+            }
+        }
+    }
+
+    // Then, add information about members
+    for (const auto& [type, type_repack_info] : types) {
+        if (!type->isStructureType()) {
+            continue;
+        }
+
+        auto& info = *layout.at(context.getCanonicalType(type)).get_if_struct();
+
+        for (auto* field : type->getAsStructureType()->getDecl()->fields()) {
+            auto field_type = field->getType().getTypePtr();
+            std::optional<uint64_t> array_size;
+            if (auto array_type = llvm::dyn_cast<clang::ConstantArrayType>(field->getType())) {
+                array_size = array_type->getSize().getZExtValue();
+                field_type = array_type->getElementType().getTypePtr();
+                if (llvm::isa<clang::ConstantArrayType>(field_type)) {
+                    throw std::runtime_error("Unsupported multi-dimensional array member \"" + field->getNameAsString() + "\" in type \"" + clang::QualType { type, 0 }.getAsString() + "\"");
+                }
+            }
+
+            StructInfo::MemberInfo member_info {
+                .size_bits = context.getTypeSize(field->getType()), // Total size even for arrays
+                .offset_bits = context.getFieldOffset(field),
+                .type_name = get_type_name(context, field_type),
+                .member_name = field->getNameAsString(),
+                .array_size = array_size,
+            };
+
+            // TODO: Process types in dependency-order. Currently we skip this
+            //       check if we haven't processed the member type already,
+            //       which is only safe since this is a consistency check
+            if (field_type->isStructureType() && layout.contains(context.getCanonicalType(field_type))) {
+                // Assert for self-consistency
+                auto field_meta = layout.at(context.getCanonicalType(field_type));
+                (void)types.at(context.getCanonicalType(field_type));
+                if (auto field_info = field_meta.get_if_simple_or_struct()) {
+                    if (field_info->size_bits != member_info.size_bits / member_info.array_size.value_or(1)) {
+                        throw std::runtime_error("Inconsistent type size detected");
+                    }
+                }
+            }
+
+            // Add built-in types, even if referenced through a pointer
+            for (auto* inner_field_type = field_type; inner_field_type; inner_field_type = inner_field_type->getPointeeType().getTypePtrOrNull()) {
+                if (inner_field_type->isBuiltinType() || inner_field_type->isEnumeralType()) {
+                    // The analysis pass doesn't explicitly register built-in types, so add them manually here
+                    SimpleTypeInfo info {
+                        .size_bits = context.getTypeSize(inner_field_type),
+                        .alignment_bits = context.getTypeAlign(inner_field_type),
+                    };
+                    if (!inner_field_type->isBuiltinType()) {
+                        inner_field_type = context.getCanonicalType(inner_field_type);
+                    }
+                    [[maybe_unused]] auto [prev, inserted] = layout.insert(std::pair { inner_field_type, info });
+//                    if (!inserted && prev->second != TypeInfo { info }) {
+//                        // TODO: Throw error since consistency check failed
+//                    }
+                }
+            }
+
+            info.members.push_back(member_info);
+        }
+    }
+
+    if (enable_debug_output) {
+        for (const auto& [type, info] : layout) {
+            auto basic_info = info.get_if_simple_or_struct();
+            if (!basic_info) {
+                continue;
+            }
+
+            fprintf(stderr, "  Host entry %s: %lu (%lu)\n", clang::QualType { type, 0 }.getAsString().c_str(), basic_info->size_bits / 8, basic_info->alignment_bits / 8);
+
+            if (auto struct_info = info.get_if_struct()) {
+                for (const auto& member : struct_info->members) {
+                    fprintf(stderr, "    Offset %lu-%lu: %s %s%s\n", member.offset_bits / 8, (member.offset_bits + member.size_bits - 1) / 8, member.type_name.c_str(), member.member_name.c_str(), member.array_size ? fmt::format("[{}]", member.array_size.value()).c_str() : "");
+                }
+            }
+        }
+    }
+
+    return layout;
+}
diff --git a/ThunkLibs/Generator/data_layout.h b/ThunkLibs/Generator/data_layout.h
new file mode 100644
index 0000000000..27d197c5da
--- /dev/null
+++ b/ThunkLibs/Generator/data_layout.h
@@ -0,0 +1,82 @@
+#pragma once
+
+#include "analysis.h"
+
+#include <clang/Frontend/FrontendAction.h>
+
+#include <cstdint>
+#include <optional>
+#include <string>
+#include <unordered_map>
+#include <variant>
+#include <vector>
+
+struct SimpleTypeInfo {
+    uint64_t size_bits;
+    uint64_t alignment_bits;
+
+    bool operator==(const SimpleTypeInfo& other) const {
+        return  size_bits == other.size_bits &&
+                alignment_bits == other.alignment_bits;
+    }
+};
+
+struct StructInfo : SimpleTypeInfo {
+    struct MemberInfo {
+        uint64_t size_bits; // size of this member. For arrays, total size of all elements
+        uint64_t offset_bits;
+        std::string type_name;
+        std::string member_name;
+        std::optional<uint64_t> array_size;
+
+        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;
+        }
+    };
+
+    std::vector<MemberInfo> members;
+
+    bool operator==(const StructInfo& other) const {
+        return (const SimpleTypeInfo&)*this == (const SimpleTypeInfo&)other &&
+                std::equal(members.begin(), members.end(), other.members.begin(), other.members.end());
+    }
+};
+
+struct TypeInfo : std::variant<std::monostate, SimpleTypeInfo, StructInfo> {
+    using Parent = std::variant<std::monostate, SimpleTypeInfo, StructInfo>;
+
+    TypeInfo() = default;
+    TypeInfo(const SimpleTypeInfo& info) : Parent(info) {}
+    TypeInfo(const StructInfo& info) : Parent(info) {}
+
+    // Opaque declaration with no full definition.
+    // Pointers to these can still be passed along ABI boundaries assuming
+    // implementation details are only ever accessed on one side.
+    bool is_opaque() const {
+        return std::holds_alternative<std::monostate>(*this);
+    }
+
+    const StructInfo* get_if_struct() const {
+        return std::get_if<StructInfo>(this);
+    }
+
+    StructInfo* get_if_struct() {
+        return std::get_if<StructInfo>(this);
+    }
+
+    const SimpleTypeInfo* get_if_simple_or_struct() const {
+        auto as_struct = std::get_if<StructInfo>(this);
+        if (as_struct) {
+            return as_struct;
+        }
+        return std::get_if<SimpleTypeInfo>(this);
+    }
+};
+
+std::unordered_map<const clang::Type*, TypeInfo>
+ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, AnalysisAction::RepackedType>& types);
+
diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp
index 1a93d93720..acd822a2e0 100644
--- a/ThunkLibs/Generator/gen.cpp
+++ b/ThunkLibs/Generator/gen.cpp
@@ -1,4 +1,5 @@
 #include "analysis.h"
+#include "data_layout.h"
 #include "interface.h"
 
 #include <clang/Frontend/CompilerInstance.h>
@@ -48,6 +49,11 @@ static std::string format_function_args(const FunctionParams& params, Fn&& forma
 };
 
 void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
+    if (StrictModeEnabled(context))
+    {
+        const auto host_abi = ComputeDataLayout(context, types);
+    }
+
     static auto format_decl = [](clang::QualType type, const std::string_view& name) {
         clang::QualType innermostPointee = type;
         while (innermostPointee->isPointerType()) {

From 7f931b562390ffe2a0bee97acd9fd6e0597a62a7 Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:03 +0200
Subject: [PATCH 5/9] Thunks/gen: Add detection logic for data layout
 differences

This runs the data layout analysis pass added in the previous change twice:
Once for the host architecture and once for the guest architecture. This
allows the new DataLayoutCompareAction to query architecture differences for
each type, which can then be used to instruct code generation accordingly.

Currently, type compatibility is classified into 3 categories:
* Fully compatible (same size/alignment for the type itself and any members)
* Repackable (incompatibility can be resolved with emission of automatable
  repacking code, e.g. when struct members are located at differing offsets
  due to padding bytes)
* Incompatible
---
 ThunkLibs/Generator/data_layout.cpp | 195 ++++++++++++++++++++++++++++
 ThunkLibs/Generator/data_layout.h   |  34 +++++
 ThunkLibs/Generator/gen.cpp         |  28 ++--
 ThunkLibs/Generator/interface.h     |  33 ++++-
 ThunkLibs/Generator/main.cpp        |  34 ++++-
 ThunkLibs/GuestLibs/CMakeLists.txt  |  11 +-
 ThunkLibs/HostLibs/CMakeLists.txt   |  12 +-
 unittests/ThunkLibs/generator.cpp   |  18 ++-
 8 files changed, 342 insertions(+), 23 deletions(-)

diff --git a/ThunkLibs/Generator/data_layout.cpp b/ThunkLibs/Generator/data_layout.cpp
index ebefd20574..da05a6ee83 100644
--- a/ThunkLibs/Generator/data_layout.cpp
+++ b/ThunkLibs/Generator/data_layout.cpp
@@ -8,6 +8,19 @@
 
 constexpr bool enable_debug_output = false;
 
+// Visitor for gathering data layout information that can be passed across libclang invocations
+class AnalyzeDataLayoutAction : public AnalysisAction {
+    ABI& type_abi;
+
+    void OnAnalysisComplete(clang::ASTContext&) override;
+
+public:
+    AnalyzeDataLayoutAction(ABI&);
+};
+
+AnalyzeDataLayoutAction::AnalyzeDataLayoutAction(ABI& abi_) : type_abi(abi_) {
+}
+
 std::unordered_map<const clang::Type*, TypeInfo> ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, AnalysisAction::RepackedType>& types) {
     std::unordered_map<const clang::Type*, TypeInfo> layout;
 
@@ -121,3 +134,185 @@ std::unordered_map<const clang::Type*, TypeInfo> ComputeDataLayout(const clang::
 
     return layout;
 }
+
+ABI GetStableLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, TypeInfo>& data_layout) {
+    ABI stable_layout;
+
+    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) {
+            throw std::runtime_error("Duplicate type information: Tried to re-register type \"" + type_name + "\"");
+        }
+    }
+
+    stable_layout.pointer_size = context.getTypeSize(context.getUIntPtrType()) / 8;
+
+    return stable_layout;
+}
+
+void AnalyzeDataLayoutAction::OnAnalysisComplete(clang::ASTContext& context) {
+    if (StrictModeEnabled(context)) {
+    type_abi = GetStableLayout(context, ComputeDataLayout(context, types));
+    }
+}
+
+TypeCompatibility DataLayoutCompareAction::GetTypeCompatibility(
+        const clang::ASTContext& context,
+        const clang::Type* type,
+        const std::unordered_map<const clang::Type*, TypeInfo> host_abi,
+        std::unordered_map<const clang::Type*, TypeCompatibility>& type_compat) {
+    assert(type->isCanonicalUnqualified() || type->isBuiltinType() || type->isEnumeralType());
+
+    {
+        // Reserve a slot to be filled later. The placeholder value is used
+        // to detect infinite recursions.
+        constexpr auto placeholder_compat = TypeCompatibility { 100 };
+        auto [existing_compat_it, is_new_type] = type_compat.emplace(type, placeholder_compat);
+        if (!is_new_type) {
+            if (existing_compat_it->second == placeholder_compat) {
+                throw std::runtime_error("Found recursive reference to type \"" + clang::QualType { type, 0 }.getAsString() + "\"");
+            }
+
+            return existing_compat_it->second;
+        }
+    }
+
+    auto type_name = get_type_name(context, type);
+    auto& guest_info = guest_abi.at(type_name);
+    auto& host_info = host_abi.at(type->isBuiltinType() ? type : context.getCanonicalType(type));
+
+    const bool is_32bit = (guest_abi.pointer_size == 4);
+
+    // Assume full compatibility, then downgrade as needed
+    auto compat = TypeCompatibility::Full;
+
+    if (guest_info != host_info) {
+        // Non-matching data layout... downgrade to Repackable
+        // TODO: Even for non-structs, this only works if the types are reasonably similar (e.g. uint32_t -> uint64_t)
+        compat = TypeCompatibility::Repackable;
+    }
+
+    auto guest_struct_info = guest_info.get_if_struct();
+    if (guest_struct_info && guest_struct_info->members.size() != host_info.get_if_struct()->members.size()) {
+        // Members are missing from either the guest or host layout
+        // NOTE: If the members are merely named differently, this will be caught in the else-if below
+        compat = TypeCompatibility::None;
+    } else if (guest_struct_info) {
+        std::vector<TypeCompatibility> member_compat;
+        for (std::size_t member_idx = 0; member_idx < guest_struct_info->members.size(); ++member_idx) {
+            // Look up the corresponding member in the host struct definition.
+            // The members may be listed in a different order, so we can't
+            // directly use member_idx for this
+            auto* host_member_field = [&]() -> clang::FieldDecl* {
+                auto struct_decl = type->getAsStructureType()->getDecl();
+                auto it = std::find_if(struct_decl->field_begin(), struct_decl->field_end(), [&](auto* field) {
+                    return field->getName() == guest_struct_info->members.at(member_idx).member_name;
+                });
+                if (it == struct_decl->field_end()) {
+                    return nullptr;
+                }
+                return *it;
+            }();
+            if (!host_member_field) {
+                // No corresponding host struct member
+                // TODO: Also detect host members that are missing from the guest struct
+                member_compat.push_back(TypeCompatibility::None);
+                break;
+            }
+
+            auto host_member_type = context.getCanonicalType(host_member_field->getType().getTypePtr());
+            if (auto array_type = llvm::dyn_cast<clang::ConstantArrayType>(host_member_type)) {
+                // Compare array element type only. The array size is already considered by the layout information of the containing struct.
+                host_member_type = context.getCanonicalType(array_type->getElementType().getTypePtr());
+            }
+
+            if (host_member_type->isPointerType()) {
+                // Automatic repacking of pointers to non-compatible types is only possible if:
+                // * Pointee is fully compatible, or
+                // * 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 (host_member_pointee_type->isPointerType()) {
+                    // This is a nested pointer, e.g. void**
+
+                    if (is_32bit) {
+                        // Nested pointers can't be repacked on 32-bit
+                        member_compat.push_back(TypeCompatibility::None);
+                    } else {
+                        // Check the innermost type's compatibility on 64-bit
+                        auto pointee_pointee_type = host_member_pointee_type->getPointeeType().getTypePtr();
+                        // TODO: Not sure how to handle void here. Probably should require an annotation instead of "just working"
+                        auto pointee_pointee_compat = pointee_pointee_type->isVoidType() ? TypeCompatibility::Full : GetTypeCompatibility(context, pointee_pointee_type, host_abi, type_compat);
+                        if (pointee_pointee_compat == TypeCompatibility::Full) {
+                            member_compat.push_back(TypeCompatibility::Full);
+                        } else {
+                            member_compat.push_back(TypeCompatibility::None);
+                        }
+                    }
+                } else if (!host_member_pointee_type->isVoidType() && (host_member_pointee_type->isBuiltinType() || host_member_pointee_type->isEnumeralType())) {
+                    // TODO: What are good heuristics for this?
+                    // size_t should yield TypeCompatibility::Repackable
+                    // inconsistent types should probably default to TypeCompatibility::None
+                    // For now, just always assume compatible... (will degrade to Repackable below)
+                    member_compat.push_back(TypeCompatibility::Full);
+                } else if (!host_member_pointee_type->isVoidType() && (host_member_pointee_type->isStructureType() || types.contains(host_member_pointee_type))) {
+                    auto pointee_compat = GetTypeCompatibility(context, host_member_pointee_type, host_abi, type_compat);
+                    if (pointee_compat == TypeCompatibility::Full) {
+                        // Pointee is fully compatible, so automatic repacking only requires converting the pointers themselves
+                        member_compat.push_back(is_32bit ? TypeCompatibility::Repackable : TypeCompatibility::Full);
+                    } else {
+                        // If the pointee is incompatible (even if repackable), automatic repacking isn't possible
+                        member_compat.push_back(TypeCompatibility::None);
+                    }
+                } else if (!is_32bit && host_member_pointee_type->isVoidType()) {
+                    // TODO: Not sure how to handle void here. Probably should require an annotation instead of "just working"
+                    member_compat.push_back(TypeCompatibility::Full);
+                } else {
+                    member_compat.push_back(TypeCompatibility::None);
+                }
+                continue;
+            }
+
+            if (guest_abi.at(guest_struct_info->members[member_idx].type_name).get_if_struct()) {
+                auto host_type_info = host_abi.at(host_member_type);
+                member_compat.push_back(GetTypeCompatibility(context, host_member_type, host_abi, type_compat));
+            } else {
+                // Member was checked for size/alignment above already
+            }
+        }
+
+        if (std::all_of(member_compat.begin(), member_compat.end(), [](auto compat) { return compat == TypeCompatibility::Full; })) {
+            // TypeCompatibility::Full or ::Repackable
+        } else if (std::none_of(member_compat.begin(), member_compat.end(), [](auto compat) { return compat == TypeCompatibility::None; })) {
+            // Downgrade to Repackable
+            compat = TypeCompatibility::Repackable;
+        } else {
+            // Downgrade to None
+            compat = TypeCompatibility::None;
+        }
+    }
+
+    type_compat.at(type) = compat;
+    return compat;
+}
+
+DataLayoutCompareActionFactory::DataLayoutCompareActionFactory(const ABI& abi) : abi(abi) {
+
+}
+
+DataLayoutCompareActionFactory::~DataLayoutCompareActionFactory() = default;
+
+std::unique_ptr<clang::FrontendAction> DataLayoutCompareActionFactory::create() {
+    return std::make_unique<DataLayoutCompareAction>(abi);
+}
+
+AnalyzeDataLayoutActionFactory::AnalyzeDataLayoutActionFactory() : abi(std::make_unique<ABI>()) {
+
+}
+
+AnalyzeDataLayoutActionFactory::~AnalyzeDataLayoutActionFactory() = default;
+
+std::unique_ptr<clang::FrontendAction> AnalyzeDataLayoutActionFactory::create() {
+    return std::make_unique<AnalyzeDataLayoutAction>(*abi);
+}
diff --git a/ThunkLibs/Generator/data_layout.h b/ThunkLibs/Generator/data_layout.h
index 27d197c5da..3d58357f4f 100644
--- a/ThunkLibs/Generator/data_layout.h
+++ b/ThunkLibs/Generator/data_layout.h
@@ -77,6 +77,40 @@ struct TypeInfo : std::variant<std::monostate, SimpleTypeInfo, StructInfo> {
     }
 };
 
+struct FuncPtrInfo {
+    std::array<uint8_t, 32> sha256;
+    std::string result;
+    std::vector<std::string> args;
+};
+
+struct ABI : std::unordered_map<std::string, TypeInfo> {
+    int pointer_size; // in bytes
+};
+
 std::unordered_map<const clang::Type*, TypeInfo>
 ComputeDataLayout(const clang::ASTContext& context, const std::unordered_map<const clang::Type*, AnalysisAction::RepackedType>& types);
 
+// Convert the output of ComputeDataLayout to a format that isn't tied to a libclang session.
+// 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);
+
+enum class TypeCompatibility {
+    Full,       // Type has matching data layout across architectures
+    Repackable, // Type has different data layout but can be repacked automatically
+    None,       // Type has different data layout and cannot be repacked automatically
+};
+
+class DataLayoutCompareAction : public AnalysisAction {
+public:
+    DataLayoutCompareAction(const ABI& guest_abi) : guest_abi(guest_abi) {
+    }
+
+    TypeCompatibility GetTypeCompatibility(
+            const clang::ASTContext&,
+            const clang::Type*,
+            const std::unordered_map<const clang::Type*, TypeInfo> host_abi,
+            std::unordered_map<const clang::Type*, TypeCompatibility>& type_compat);
+
+private:
+    const ABI& guest_abi;
+};
diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp
index acd822a2e0..eb0139cd77 100644
--- a/ThunkLibs/Generator/gen.cpp
+++ b/ThunkLibs/Generator/gen.cpp
@@ -1,7 +1,7 @@
 #include "analysis.h"
 #include "data_layout.h"
+#include "diagnostics.h"
 #include "interface.h"
-
 #include <clang/Frontend/CompilerInstance.h>
 
 #include <fstream>
@@ -9,15 +9,16 @@
 #include <iostream>
 #include <string_view>
 #include <unordered_map>
+#include <variant>
 
 #include <fmt/format.h>
 #include <fmt/ostream.h>
 
 #include <openssl/sha.h>
 
-class GenerateThunkLibsAction : public AnalysisAction {
+class GenerateThunkLibsAction : public DataLayoutCompareAction {
 public:
-    GenerateThunkLibsAction(const std::string& libname, const OutputFilenames&);
+    GenerateThunkLibsAction(const std::string& libname, const OutputFilenames&, const ABI& abi);
 
 private:
     // Generate helper code for thunk libraries and write them to the output file
@@ -28,8 +29,8 @@ class GenerateThunkLibsAction : public AnalysisAction {
     const OutputFilenames& output_filenames;
 };
 
-GenerateThunkLibsAction::GenerateThunkLibsAction(const std::string& libname_, const OutputFilenames& output_filenames_)
-    : libfilename(libname_), libname(libname_), output_filenames(output_filenames_) {
+GenerateThunkLibsAction::GenerateThunkLibsAction(const std::string& libname_, const OutputFilenames& output_filenames_, const ABI& abi)
+    : DataLayoutCompareAction(abi), libfilename(libname_), libname(libname_), output_filenames(output_filenames_) {
     for (auto& c : libname) {
         if (c == '-') {
             c = '_';
@@ -49,10 +50,19 @@ static std::string format_function_args(const FunctionParams& params, Fn&& forma
 };
 
 void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
-    if (StrictModeEnabled(context))
-    {
+    ErrorReporter report_error { context };
+
+    // Compute data layout differences between host and guest
+    auto type_compat = [&]() {
+        std::unordered_map<const clang::Type*, TypeCompatibility> ret;
+        if (StrictModeEnabled(context)) {
         const auto host_abi = ComputeDataLayout(context, types);
-    }
+        for (const auto& [type, type_repack_info] : types) {
+            GetTypeCompatibility(context, type, host_abi, ret);
+        }
+        }
+        return ret;
+    }();
 
     static auto format_decl = [](clang::QualType type, const std::string_view& name) {
         clang::QualType innermostPointee = type;
@@ -377,7 +387,7 @@ bool GenerateThunkLibsActionFactory::runInvocation(
   Compiler.setInvocation(std::move(Invocation));
   Compiler.setFileManager(Files);
 
-  GenerateThunkLibsAction Action(libname, output_filenames);
+  GenerateThunkLibsAction Action(libname, output_filenames, abi);
 
   Compiler.createDiagnostics(DiagConsumer, false);
   if (!Compiler.hasDiagnostics())
diff --git a/ThunkLibs/Generator/interface.h b/ThunkLibs/Generator/interface.h
index 62883670ac..86768d34f4 100644
--- a/ThunkLibs/Generator/interface.h
+++ b/ThunkLibs/Generator/interface.h
@@ -8,10 +8,38 @@ struct OutputFilenames {
     std::string guest;
 };
 
+class AnalyzeDataLayoutActionFactory : public clang::tooling::FrontendActionFactory {
+    std::unique_ptr<struct ABI> abi;
+
+public:
+    AnalyzeDataLayoutActionFactory();
+    ~AnalyzeDataLayoutActionFactory();
+
+    std::unique_ptr<clang::FrontendAction> create() override;
+
+    const ABI& GetDataLayout() {
+        return *abi;
+    }
+
+    std::unique_ptr<ABI> TakeDataLayout() {
+        return std::move(abi);
+    }
+};
+
+class DataLayoutCompareActionFactory : public clang::tooling::FrontendActionFactory {
+    const ABI& abi;
+
+public:
+    DataLayoutCompareActionFactory(const ABI&);
+    ~DataLayoutCompareActionFactory();
+
+    std::unique_ptr<clang::FrontendAction> create() override;
+};
+
 class GenerateThunkLibsActionFactory : public clang::tooling::ToolAction {
 public:
-    GenerateThunkLibsActionFactory(std::string_view libname_, OutputFilenames output_filenames_)
-        : libname(std::move(libname_)), output_filenames(std::move(output_filenames_)) {
+    GenerateThunkLibsActionFactory(std::string_view libname_, OutputFilenames output_filenames_, const ABI& abi_)
+        : libname(std::move(libname_)), output_filenames(std::move(output_filenames_)), abi(abi_) {
     }
 
     bool runInvocation(
@@ -22,4 +50,5 @@ class GenerateThunkLibsActionFactory : public clang::tooling::ToolAction {
 private:
     std::string libname;
     OutputFilenames output_filenames;
+    const ABI& abi;
 };
diff --git a/ThunkLibs/Generator/main.cpp b/ThunkLibs/Generator/main.cpp
index ecc32bd684..726e5cbfa4 100644
--- a/ThunkLibs/Generator/main.cpp
+++ b/ThunkLibs/Generator/main.cpp
@@ -14,10 +14,10 @@ void print_usage(const char* program_name) {
     std::cerr << "Usage: " << program_name << " <filename> <libname> <gen_target> <output_filename> -- <clang_flags>\n";
 }
 
-int main(int argc, char* argv[]) {
+int main(int argc, char* const argv[]) {
     llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
 
-    if (argc < 6) {
+    if (argc < 5) {
         print_usage(argv[0]);
         return EXIT_FAILURE;
     }
@@ -32,12 +32,12 @@ int main(int argc, char* argv[]) {
     }
 
     // Process arguments before the "--" separator
-    if (argc != 5) {
+    if (argc != 5 && argc != 6) {
         print_usage(argv[0]);
         return EXIT_FAILURE;
     }
 
-    char** arg = argv + 1;
+    char* const* arg = argv + 1;
     const auto filename = *arg++;
     const std::string libname = *arg++;
     const std::string target_abi = *arg++;
@@ -62,5 +62,29 @@ int main(int argc, char* argv[]) {
         };
         Tool.appendArgumentsAdjuster(set_resource_directory);
     }
-    return Tool.run(std::make_unique<GenerateThunkLibsActionFactory>(std::move(libname), std::move(output_filenames)).get());
+
+    ClangTool GuestTool = Tool;
+
+    {
+        const bool is_32bit_guest = (argv[5] == std::string_view { "-for-32bit-guest" });
+        auto append_guest_args = [is_32bit_guest](const clang::tooling::CommandLineArguments &Args, clang::StringRef) {
+            clang::tooling::CommandLineArguments AdjustedArgs = Args;
+            const char* platform = is_32bit_guest ? "i686" : "x86_64";
+            if (is_32bit_guest) {
+                AdjustedArgs.push_back("-m32");
+                AdjustedArgs.push_back("-DIS_32BIT_THUNK");
+            }
+            AdjustedArgs.push_back(std::string { "--target=" } + platform + "-linux-unknown");
+            AdjustedArgs.push_back("-isystem");
+            AdjustedArgs.push_back(std::string { "/usr/" } + platform + "-linux-gnu/include/");
+            return AdjustedArgs;
+        };
+        GuestTool.appendArgumentsAdjuster(append_guest_args);
+    }
+
+    auto data_layout_analysis_factory = std::make_unique<AnalyzeDataLayoutActionFactory>();
+    GuestTool.run(data_layout_analysis_factory.get());
+    auto& data_layout = data_layout_analysis_factory->GetDataLayout();
+
+    return Tool.run(std::make_unique<GenerateThunkLibsActionFactory>(std::move(libname), std::move(output_filenames), data_layout).get());
 }
diff --git a/ThunkLibs/GuestLibs/CMakeLists.txt b/ThunkLibs/GuestLibs/CMakeLists.txt
index 69223e8a4d..dbe718b49d 100644
--- a/ThunkLibs/GuestLibs/CMakeLists.txt
+++ b/ThunkLibs/GuestLibs/CMakeLists.txt
@@ -52,6 +52,9 @@ function(generate NAME SOURCE_FILE)
   add_library(${NAME}-guest-deps INTERFACE)
   target_include_directories(${NAME}-guest-deps INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/../include")
   target_compile_definitions(${NAME}-guest-deps INTERFACE GUEST_THUNK_LIBRARY)
+  if (BITNESS EQUAL 32)
+    target_compile_definitions(${NAME}-guest-deps INTERFACE IS_32BIT_THUNK)
+  endif ()
   # Shorthand for the include directories added after calling this function.
   # This is not evaluated directly, hence directories added after return are still picked up
   set(prop "$<TARGET_PROPERTY:${NAME}-guest-deps,INTERFACE_INCLUDE_DIRECTORIES>")
@@ -64,16 +67,18 @@ function(generate NAME SOURCE_FILE)
   file(MAKE_DIRECTORY "${OUTFOLDER}")
 
   if (BITNESS EQUAL 32)
-    set (BITNESS_FLAGS "-m32" "--target=i686-linux-unknown" "-isystem" "/usr/i686-linux-gnu/include/")
+    set(BITNESS_FLAGS "-for-32bit-guest")
+    set(BITNESS_FLAGS2 "-m32" "--target=i686-linux-unknown" "-isystem" "/usr/i686-linux-gnu/include/")
   else()
-    set (BITNESS_FLAGS "--target=x86_64-linux-unknown" "-isystem" "/usr/x86_64-linux-gnu/include/")
+    set(BITNESS_FLAGS "")
+    set(BITNESS_FLAGS2 "--target=x86_64-linux-unknown" "-isystem" "/usr/x86_64-linux-gnu/include/")
   endif()
 
   add_custom_command(
     OUTPUT "${OUTFILE}"
     DEPENDS "${GENERATOR_EXE}"
     DEPENDS "${SOURCE_FILE}"
-    COMMAND "${GENERATOR_EXE}" "${SOURCE_FILE}" "${NAME}" "-guest" "${OUTFILE}" -- -std=c++17 ${BITNESS_FLAGS}
+    COMMAND "${GENERATOR_EXE}" "${SOURCE_FILE}" "${NAME}" "-guest" "${OUTFILE}" ${BITNESS_FLAGS} -- -std=c++20 ${BITNESS_FLAGS2}
       # Expand compile definitions to space-separated list of -D parameters
       "$<$<BOOL:${compile_prop}>:;-D$<JOIN:${compile_prop},;-D>>"
       # Expand include directories to space-separated list of -isystem parameters
diff --git a/ThunkLibs/HostLibs/CMakeLists.txt b/ThunkLibs/HostLibs/CMakeLists.txt
index bdc9d4e592..f025727509 100644
--- a/ThunkLibs/HostLibs/CMakeLists.txt
+++ b/ThunkLibs/HostLibs/CMakeLists.txt
@@ -2,7 +2,7 @@ cmake_minimum_required(VERSION 3.14)
 project(host-thunks)
 include(${FEX_PROJECT_SOURCE_DIR}/CMakeFiles/version_to_variables.cmake)
 
-set(CMAKE_CXX_STANDARD 17)
+set(CMAKE_CXX_STANDARD 20)
 set (HOSTLIBS_DATA_DIRECTORY "${CMAKE_INSTALL_PREFIX}/lib/fex-emu" CACHE PATH "global data directory")
 option(ENABLE_CLANG_THUNKS "Enable building thunks with clang" FALSE)
 
@@ -20,6 +20,9 @@ function(generate NAME SOURCE_FILE GUEST_BITNESS)
   # Interface target for the user to add include directories
   add_library(${NAME}-${GUEST_BITNESS}-deps INTERFACE)
   target_include_directories(${NAME}-${GUEST_BITNESS}-deps INTERFACE "${CMAKE_CURRENT_SOURCE_DIR}/../include")
+  if (GUEST_BITNESS EQUAL 32)
+    target_compile_definitions(${NAME}-${GUEST_BITNESS}-deps INTERFACE IS_32BIT_THUNK)
+  endif ()
   # Shorthand for the include directories added after calling this function.
   # This is not evaluated directly, hence directories added after return are still picked up
   set(prop "$<TARGET_PROPERTY:${NAME}-${GUEST_BITNESS}-deps,INTERFACE_INCLUDE_DIRECTORIES>")
@@ -40,11 +43,16 @@ function(generate NAME SOURCE_FILE GUEST_BITNESS)
 
   file(MAKE_DIRECTORY "${OUTFOLDER}")
 
+  set (BITNESS_FLAGS "")
+  if (GUEST_BITNESS EQUAL 32)
+    set (BITNESS_FLAGS "-for-32bit-guest")
+  endif()
+
   add_custom_command(
     OUTPUT "${OUTFILE}"
     DEPENDS "${SOURCE_FILE}"
     DEPENDS thunkgen
-    COMMAND thunkgen "${SOURCE_FILE}" "${NAME}" "-host" "${OUTFILE}" -- -std=c++17
+    COMMAND thunkgen "${SOURCE_FILE}" "${NAME}" "-host" "${OUTFILE}" ${BITNESS_FLAGS} -- -std=c++20
       # Expand compile definitions to space-separated list of -D parameters
       "$<$<BOOL:${compile_prop}>:;-D$<JOIN:${compile_prop},;-D>>"
       # Expand include directories to space-separated list of -isystem parameters
diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp
index 4958d957eb..ae07a82fe1 100644
--- a/unittests/ThunkLibs/generator.cpp
+++ b/unittests/ThunkLibs/generator.cpp
@@ -202,7 +202,14 @@ SourceWithAST::SourceWithAST(std::string_view input) : code(input) {
  */
 SourceWithAST Fixture::run_thunkgen_guest(std::string_view prelude, std::string_view code, bool silent) {
     const std::string full_code = std::string { prelude } + std::string { code };
-    run_tool(std::make_unique<GenerateThunkLibsActionFactory>(libname, output_filenames), full_code, silent);
+
+    // These tests don't deal with data layout differences, so just run data
+    // layout analysis with host configuration
+    auto data_layout_analysis_factory = std::make_unique<AnalyzeDataLayoutActionFactory>();
+    run_tool(*data_layout_analysis_factory, full_code, silent);
+    auto& data_layout = data_layout_analysis_factory->GetDataLayout();
+
+    run_tool(std::make_unique<GenerateThunkLibsActionFactory>(libname, output_filenames, data_layout), full_code, silent);
 
     std::string result =
         "#include <cstdint>\n"
@@ -231,7 +238,14 @@ SourceWithAST Fixture::run_thunkgen_guest(std::string_view prelude, std::string_
  */
 SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_view code, bool silent) {
     const std::string full_code = std::string { prelude } + std::string { code };
-    run_tool(std::make_unique<GenerateThunkLibsActionFactory>(libname, output_filenames), full_code, silent);
+
+    // These tests don't deal with data layout differences, so just run data
+    // layout analysis with host configuration
+    auto data_layout_analysis_factory = std::make_unique<AnalyzeDataLayoutActionFactory>();
+    run_tool(*data_layout_analysis_factory, full_code, silent);
+    auto& data_layout = data_layout_analysis_factory->GetDataLayout();
+
+    run_tool(std::make_unique<GenerateThunkLibsActionFactory>(libname, output_filenames, data_layout), full_code, silent);
 
     std::string result =
         "#include <cstdint>\n"

From 2b472cb962c06930c18c86db576c9f8676910625 Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:03 +0200
Subject: [PATCH 6/9] Thunks/gen: Enforce type compatibility for function
 parameters

---
 ThunkLibs/Generator/gen.cpp | 43 +++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/ThunkLibs/Generator/gen.cpp b/ThunkLibs/Generator/gen.cpp
index eb0139cd77..5db5142a18 100644
--- a/ThunkLibs/Generator/gen.cpp
+++ b/ThunkLibs/Generator/gen.cpp
@@ -286,6 +286,24 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
                 file << ") -> " << thunk.return_type.getAsString() << ";\n";
             }
 
+            // Check data layout compatibility of parameter types
+            // TODO: Also check non-struct/non-pointer types
+            // TODO: Also check return type
+            for (size_t param_idx = 0; StrictModeEnabled(context) && param_idx != thunk.param_types.size(); ++param_idx) {
+                const auto& param_type = thunk.param_types[param_idx];
+                if (!param_type->isPointerType() || !param_type->getPointeeType()->isStructureType()) {
+                    continue;
+                }
+                auto type = param_type->getPointeeType();
+                if (type_compat.at(context.getCanonicalType(type.getTypePtr())) == TypeCompatibility::None) {
+                    // TODO: Factor in "assume_compatible_layout" annotations here
+                    //       That annotation should cause the type to be treated as TypeCompatibility::Full
+                    {
+                        throw report_error(thunk.decl->getLocation(), "Unsupported parameter type %0").AddTaggedVal(param_type);
+                    }
+                }
+            }
+
             // Packed argument structs used in fexfn_unpack_*
             auto GeneratePackedArgs = [&](const auto &function_name, const auto &thunk) -> std::string {
                 std::string struct_name = "fexfn_packed_args_" + libname + "_" + function_name;
@@ -311,6 +329,31 @@ void GenerateThunkLibsAction::OnAnalysisComplete(clang::ASTContext& context) {
 
             file << "static void fexfn_unpack_" << libname << "_" << function_name << "(" << struct_name << "* args) {\n";
             file << (thunk.return_type->isVoidType() ? "  " : "  args->rv = ") << function_to_call << "(";
+
+            for (unsigned param_idx = 0; StrictModeEnabled(context) && param_idx != thunk.param_types.size(); ++param_idx) {
+                if (thunk.callbacks.contains(param_idx) && thunk.callbacks.at(param_idx).is_stub) {
+                    continue;
+                }
+
+                auto& param_type = thunk.param_types[param_idx];
+
+                std::optional<TypeCompatibility> pointee_compat;
+                if (param_type->isPointerType()) {
+                    // Get TypeCompatibility from existing entry, or register TypeCompatibility::None if no entry exists
+                    // TODO: Currently needs TypeCompatibility::Full workaround...
+                    pointee_compat = type_compat.emplace(context.getCanonicalType(param_type->getPointeeType().getTypePtr()), TypeCompatibility::Full).first->second;
+                }
+
+                if (!param_type->isPointerType() || pointee_compat == TypeCompatibility::Full ||
+                    param_type->getPointeeType()->isBuiltinType() /* TODO: handle size_t. Actually, properly check for data layout compatibility */) {
+                    // Fully compatible
+                } else if (pointee_compat == TypeCompatibility::Repackable) {
+                    throw report_error(thunk.decl->getLocation(), "Pointer parameter %1 of function %0 requires automatic repacking, which is not implemented yet").AddString(function_name).AddTaggedVal(param_type);
+                } else {
+                    throw report_error(thunk.decl->getLocation(), "Cannot generate unpacking function for function %0 with unannotated pointer parameter %1").AddString(function_name).AddTaggedVal(param_type);
+                }
+            }
+
             {
                 auto format_param = [&](std::size_t idx) {
                     auto cb = thunk.callbacks.find(idx);

From b04b0549a9969539a4307eed9eafcbf56148f9cf Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:03 +0200
Subject: [PATCH 7/9] unittests/ThunkLibs: Add data layout tests

---
 unittests/ThunkLibs/CMakeLists.txt |   3 +-
 unittests/ThunkLibs/abi.cpp        | 643 +++++++++++++++++++++++++++++
 unittests/ThunkLibs/common.h       |  42 +-
 3 files changed, 684 insertions(+), 4 deletions(-)
 create mode 100644 unittests/ThunkLibs/abi.cpp

diff --git a/unittests/ThunkLibs/CMakeLists.txt b/unittests/ThunkLibs/CMakeLists.txt
index ccde2918f3..8618eaa662 100644
--- a/unittests/ThunkLibs/CMakeLists.txt
+++ b/unittests/ThunkLibs/CMakeLists.txt
@@ -1,5 +1,6 @@
-add_executable(thunkgentest generator.cpp)
+add_executable(thunkgentest generator.cpp abi.cpp)
 target_link_libraries(thunkgentest PRIVATE Catch2::Catch2WithMain)
+target_link_libraries(thunkgentest PRIVATE fmt::fmt)
 target_link_libraries(thunkgentest PRIVATE thunkgenlib)
 catch_discover_tests(thunkgentest TEST_SUFFIX ".ThunkGen")
 
diff --git a/unittests/ThunkLibs/abi.cpp b/unittests/ThunkLibs/abi.cpp
new file mode 100644
index 0000000000..ca9b193c5b
--- /dev/null
+++ b/unittests/ThunkLibs/abi.cpp
@@ -0,0 +1,643 @@
+#include <clang/Frontend/CompilerInstance.h>
+#include <catch2/catch.hpp>
+
+#include <data_layout.h>
+#include <interface.h>
+#include "common.h"
+
+#include <fmt/format.h>
+
+#include <string_view>
+
+// run_tool will leak memory when the ToolAction throws an exception, so
+// disable AddressSanitizer's leak detection
+const char* __asan_default_options() {
+    return "detect_leaks=0";
+}
+
+inline std::ostream& operator<<(std::ostream& os, TypeCompatibility compat) {
+    if (compat == TypeCompatibility::Full) {
+        os << "Compatible";
+    } else if (compat == TypeCompatibility::Repackable) {
+        os << "Repackable";
+    } else if (compat == TypeCompatibility::None) {
+        os << "Incompatible";
+    } else {
+        os << "(INVALID)";
+    }
+    return os;
+}
+
+class DataLayoutCompareActionForTest;
+
+namespace {
+
+struct Fixture {
+    /**
+     * Parses annotations from the input source and generates data layout descriptions from it.
+     *
+     * Input code with common definitions (types, functions, ...) should be specified in "prelude".
+     * It will be prepended to "code" before processing and also to the generator output.
+     */
+    std::unique_ptr<DataLayoutCompareActionForTest> compute_data_layout(std::string_view prelude, std::string_view code, GuestABI);
+};
+
+}
+
+class DataLayoutCompareActionForTest : public DataLayoutCompareAction {
+    std::unordered_map<const clang::Type*, TypeCompatibility> type_compat_cache;
+
+    // Persistent reference taken to enable accessing the ASTContext after CompilerInstance::ExecuteAction returns
+    llvm::IntrusiveRefCntPtr<clang::ASTContext> ast_context;
+    std::shared_ptr<clang::Preprocessor> preprocessor;
+
+public:
+    DataLayoutCompareActionForTest(std::unique_ptr<ABI> guest_layout) : DataLayoutCompareAction(*guest_layout), guest_layout(std::move(guest_layout)) {
+    }
+
+    void ExecuteAction() override {
+        AnalysisAction::ExecuteAction();
+
+        ast_context = &getCompilerInstance().getASTContext();
+        preprocessor = getCompilerInstance().getPreprocessorPtr();
+        host_layout = ComputeDataLayout(*ast_context, types);
+    }
+
+    std::unique_ptr<ABI> guest_layout;
+    std::unordered_map<const clang::Type*, TypeInfo> host_layout;
+
+    TypeCompatibility GetTypeCompatibility(std::string_view type_name) {
+        for (const auto& [type, _] : host_layout) {
+            if (clang::QualType { type, 0 }.getAsString() == type_name) {
+                return DataLayoutCompareAction::GetTypeCompatibility(*ast_context, type, host_layout, type_compat_cache);
+            }
+        }
+
+        throw std::runtime_error("No data layout information recorded for type \"" + std::string { type_name } + "\"");
+    }
+};
+
+/**
+ * Same as clang::FrontendActionFactory but takes an external FrontendAction
+ * reference instead of constructing an internal one. Since the FrontendAction
+ * lifetime may extend past this ToolAction, state captured by the
+ * FrontendAction can be accessed after the ToolAction returns.
+ */
+class ThunkTestToolAction : public clang::tooling::ToolAction {
+public:
+    clang::FrontendAction& ScopedToolAction;
+
+public:
+    ThunkTestToolAction(clang::FrontendAction& action) : ScopedToolAction(action) {
+    }
+    ~ThunkTestToolAction() = default;
+
+    // Same as FrontendActionFactory but keeps ScopedToolAction alive when returning
+    bool runInvocation( std::shared_ptr<clang::CompilerInvocation> invocation, clang::FileManager *files,
+                        std::shared_ptr<clang::PCHContainerOperations> pch,
+                        clang::DiagnosticConsumer *diag_consumer) override {
+
+        auto diagnostics = clang::CompilerInstance::createDiagnostics(&invocation->getDiagnosticOpts(), diag_consumer, false);
+
+        clang::CompilerInstance Compiler(std::move(pch));
+        Compiler.setInvocation(std::move(invocation));
+        Compiler.setFileManager(files);
+        Compiler.createDiagnostics(diag_consumer, false);
+        if (!Compiler.hasDiagnostics())
+          return false;
+        Compiler.createSourceManager(*files);
+
+        const bool Success = Compiler.ExecuteAction(ScopedToolAction);
+
+        files->clearStatCache();
+        return Success;
+    }
+};
+
+std::unique_ptr<DataLayoutCompareActionForTest> Fixture::compute_data_layout(std::string_view prelude, std::string_view code, GuestABI guest_abi) {
+    const std::string full_code = std::string { prelude } + std::string { code };
+
+    // Compute guest data layout
+    auto data_layout_analysis_factory = std::make_unique<AnalyzeDataLayoutActionFactory>();
+    run_tool(*data_layout_analysis_factory, full_code, false, guest_abi);
+
+    // Compute host data layout
+    auto ScopedToolAction = std::make_unique<DataLayoutCompareActionForTest>(data_layout_analysis_factory->TakeDataLayout());
+    run_tool(std::make_unique<ThunkTestToolAction>(*ScopedToolAction), full_code, false, std::nullopt);
+
+    return ScopedToolAction;
+}
+
+static std::string FormatDataLayout(const std::unordered_map<const clang::Type*, TypeInfo>& layout) {
+     std::string ret;
+
+    for (const auto& [type, info] : layout) {
+        auto basic_info = info.get_if_simple_or_struct();
+        if (!basic_info) {
+            continue;
+        }
+
+        ret += fmt::format("  Host entry {}: {} ({})\n", clang::QualType { type, 0 }.getAsString().c_str(), basic_info->size_bits / 8, basic_info->alignment_bits / 8);
+
+        if (auto struct_info = info.get_if_struct()) {
+            for (const auto& member : struct_info->members) {
+                ret += fmt::format("    Offset {}-{}: {} {}{}\n", member.offset_bits / 8, (member.offset_bits + member.size_bits - 1) / 8, member.type_name.c_str(), member.member_name.c_str(), member.array_size ? fmt::format("[{}]", member.array_size.value()).c_str() : "");
+            }
+        }
+    }
+
+    return ret;
+}
+
+TEST_CASE_METHOD(Fixture, "DataLayout") {
+    auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64);
+    INFO(guest_abi);
+
+    SECTION("Trivial") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n",
+            "struct A { int a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        REQUIRE(action->guest_layout->contains("A"));
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+    }
+
+    SECTION("Builtin types") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n",
+            "struct A { char a; short b; int c; float d; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        REQUIRE(action->guest_layout->contains("A"));
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("char") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("short") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("int") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("float") == TypeCompatibility::Full);
+    }
+
+    SECTION("Padding after int16_t") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int16_t a; int32_t b; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+    }
+
+    SECTION("Array of int16_t") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int16_t a[64]; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+    }
+
+    const auto compat_full64_repackable32 = (guest_abi == GuestABI::X86_32 ? TypeCompatibility::Repackable : TypeCompatibility::Full);
+
+    SECTION("Type with platform-dependent size (size_t)") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdlib>\n",
+            "struct A { size_t a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("int64_t has stricter alignment requirements on 64-bit platforms") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int64_t a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->alignment_bits == (guest_abi == GuestABI::X86_32 ? 32 : 64));
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("Array of int64_t") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int64_t a[64]; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("int64_t with explicit alignment specification") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct alignas(8) A { int64_t a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->alignment_bits == 64);
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+    }
+
+    SECTION("int64_t alignment requirements propagate to parent struct") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int32_t a; int32_t b; int64_t c; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->alignment_bits == (guest_abi == GuestABI::X86_32 ? 32 : 64));
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("Padding before int64_t member") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int32_t a; int64_t b; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->members[1].offset_bits == (guest_abi == GuestABI::X86_32 ? 32 : 64));
+
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("Padding at end of struct due to int64_t alignment (like VkMemoryHeap)") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { int64_t a; int32_t b; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->size_bits == (guest_abi == GuestABI::X86_32 ? 96 : 128));
+
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("Different struct definition between guest and host; different member order") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct A { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct A { int32_t b; int32_t a; };\n"
+            "#endif\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->members.at(0).member_name == "b");
+        CHECK(action->guest_layout->at("A").get_if_struct()->members.at(1).member_name == "a");
+
+        REQUIRE(!action->host_layout.empty());
+        CHECK(action->host_layout.begin()->second.get_if_struct()->members.at(0).member_name == "a");
+        CHECK(action->host_layout.begin()->second.get_if_struct()->members.at(1).member_name == "b");
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Repackable);
+    }
+
+    SECTION("Different struct definition between guest and host; different member size") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct A { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct A { int32_t a; int64_t b; };\n"
+            "#endif\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->members.at(0).size_bits == 32);
+        CHECK(action->guest_layout->at("A").get_if_struct()->members.at(1).size_bits == 64);
+
+        REQUIRE(!action->host_layout.empty());
+        CHECK(action->host_layout.begin()->second.get_if_struct()->members.at(0).size_bits == 32);
+        CHECK(action->host_layout.begin()->second.get_if_struct()->members.at(1).size_bits == 32);
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Repackable);
+    }
+
+    SECTION("Different struct definition between guest and host; completely different members") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct A { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct A { int32_t c; int32_t d; };\n"
+            "#endif\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::None);
+    }
+
+    SECTION("Different struct definition between guest and host; member missing from guest") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct A { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct A { int32_t a; };\n"
+            "#endif\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::None);
+    }
+
+    SECTION("Different struct definition between guest and host; member missing from host") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct A { int32_t a; };\n"
+            "#else\n"
+            "struct A { int32_t a; int32_t b; };\n"
+            "#endif\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::None);
+    }
+
+    SECTION("Nesting structs of consistent data layout") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct C { int32_t a; int16_t b; };\n"
+            "struct B { C a; int16_t b; };\n"
+            "struct A { int32_t a; B b; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        REQUIRE(action->guest_layout->contains("B"));
+        REQUIRE(action->guest_layout->contains("C"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->members.at(0).size_bits == 32);
+
+        CHECK(action->GetTypeCompatibility("struct C") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("struct B") == TypeCompatibility::Full);
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Full);
+    }
+
+    SECTION("Nesting repackable structs by embedding") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct C { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct C { int32_t b; int32_t a; };\n"
+            "#endif\n"
+            "struct B { C a; int16_t b; };\n"
+            "struct A { int32_t a; B b; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        REQUIRE(action->guest_layout->contains("B"));
+        REQUIRE(action->guest_layout->contains("C"));
+        CHECK(action->guest_layout->at("A").get_if_struct()->size_bits == 128);
+        CHECK(action->guest_layout->at("A").get_if_struct()->alignment_bits == 32);
+
+        CHECK(action->GetTypeCompatibility("struct C") == TypeCompatibility::Repackable);
+        CHECK(action->GetTypeCompatibility("struct B") == TypeCompatibility::Repackable);
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::Repackable);
+    }
+
+    SECTION("Embedded union type (like VkRenderingAttachmentInfo)") {
+        SECTION("without annotation") {
+            CHECK_THROWS_WITH(compute_data_layout(
+                "#include <thunks_common.h>\n"
+                "#include <cstdint>\n",
+                "union B { int32_t a; uint32_t b; };\n"
+                "struct A { B a; };\n"
+                "template<> struct fex_gen_type<A> {};\n", guest_abi),
+                Catch::Contains("unannotated member") && Catch::Contains("union type"));
+        }
+    }
+}
+
+TEST_CASE_METHOD(Fixture, "DataLayoutPointers") {
+    auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64);
+    INFO(guest_abi);
+
+    const auto compat_full64_repackable32 = (guest_abi == GuestABI::X86_32 ? TypeCompatibility::Repackable : TypeCompatibility::Full);
+
+    SECTION("Pointer to data with consistent layout") {
+        std::string type = GENERATE("char", "short", "int", "float", "struct B { int a; }");
+        INFO(type);
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { " + type + "* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        // The pointer itself needs repacking on 32-bit. On 64-bit, no repacking is needed at all.
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+        if (!type.starts_with("struct B")) {
+            CHECK(action->GetTypeCompatibility(type) == TypeCompatibility::Full);
+        }
+    }
+
+    SECTION("Pointer to struct with consistent layout") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct B { int32_t a; };\n"
+            "struct A { B* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("B"));
+        CHECK(action->GetTypeCompatibility("struct B") == TypeCompatibility::Full);
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+    }
+
+    SECTION("Unannotated pointer to incomplete type") {
+        CHECK_THROWS_WITH(compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct B;\n"
+            "struct A { B* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi),
+            Catch::Contains("incomplete type"));
+    }
+
+    SECTION("Unannotated pointer to repackable type") {
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "#ifdef HOST\n"
+            "struct B { int32_t a; int32_t b; };\n"
+            "#else\n"
+            "struct B { int32_t a; int64_t b; };\n"
+            "#endif\n"
+            "struct A { B* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::None);
+    }
+
+    SECTION("Nesting repackable structs by pointers") {
+        SECTION("Innermost struct is compatible") {
+            auto action = compute_data_layout(
+                "#include <thunks_common.h>\n"
+                "#include <cstdint>\n",
+                "struct C { int32_t a; int32_t b; };\n"
+                "struct B { C* a; int16_t b; };\n"
+                "struct A { int32_t a; B b; };\n"
+                "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+            INFO(FormatDataLayout(action->host_layout));
+
+            REQUIRE(action->guest_layout->contains("A"));
+            REQUIRE(action->guest_layout->contains("B"));
+            REQUIRE(action->guest_layout->contains("C"));
+
+            // 64-bit is fully compatible, but 32-bit needs to zero-extend the pointer itself
+            CHECK(action->GetTypeCompatibility("struct C") == TypeCompatibility::Full);
+            CHECK(action->GetTypeCompatibility("struct B") == compat_full64_repackable32);
+            CHECK(action->GetTypeCompatibility("struct A") == compat_full64_repackable32);
+        }
+
+        SECTION("Innermost struct is incompatible") {
+            auto action = compute_data_layout(
+                "#include <thunks_common.h>\n"
+                "#include <cstdint>\n",
+                "#ifdef HOST\n"
+                "struct C { int32_t a; int32_t b; };\n"
+                "#else\n"
+                "struct C { int32_t b; int32_t a; };\n"
+                "#endif\n"
+                "struct B { C* a; int16_t b; };\n"
+                "struct A { int32_t a; B b; };\n"
+                "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+            INFO(FormatDataLayout(action->host_layout));
+
+            REQUIRE(action->guest_layout->contains("A"));
+            REQUIRE(action->guest_layout->contains("B"));
+            REQUIRE(action->guest_layout->contains("C"));
+
+            CHECK(action->GetTypeCompatibility("struct C") == TypeCompatibility::Repackable);
+            CHECK(action->GetTypeCompatibility("struct B") == TypeCompatibility::None);
+            CHECK(action->GetTypeCompatibility("struct A") == TypeCompatibility::None);
+        }
+    }
+
+    SECTION("Unannotated pointer to union type") {
+        CHECK_THROWS_WITH(compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "union B { int32_t a; uint32_t b; };\n"
+            "struct A { B* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi),
+            Catch::Contains("unannotated member") && Catch::Contains("union type"));
+    }
+
+    SECTION("Self-referencing struct (like VkBaseOutStructure)") {
+        // Without annotation
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { A* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK_THROWS_WITH(action->GetTypeCompatibility("struct A"), Catch::Contains("recursive reference"));
+    }
+
+    SECTION("Circularly referencing structs") {
+        // Without annotation
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct B;\n"
+            "struct A { B* a; };\n"
+            "struct B { A* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        REQUIRE(action->guest_layout->contains("B"));
+        CHECK_THROWS_WITH(action->GetTypeCompatibility("struct A"), Catch::Contains("recursive reference"));
+        CHECK_THROWS_WITH(action->GetTypeCompatibility("struct B"), Catch::Contains("recursive reference"));
+    }
+
+    SECTION("Pointers to void") {
+        // Without annotation
+        auto action = compute_data_layout(
+            "#include <thunks_common.h>\n"
+            "#include <cstdint>\n",
+            "struct A { void* a; };\n"
+            "template<> struct fex_gen_type<A> {};\n", guest_abi);
+
+        INFO(FormatDataLayout(action->host_layout));
+
+        REQUIRE(action->guest_layout->contains("A"));
+        CHECK(action->GetTypeCompatibility("struct A") == (guest_abi == GuestABI::X86_32 ? TypeCompatibility::None : TypeCompatibility::Full));
+    }
+
+    // TODO: Double pointers to compatible data: struct B { int a ; }; struct A { B** b; };
+}
diff --git a/unittests/ThunkLibs/common.h b/unittests/ThunkLibs/common.h
index 47c83bd9bd..d31b104104 100644
--- a/unittests/ThunkLibs/common.h
+++ b/unittests/ThunkLibs/common.h
@@ -42,15 +42,42 @@ class TestDiagnosticConsumer : public clang::TextDiagnosticPrinter {
     }
 };
 
+enum class GuestABI {
+    X86_32,
+    X86_64,
+};
+
+inline std::ostream& operator<<(std::ostream& os, GuestABI abi) {
+    if (abi == GuestABI::X86_32) {
+        os << "X86_32";
+    } else if (abi == GuestABI::X86_64) {
+        os << "X86_64";
+    }
+    return os;
+}
+
 /**
  * Run the given ToolAction on the input code.
  *
  * The "silent" parameter is used to suppress non-fatal diagnostics in tests that expect failure
  */
-inline void run_tool(clang::tooling::ToolAction& action, std::string_view code, bool silent = false) {
+inline void run_tool(clang::tooling::ToolAction& action, std::string_view code, bool silent = false, std::optional<GuestABI> guest_abi = std::nullopt) {
     const char* memory_filename = "gen_input.cpp";
     auto adjuster = clang::tooling::getClangStripDependencyFileAdjuster();
     std::vector<std::string> args = { "clang-tool", "-fsyntax-only", "-std=c++17", "-Werror", "-I.", memory_filename };
+    if (guest_abi == GuestABI::X86_64) {
+        args.push_back("-target");
+        args.push_back("x86_64-linux-gnu");
+        args.push_back("-isystem");
+        args.push_back("/usr/x86_64-linux-gnu/include/");
+    } else if (guest_abi == GuestABI::X86_32) {
+        args.push_back("-target");
+        args.push_back("i686-linux-gnu");
+        args.push_back("-isystem");
+        args.push_back("/usr/i686-linux-gnu/include/");
+    } else {
+        args.push_back("-DHOST");
+    }
 
     // Corresponds to the content of GeneratorInterface.h
     const char* common_header_code = R"(namespace fexgen {
@@ -60,6 +87,12 @@ struct callback_annotation_base { bool prevent_multiple; };
 struct callback_stub : callback_annotation_base {};
 struct callback_guest : callback_annotation_base {};
 } // namespace fexgen
+
+template<typename>
+struct fex_gen_type;
+template<auto>
+struct fex_gen_config;
+
 )";
 
     llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> overlay_fs(new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
@@ -73,6 +106,9 @@ struct callback_guest : callback_annotation_base {};
 
     TestDiagnosticConsumer consumer(silent);
     invocation.setDiagnosticConsumer(&consumer);
+
+    // Process the actual ToolAction.
+    // NOTE: If the ToolAction throws an exception, clang will leak memory here.
     invocation.run();
 
     if (auto error = consumer.GetFirstError()) {
@@ -80,6 +116,6 @@ struct callback_guest : callback_annotation_base {};
     }
 }
 
-inline void run_tool(std::unique_ptr<clang::tooling::ToolAction> action, std::string_view code, bool silent = false) {
-    return run_tool(*action, code, silent);
+inline void run_tool(std::unique_ptr<clang::tooling::ToolAction> action, std::string_view code, bool silent = false, std::optional<GuestABI> guest_abi = std::nullopt) {
+    return run_tool(*action, code, silent, guest_abi);
 }

From 2d9e816ff5632d85fac111d2989eac54b9b522dc Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Mon, 25 Sep 2023 23:10:03 +0200
Subject: [PATCH 8/9] unittests/ThunkLibs: Add various tests for structs
 repacking and for void parameters

---
 unittests/ThunkLibs/generator.cpp | 94 +++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 6 deletions(-)

diff --git a/unittests/ThunkLibs/generator.cpp b/unittests/ThunkLibs/generator.cpp
index ae07a82fe1..7144062ae3 100644
--- a/unittests/ThunkLibs/generator.cpp
+++ b/unittests/ThunkLibs/generator.cpp
@@ -73,7 +73,7 @@ struct Fixture {
      * It will be prepended to "code" before processing and also to the generator output.
      */
     SourceWithAST run_thunkgen_guest(std::string_view prelude, std::string_view code, bool silent = false);
-    SourceWithAST run_thunkgen_host(std::string_view prelude, std::string_view code, bool silent = false);
+    SourceWithAST run_thunkgen_host(std::string_view prelude, std::string_view code, GuestABI = GuestABI::X86_64, bool silent = false);
     GenOutput run_thunkgen(std::string_view prelude, std::string_view code, bool silent = false);
 
     const std::string libname = "libtest";
@@ -236,13 +236,13 @@ SourceWithAST Fixture::run_thunkgen_guest(std::string_view prelude, std::string_
 /**
  * Generates host thunk library code from the given input
  */
-SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_view code, bool silent) {
+SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_view code, GuestABI guest_abi, bool silent) {
     const std::string full_code = std::string { prelude } + std::string { code };
 
     // These tests don't deal with data layout differences, so just run data
     // layout analysis with host configuration
     auto data_layout_analysis_factory = std::make_unique<AnalyzeDataLayoutActionFactory>();
-    run_tool(*data_layout_analysis_factory, full_code, silent);
+    run_tool(*data_layout_analysis_factory, full_code, silent, guest_abi);
     auto& data_layout = data_layout_analysis_factory->GetDataLayout();
 
     run_tool(std::make_unique<GenerateThunkLibsActionFactory>(libname, output_filenames, data_layout), full_code, silent);
@@ -282,17 +282,27 @@ SourceWithAST Fixture::run_thunkgen_host(std::string_view prelude, std::string_v
     auto& filename = output_filenames.host;
     {
         std::ifstream file(filename);
-        const auto current_size = result.size();
+        const auto prelude_size = result.size();
         const auto new_data_size = std::filesystem::file_size(filename);
         result.resize(result.size() + new_data_size);
-        file.read(result.data() + current_size, result.size());
+        file.read(result.data() + prelude_size, result.size());
+
+        // Force all functions to be non-static, since having to define them
+        // would add a lot of noise to simple tests.
+        while (true) {
+            auto pos = result.find("static ", prelude_size);
+            if (pos == std::string::npos) {
+                break;
+            }
+            result.replace(pos, 6, "      "); // Replace "static" with 6 spaces (avoiding reallocation)
+        }
     }
     return SourceWithAST { std::string { prelude } + result };
 }
 
 Fixture::GenOutput Fixture::run_thunkgen(std::string_view prelude, std::string_view code, bool silent) {
     return { run_thunkgen_guest(prelude, code, silent),
-             run_thunkgen_host(prelude, code, silent) };
+             run_thunkgen_host(prelude, code, GuestABI::X86_64, silent) };
 }
 
 TEST_CASE_METHOD(Fixture, "Trivial") {
@@ -525,3 +535,75 @@ TEST_CASE_METHOD(Fixture, "VariadicFunctionsWithoutAnnotation") {
         "template<auto> struct fex_gen_config {};\n"
         "template<> struct fex_gen_config<func> {};\n", true));
 }
+
+TEST_CASE_METHOD(Fixture, "StructRepacking") {
+    auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64);
+    INFO(guest_abi);
+
+    // All tests use the same function, but the prelude defining its parameter type "A" varies
+    const std::string code =
+        "#include <thunks_common.h>\n"
+        "void func(A*);\n"
+        "template<auto> struct fex_gen_config {};\n"
+        "template<> struct fex_gen_config<func> : fexgen::custom_host_impl {};\n";
+
+    SECTION("Pointer to struct with consistent data layout") {
+        CHECK_NOTHROW(run_thunkgen_host("struct A { int a; };\n", code, guest_abi));
+    }
+
+    SECTION("Pointer to struct with unannotated pointer member with inconsistent data layout") {
+        const auto prelude =
+            "#ifdef HOST\n"
+            "struct B { int a; };\n"
+            "#else\n"
+            "struct B { int b; };\n"
+            "#endif\n"
+            "struct A { B* a; };\n";
+
+        SECTION("Parameter unannotated") {
+            CHECK_THROWS(run_thunkgen_host(prelude, code, guest_abi, true));
+        }
+    }
+
+    SECTION("Pointer to struct with pointer member of opaque type") {
+        const auto prelude =
+            "struct B;\n"
+            "struct A { B* a; };\n";
+
+        // Unannotated
+        REQUIRE_THROWS_WITH(run_thunkgen_host(prelude, code, guest_abi), Catch::Contains("incomplete type"));
+    }
+}
+
+TEST_CASE_METHOD(Fixture, "VoidPointerParameter") {
+    auto guest_abi = GENERATE(GuestABI::X86_32, GuestABI::X86_64);
+    INFO(guest_abi);
+
+    SECTION("Unannotated") {
+        const char* code =
+            "#include <thunks_common.h>\n"
+            "void func(void*);\n"
+            "template<> struct fex_gen_config<func> {};\n";
+        if (guest_abi == GuestABI::X86_32) {
+            // TODO: Currently not considered an error
+//            CHECK_THROWS_WITH(run_thunkgen_host("", code, guest_abi, true), Catch::Contains("unsupported parameter type", Catch::CaseSensitive::No));
+        } else {
+            // Pointee data is assumed to be compatible on 64-bit
+            CHECK_NOTHROW(run_thunkgen_host("", code, guest_abi));
+        }
+    }
+
+    SECTION("Unannotated in struct") {
+        const char* prelude =
+            "struct A { void* a; };\n";
+        const char* code =
+            "#include <thunks_common.h>\n"
+            "void func(A*);\n"
+            "template<> struct fex_gen_config<func> {};\n";
+        if (guest_abi == GuestABI::X86_32) {
+            CHECK_THROWS_WITH(run_thunkgen_host(prelude, code, guest_abi, true), Catch::Contains("unsupported parameter type", Catch::CaseSensitive::No));
+        } else {
+            CHECK_NOTHROW(run_thunkgen_host(prelude, code, guest_abi));
+        }
+    }
+}

From fe681ab3354aafc6c841ea7cfd4db103729b5c5c Mon Sep 17 00:00:00 2001
From: Tony Wasserka <neobrain@users.noreply.github.com>
Date: Thu, 28 Sep 2023 22:39:14 +0200
Subject: [PATCH 9/9] unittests/ThunkLibs: Specify clang resource directory
 when compiling test code

---
 ThunkLibs/Generator/CMakeLists.txt | 16 ++++++++--------
 unittests/ThunkLibs/common.h       |  4 ++++
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/ThunkLibs/Generator/CMakeLists.txt b/ThunkLibs/Generator/CMakeLists.txt
index f628db6874..8214c558f5 100644
--- a/ThunkLibs/Generator/CMakeLists.txt
+++ b/ThunkLibs/Generator/CMakeLists.txt
@@ -1,13 +1,6 @@
 find_package(Clang REQUIRED CONFIG)
 find_package(OpenSSL REQUIRED COMPONENTS Crypto)
 
-add_library(thunkgenlib analysis.cpp data_layout.cpp gen.cpp)
-target_include_directories(thunkgenlib INTERFACE ${CMAKE_CURRENT_SOURCE_DIR})
-target_include_directories(thunkgenlib SYSTEM PUBLIC ${CLANG_INCLUDE_DIRS})
-target_link_libraries(thunkgenlib PUBLIC clang-cpp LLVM)
-target_link_libraries(thunkgenlib PRIVATE OpenSSL::Crypto)
-target_link_libraries(thunkgenlib PRIVATE fmt::fmt)
-
 # Query clang's global resource directory for system include directories
 if (NOT CLANG_RESOURCE_DIR)
   find_program(CLANG_EXEC_PATH clang REQUIRED)
@@ -18,6 +11,13 @@ if (NOT CLANG_RESOURCE_DIR)
     OUTPUT_STRIP_TRAILING_WHITESPACE)
 endif()
 
+add_library(thunkgenlib analysis.cpp data_layout.cpp gen.cpp)
+target_include_directories(thunkgenlib INTERFACE ${CMAKE_CURRENT_SOURCE_DIR})
+target_include_directories(thunkgenlib SYSTEM PUBLIC ${CLANG_INCLUDE_DIRS})
+target_link_libraries(thunkgenlib PUBLIC clang-cpp LLVM)
+target_link_libraries(thunkgenlib PRIVATE OpenSSL::Crypto)
+target_link_libraries(thunkgenlib PRIVATE fmt::fmt)
+target_compile_definitions(thunkgenlib INTERFACE -DCLANG_RESOURCE_DIR="${CLANG_RESOURCE_DIR}")
+
 add_executable(thunkgen main.cpp)
 target_link_libraries(thunkgen PRIVATE thunkgenlib)
-target_compile_definitions(thunkgen PRIVATE -DCLANG_RESOURCE_DIR="${CLANG_RESOURCE_DIR}")
diff --git a/unittests/ThunkLibs/common.h b/unittests/ThunkLibs/common.h
index d31b104104..6fe13022eb 100644
--- a/unittests/ThunkLibs/common.h
+++ b/unittests/ThunkLibs/common.h
@@ -65,6 +65,10 @@ inline void run_tool(clang::tooling::ToolAction& action, std::string_view code,
     const char* memory_filename = "gen_input.cpp";
     auto adjuster = clang::tooling::getClangStripDependencyFileAdjuster();
     std::vector<std::string> args = { "clang-tool", "-fsyntax-only", "-std=c++17", "-Werror", "-I.", memory_filename };
+    if (CLANG_RESOURCE_DIR[0] != 0) {
+        args.push_back("-resource-dir");
+        args.push_back(CLANG_RESOURCE_DIR);
+    }
     if (guest_abi == GuestABI::X86_64) {
         args.push_back("-target");
         args.push_back("x86_64-linux-gnu");