Merge pull request #1863 from neobrain/feature_thunk_xfree

Thunks/X11: Distinguish between host and guest pointers in XFree

External/FEXCore/Source/Interface/HLE/Thunks/Thunks.cpp

@@ -28,6 +28,8 @@ $end_info$
 #include <string>
 #include <utility>
 
+#include "jemalloc/jemalloc.h"
+
 struct LoadlibArgs {
     const char *Name;
 };
@@ -108,6 +110,11 @@ namespace FEXCore {
                 { 0xee, 0x57, 0xba, 0x0c, 0x5f, 0x6e, 0xef, 0x2a, 0x8c, 0xb5, 0x19, 0x81, 0xc9, 0x23, 0xe6, 0x51, 0xae, 0x65, 0x02, 0x8f, 0x2b, 0x5d, 0x59, 0x90, 0x6a, 0x7e, 0xe2, 0xe7, 0x1c, 0x33, 0x8a, 0xff },
                 &IsLibLoaded
             },
+            {
+                // sha256(fex:is_host_heap_allocation)
+                { 0xf5, 0x77, 0x68, 0x43, 0xbb, 0x6b, 0x28, 0x18, 0x40, 0xb0, 0xdb, 0x8a, 0x66, 0xfb, 0x0e, 0x2d, 0x98, 0xc2, 0xad, 0xe2, 0x5a, 0x18, 0x5a, 0x37, 0x2e, 0x13, 0xc9, 0xe7, 0xb9, 0x8c, 0xa9, 0x3e },
+                &IsHostHeapAllocation
+            },
             {
                 // sha256(fex:link_address_to_function)
                 { 0xe6, 0xa8, 0xec, 0x1c, 0x7b, 0x74, 0x35, 0x27, 0xe9, 0x4f, 0x5b, 0x6e, 0x2d, 0xc9, 0xa0, 0x27, 0xd6, 0x1f, 0x2b, 0x87, 0x8f, 0x2d, 0x35, 0x50, 0xea, 0x16, 0xb8, 0xc4, 0x5e, 0x42, 0xfd, 0x77 },
@@ -288,6 +295,21 @@ namespace FEXCore {
           ThunkHandler->GuestcallToHostTrampoline[gci] = args->rv;
         }
 
+        /**
+         * Checks if the given pointer is allocated on the host heap.
+         *
+         * This is useful for thunking APIs that need to work with both guest
+         * and host heap pointers.
+         */
+        static void IsHostHeapAllocation(void* ArgsRV) {
+            struct ArgsRV_t {
+                void* ptr;
+                bool rv;
+            } *args = reinterpret_cast<ArgsRV_t*>(ArgsRV);
+
+            args->rv = je_is_known_allocation(args->ptr);
+        }
+
         static void LoadLib(void *ArgsV) {
             auto CTX = Thread->CTX;
 

ThunkLibs/include/common/Guest.h

@@ -33,6 +33,7 @@ struct LoadlibArgs {
 
 MAKE_THUNK(fex, loadlib, "0x27, 0x7e, 0xb7, 0x69, 0x5b, 0xe9, 0xab, 0x12, 0x6e, 0xf7, 0x85, 0x9d, 0x4b, 0xc9, 0xa2, 0x44, 0x46, 0xcf, 0xbd, 0xb5, 0x87, 0x43, 0xef, 0x28, 0xa2, 0x65, 0xba, 0xfc, 0x89, 0x0f, 0x77, 0x80")
 MAKE_THUNK(fex, is_lib_loaded, "0xee, 0x57, 0xba, 0x0c, 0x5f, 0x6e, 0xef, 0x2a, 0x8c, 0xb5, 0x19, 0x81, 0xc9, 0x23, 0xe6, 0x51, 0xae, 0x65, 0x02, 0x8f, 0x2b, 0x5d, 0x59, 0x90, 0x6a, 0x7e, 0xe2, 0xe7, 0x1c, 0x33, 0x8a, 0xff")
+MAKE_THUNK(fex, is_host_heap_allocation, "0xf5, 0x77, 0x68, 0x43, 0xbb, 0x6b, 0x28, 0x18, 0x40, 0xb0, 0xdb, 0x8a, 0x66, 0xfb, 0x0e, 0x2d, 0x98, 0xc2, 0xad, 0xe2, 0x5a, 0x18, 0x5a, 0x37, 0x2e, 0x13, 0xc9, 0xe7, 0xb9, 0x8c, 0xa9, 0x3e")
 MAKE_THUNK(fex, link_address_to_function, "0xe6, 0xa8, 0xec, 0x1c, 0x7b, 0x74, 0x35, 0x27, 0xe9, 0x4f, 0x5b, 0x6e, 0x2d, 0xc9, 0xa0, 0x27, 0xd6, 0x1f, 0x2b, 0x87, 0x8f, 0x2d, 0x35, 0x50, 0xea, 0x16, 0xb8, 0xc4, 0x5e, 0x42, 0xfd, 0x77")
 MAKE_THUNK(fex, make_host_trampoline_for_guest_function, "0x1e, 0x51, 0x6b, 0x07, 0x39, 0xeb, 0x50, 0x59, 0xb3, 0xf3, 0x4f, 0xca, 0xdd, 0x58, 0x37, 0xe9, 0xf0, 0x30, 0xe5, 0x89, 0x81, 0xc7, 0x14, 0xfb, 0x24, 0xf9, 0xba, 0xe7, 0x0e, 0x00, 0x1e, 0x86")
 
@@ -201,3 +202,13 @@ inline Target *MakeHostTrampolineForGuestFunction(uint8_t HostPacker[32], void (
 
   return (Target *)argsrv.rv;
 }
+
+inline bool IsHostHeapAllocation(void* ptr) {
+    struct {
+        void* ptr;
+        bool rv;
+    } args = { ptr, {} };
+
+    fexthunks_fex_is_host_heap_allocation(&args);
+    return args.rv;
+}

ThunkLibs/libX11/libX11_Guest.cpp

@@ -9,6 +9,11 @@ $end_info$
 #include <X11/Xutil.h>
 #include <X11/Xresource.h>
 
+// Include Xlibint.h and undefine some of its macros that clash with the standard library
+#include <X11/Xlibint.h>
+#undef min
+#undef max
+
 #include <cstdint>
 #include <stdio.h>
 #include <cstring>
@@ -68,16 +73,44 @@ extern "C" {
         return rv;
     }
 
-    static void LockMutexFunction() {
+    static void LockMutexFunction(LockInfoPtr) {
       fprintf(stderr, "libX11: LockMutex\n");
     }
 
-    static void UnlockMutexFunction() {
+    static void UnlockMutexFunction(LockInfoPtr) {
       fprintf(stderr, "libX11: LockMutex\n");
     }
 
-  void (*_XLockMutex_fn)() = LockMutexFunction;
-  void (*_XUnlockMutex_fn)() = UnlockMutexFunction;
+  int XFree(void* ptr) {
+    // This function must be able to handle both guest heap pointers *and* host heap pointers,
+    // so it only forwards to the native host library for the latter.
+    //
+    // This is because Xlibint users allocate memory using internal macros aliasing to libc's
+    // malloc but then they free using the function XFree. For libX11, this is not a problem since
+    // the allocation happens in a thunked API function (and hence on the host heap), but if a
+    // function from an unthunked library accesses Xlibint, it will allocate on the guest heap.
+    //
+    // One notable example where this was encountered is XF86VidModeGetAllModeLines.
+
+    if (!ptr || IsHostHeapAllocation(ptr)) {
+      return fexfn_pack_XFree(ptr);
+    } else {
+      free(ptr);
+      return 1;
+    }
+  }
+
+  void XFreeEventData(Display* display, XGenericEventCookie* cookie) {
+    // Has the same heap-mismatch issue as XFree, so we have to reimplement it manually
+    if (_XIsEventCookie(display, (XEvent*)cookie) && cookie->data) {
+      XFree(cookie->data);
+      cookie->data = nullptr;
+      cookie->cookie = 0;
+    }
+  }
+
+  void (*_XLockMutex_fn)(LockInfoPtr) = LockMutexFunction;
+  void (*_XUnlockMutex_fn)(LockInfoPtr) = UnlockMutexFunction;
   typedef struct _LockInfoRec *LockInfoPtr;
   LockInfoPtr _Xglobal_lock = (LockInfoPtr)0x4142434445464748ULL;
 }

ThunkLibs/libX11/libX11_interface.cpp

@@ -465,7 +465,7 @@ template<> struct fex_gen_config<XFreeExtensionList> {};
 template<> struct fex_gen_config<XFreeFont> {};
 template<> struct fex_gen_config<XFreeGC> {};
 template<> struct fex_gen_config<XFreePixmap> {};
-template<> struct fex_gen_config<XFree> {};
+template<> struct fex_gen_config<XFree> : fexgen::custom_guest_entrypoint {};
 template<> struct fex_gen_config<XGetErrorDatabaseText> {};
 template<> struct fex_gen_config<XGetErrorText> {};
 template<> struct fex_gen_config<XGetEventData> {};
@@ -502,7 +502,7 @@ template<> struct fex_gen_config<XWindowEvent> {};
 template<> struct fex_gen_config<XCreateBitmapFromData> {};
 template<> struct fex_gen_config<XCreatePixmap> {};
 template<> struct fex_gen_config<XDestroyIC> {};
-template<> struct fex_gen_config<XFreeEventData> {};
+template<> struct fex_gen_config<XFreeEventData> : fexgen::custom_guest_entrypoint {};
 template<> struct fex_gen_config<XLockDisplay> {};
 template<> struct fex_gen_config<XSetICFocus> {};
 template<> struct fex_gen_config<XSetWMNormalHints> {};