Merge pull request #1382 from neobrain/refactor_thunk_cleanup

Various thunk library cleanups

ThunkLibs/Generators/ThunkHelpers.py

@@ -249,7 +249,7 @@ def GenerateFunctionLdrPtr(lib, function):
     print("static fexldr_type_" + lib["name"] + "_" + function["name"] + " *fexldr_ptr_" + lib["name"] + "_" + function["name"] + ";")
 
 def GenerateFunctionLdrPtrPair(lib, function):
-    print("PAIR({0}, &fexldr_ptr_{1}_{0}),".format(function["name"], lib["name"]))
+    print("PAIR({0}, &fexldr_ptr_{1}_{0})".format(function["name"], lib["name"]))
 
 ###
 ### ldr

ThunkLibs/Generators/libxcb.py

@@ -65,7 +65,6 @@
 #cb("int XIfEventCB(Display*, XEvent*, XPointer)")
 
 fn("int xcb_take_socket_internal(xcb_connection_t *, CBType, void *, int, uint64_t *)"); no_ldr()
-cb("void xcb_take_socket_cb(void*)"); no_cb_unpack()
 
 fn("int xcb_writev(xcb_connection_t *, struct iovec *, int, uint64_t)")
 fn("void * xcb_wait_for_reply(xcb_connection_t *, unsigned int, xcb_generic_error_t **)"); no_pack()

ThunkLibs/HostLibs/CMakeLists.txt

@@ -59,6 +59,9 @@ function(add_host_lib_with_name NAME LIBNAME)
   target_link_libraries(${LIBNAME}-host PRIVATE dl)
   target_compile_options(${LIBNAME}-host PRIVATE -DLIB_NAME=${LIBNAME} -DLIBLIB_NAME=lib${LIBNAME})
 
+  # generated files forward-declare functions that need to be implemented manually, so pass --no-undefined to make sure errors are detected at compile-time rather than runtime
+  target_link_options(${LIBNAME}-host PRIVATE "-Wl,--no-undefined")
+
   add_custom_target(${LIBNAME}-host-install
     COMMAND ${CMAKE_COMMAND} -E make_directory $ENV{DESTDIR}/${DATA_DIRECTORY}/HostThunks/
     COMMAND ${CMAKE_COMMAND} -E copy_if_different ${CMAKE_BINARY_DIR}/lib${LIBNAME}-host.so $ENV{DESTDIR}/${DATA_DIRECTORY}/HostThunks/)

ThunkLibs/include/common/CrossArchEvent.h

@@ -1,9 +1,11 @@
 #pragma once
 
 #include <linux/futex.h>
+#include <sys/syscall.h>
 #include <atomic>
 #include <condition_variable>
 #include <mutex>
+#include <unistd.h>
 
 struct CrossArchEvent final {
   std::atomic<uint32_t> Futex;

ThunkLibs/libvulkan_device/Guest.cpp

@@ -23,7 +23,7 @@ tags: thunklibs|vulkan
 
 extern "C" {
 static bool Setup{};
-static std::unordered_map<std::string,PFN_vkVoidFunction*> PtrsToLookUp{};
+static std::unordered_map<std::string_view,PFN_vkVoidFunction*> PtrsToLookUp{};
 
 static PFN_vkVoidFunction fexfn_pack_vkGetDeviceProcAddr(VkDevice a_0,const char* a_1);
 static PFN_vkVoidFunction fexfn_pack_vkGetInstanceProcAddr(VkInstance a_0,const char* a_1);
@@ -32,14 +32,13 @@ static void fexfn_pack_vkCmdSetBlendConstants(VkCommandBuffer a_0,const float a_
 // Setup can't be done on shared library constructor
 // Needs to be deferred until post-constructor phase to remove the chance of crashing
 static void DoSetup() {
-    const std::vector<std::pair<const char*, PFN_vkVoidFunction*>> Map = {{
-#define PAIR(name, ptr) { #name, (PFN_vkVoidFunction*) ptr }
+    // Initialize unordered_map from generated initializer-list
+    PtrsToLookUp = {
+#define PAIR(name, ptr) { #name, (PFN_vkVoidFunction*)ptr }
 #include "function_pack_pair.inl"
 #undef PAIR
-    }};
-    for (auto &It : Map) {
-      PtrsToLookUp[It.first] = It.second;
-    }
+    };
+
     Setup = true;
 }
 static PFN_vkVoidFunction fexfn_pack_vkGetDeviceProcAddr(VkDevice a_0,const char* a_1){
@@ -60,15 +59,11 @@ static PFN_vkVoidFunction fexfn_pack_vkGetDeviceProcAddr(VkDevice a_0,const char
   // Okay, we found a host side function for this
   // Now return our local instance of this function
   auto It = PtrsToLookUp.find(a_1);
-  void *ptr{};
-  if (It != PtrsToLookUp.end()) {
-    ptr = It->second;
-  }
-  if (ptr == nullptr) {
+  if (It == PtrsToLookUp.end() || !It->second) {
     fprintf(stderr, "\tvkGetDeviceProcAddr: Couldn't find Guest symbol: '%s'\n", a_1);
     __builtin_trap();
   }
-  return (PFN_vkVoidFunction)ptr;
+  return (PFN_vkVoidFunction)It->second;
 }
 
 static PFN_vkVoidFunction fexfn_pack_vkGetInstanceProcAddr(VkInstance a_0,const char* a_1){
@@ -87,17 +82,12 @@ static PFN_vkVoidFunction fexfn_pack_vkGetInstanceProcAddr(VkInstance a_0,const
     return nullptr;
   }
 
-  void *ptr{};
-
   auto It = PtrsToLookUp.find(a_1);
-  if (It != PtrsToLookUp.end()) {
-    ptr = (void*)(It->second);
-  }
-  if (ptr == nullptr) {
+  if (It == PtrsToLookUp.end() || !It->second) {
     fprintf(stderr, "\tvkGetInstanceProcAddr: Couldn't find Guest symbol: '%s'\n", a_1);
     __builtin_trap();
   }
-  return (PFN_vkVoidFunction)ptr;
+  return (PFN_vkVoidFunction)It->second;
 }
 
 static void fexfn_pack_vkCmdSetBlendConstants(VkCommandBuffer a_0,const float a_1[4]){

ThunkLibs/libvulkan_device/Host.cpp

@@ -22,45 +22,48 @@ static bool SetupDev{};
 static bool SetupInstance{};
 std::mutex SetupMutex{};
 
-static std::unordered_map<std::string,PFN_vkVoidFunction*> PtrsToLookUp{};
+static std::unordered_map<std::string_view,PFN_vkVoidFunction*> PtrsToLookUp{};
 
-const std::vector<std::pair<const char*, PFN_vkVoidFunction*>> Map = {{
-  // Our local function
-#define PAIR(name, ptr) { #name,  (PFN_vkVoidFunction*) ptr }
+static void DoSetupWithDevice(VkDevice dev) {
+    std::unique_lock lk {SetupMutex};
+
+    auto add_ptr = [&](const char* name) {
+        auto Lookup = PtrsToLookUp.find(name);
+        if (Lookup != PtrsToLookUp.end() && *Lookup->second == nullptr) {
+            auto Res = LDR_PTR(vkGetDeviceProcAddr)(dev, name);
+            if (Res) {
+                *Lookup->second = Res;
+            }
+        }
+    };
+
+#define PAIR(name, ptr_unused) add_ptr(#name);
 #include "ldr_ptrs_pair.inl"
 #undef PAIR
-}};
 
-static void DoSetupWithDevice(VkDevice dev) {
-  std::unique_lock lk {SetupMutex};
-    for (auto &It : Map) {
-      auto Lookup = PtrsToLookUp.find(It.first);
-      if (Lookup != PtrsToLookUp.end() && *Lookup->second == nullptr)
-      {
-        auto Res = LDR_PTR(vkGetDeviceProcAddr)(dev,It.first);
-        if (Res) {
-          *PtrsToLookUp[It.first] = Res;
-        }
-      }
-    }
     SetupDev = true;
 }
 
 static void DoSetupWithInstance(VkInstance instance) {
-  std::unique_lock lk {SetupMutex};
+    std::unique_lock lk {SetupMutex};
 
-    for (auto &It : Map) {
-      auto Lookup = PtrsToLookUp.find(It.first);
-      //if (Lookup != PtrsToLookUp.end() && *Lookup->second == nullptr)
-      {
-        PFN_vkVoidFunction Res = LDR_PTR(vkGetInstanceProcAddr)(instance, It.first);
+    auto add_ptr = [&](const char* name) {
+        auto Lookup = PtrsToLookUp.find(name);
+        auto Res = LDR_PTR(vkGetInstanceProcAddr)(instance, name);
         if (Res) {
-          *Lookup->second = Res;
+            *Lookup->second = Res;
         }
-      }
-    }
+    };
 
-    SetupInstance = true;
+#define PAIR(name, ptr) add_ptr(#name);
+#include "ldr_ptrs_pair.inl"
+#undef PAIR
+
+    // Only do this lookup once.
+    // NOTE: If vkGetInstanceProcAddr was called with a null instance, only a few function pointers will be filled with non-null values, so we do repeat the lookup in that case
+    if (instance) {
+        SetupInstance = true;
+    }
 }
 
 static void UNPACKFUNC(vkGetDeviceProcAddr)(void *argsv){
@@ -76,12 +79,6 @@ static void UNPACKFUNC(vkGetDeviceProcAddr)(void *argsv){
   args->rv =
     LDR_PTR(vkGetDeviceProcAddr)
     (args->a_0,args->a_1);
-
-  // Store the result if it exists
-  // This way when a guest calls across the boundary, it calls directly in to the host
-  if (args->rv) {
-    *PtrsToLookUp[args->a_1] = args->rv;
-  }
 }
 
 static void UNPACKFUNC(vkGetInstanceProcAddr)(void *argsv){
@@ -97,12 +94,6 @@ static void UNPACKFUNC(vkGetInstanceProcAddr)(void *argsv){
   args->rv =
     LDR_PTR(vkGetInstanceProcAddr)
     (args->a_0,args->a_1);
-
-  // Store the result if it exists
-  // This way when a guest calls across the boundary, it calls directly in to the host
-  if (args->rv) {
-    *PtrsToLookUp[args->a_1] = args->rv;
-  }
 }
 
 static void UNPACKFUNC(vkCreateShaderModule)(void *argsv){
@@ -166,9 +157,12 @@ static ExportEntry exports[] = {
 #include "ldr.inl"
 
 static void DoSetup() {
-  for (auto &It : Map) {
-    PtrsToLookUp[It.first] = It.second;
-  }
+    // Initialize unordered_map from generated initializer-list
+    PtrsToLookUp = {
+#define PAIR(name, ptr) { #name, (PFN_vkVoidFunction*)ptr },
+#include "ldr_ptrs_pair.inl"
+#undef PAIR
+    };
 }
 
 static void init_func() {

ThunkLibs/libxcb/libxcb_Guest.cpp

@@ -78,17 +78,6 @@ static void CallbackThreadFunc() {
   }
 }
 
-// Callbacks
-static void fexfn_unpack_libxcb_xcb_take_socket_cb(uintptr_t cb, void *argsv){
-  // Hand the callback off to our native thread which will have proper TLS
-  CBWorkData.cb = cb;
-  CBWorkData.argsv = argsv;
-  // Tell the thread it has work
-  NotifyWorkFunc(&WaitForWork);
-  // Wait for the work to be done
-  WaitForWorkFunc(&WorkDone);
-}
-
 extern "C" {
   static void init_lib() {
     // Start a guest side thread that allows us to do callbacks from xcb safely