Library Forwarding/GL: Enable stricter pointer parameter checks

FEX-Emu · Jun 11, 2024 · 7a703e1 · 7a703e1
1 parent 02df1a2
commit 7a703e1
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 10 deletions.
diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp
@@ -468,10 +468,16 @@ void AnalysisAction::ParseInterface(clang::ASTContext& context) {
                 types.emplace(context.getCanonicalType(pointee_type.getTypePtr()), RepackedType {});
               } else if (data.param_annotations[param_idx].is_passthrough) {
                 // Nothing to do
-              } 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 {
+                // Assume this parameter type is unsupported.
+                // Since not all of our libraries are adapted for this yet, so
+                // an error is only thrown for a curated set of functions.
+                // TODO: At least detect and reject pointers-to-pointers on 32-bit
+                if (emitted_function->getNameAsString().starts_with("gl") && pointee_type->isPointerType()) {
+                  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

diff --git a/ThunkLibs/libGL/libGL_Host.cpp b/ThunkLibs/libGL/libGL_Host.cpp
@@ -291,9 +291,9 @@ void fexfn_impl_libGL_glShaderSourceARB(GLuint a_0, GLsizei count, guest_layout<
 // Relocate data to guest heap so it can be called with XFree.
 // The memory at the given host location will be de-allocated.
 template<typename T>
-T* RelocateArrayToGuestHeap(T* Data, int NumItems) {
+guest_layout<T*> RelocateArrayToGuestHeap(T* Data, int NumItems) {
   if (!Data) {
-    return nullptr;
+    return guest_layout<T*> {.data = 0};
   }
 
   guest_layout<T*> GuestData;
@@ -302,7 +302,7 @@ T* RelocateArrayToGuestHeap(T* Data, int NumItems) {
     GuestData.get_pointer()[Index] = to_guest(to_host_layout(Data[Index]));
   }
   x11_manager.HostXFree(Data);
-  return GuestData.force_get_host_pointer();
+  return GuestData;
 }
 
 // Maps to a host-side XVisualInfo, which must be XFree'ed by the caller.
@@ -350,12 +350,12 @@ static guest_layout<XVisualInfo*> MapToGuestVisualInfo(Display* HostDisplay, XVi
   return GuestRet;
 }
 
-GLXFBConfig* fexfn_impl_libGL_glXChooseFBConfig(Display* Display, int Screen, const int* Attributes, int* NumItems) {
+guest_layout<GLXFBConfig*> fexfn_impl_libGL_glXChooseFBConfig(Display* Display, int Screen, const int* Attributes, int* NumItems) {
   auto ret = fexldr_ptr_libGL_glXChooseFBConfig(Display, Screen, Attributes, NumItems);
   return RelocateArrayToGuestHeap(ret, *NumItems);
 }
 
-GLXFBConfigSGIX* fexfn_impl_libGL_glXChooseFBConfigSGIX(Display* Display, int Screen, int* Attributes, int* NumItems) {
+guest_layout<GLXFBConfigSGIX*> fexfn_impl_libGL_glXChooseFBConfigSGIX(Display* Display, int Screen, int* Attributes, int* NumItems) {
   auto ret = fexldr_ptr_libGL_glXChooseFBConfigSGIX(Display, Screen, Attributes, NumItems);
   return RelocateArrayToGuestHeap(ret, *NumItems);
 }
@@ -370,7 +370,7 @@ guest_layout<_XDisplay*> fexfn_impl_libGL_glXGetCurrentDisplayEXT() {
   return x11_manager.HostToGuestDisplay(ret);
 }
 
-GLXFBConfig* fexfn_impl_libGL_glXGetFBConfigs(Display* Display, int Screen, int* NumItems) {
+guest_layout<GLXFBConfig*> fexfn_impl_libGL_glXGetFBConfigs(Display* Display, int Screen, int* NumItems) {
   auto ret = fexldr_ptr_libGL_glXGetFBConfigs(Display, Screen, NumItems);
   return RelocateArrayToGuestHeap(ret, *NumItems);
 }

diff --git a/ThunkLibs/libGL/libGL_interface.cpp b/ThunkLibs/libGL/libGL_interface.cpp
@@ -45,6 +45,10 @@ struct fex_gen_type<void> : fexgen::opaque_type {};
 
 template<>
 struct fex_gen_type<std::remove_pointer_t<GLXContext>> : fexgen::opaque_type {};
+// NOTE: The data layout of this is almost the same between 64-bit and 32-bit,
+//       but the total struct size is 4 bytes larger on 64-bit due to stricter
+//       alignment requirements (8 vs 4 bytes). Since it's always allocated on
+//       the host *and* never directly used in arrays, this is not a problem.
 template<>
 struct fex_gen_type<std::remove_pointer_t<GLXFBConfig>> : fexgen::opaque_type {};
 template<>
@@ -89,6 +93,8 @@ struct fex_gen_config<glXGetCurrentReadDrawableSGI> {};
 template<>
 struct fex_gen_config<glXChooseFBConfigSGIX> : fexgen::custom_host_impl {};
 template<>
+struct fex_gen_param<glXChooseFBConfigSGIX, -1, GLXFBConfigSGIX*> : fexgen::ptr_passthrough {};
+template<>
 struct fex_gen_config<glXGetFBConfigFromVisualSGIX> : fexgen::custom_host_impl {};
 template<>
 struct fex_gen_param<glXGetFBConfigFromVisualSGIX, 1, XVisualInfo*> : fexgen::ptr_passthrough {};
@@ -179,8 +185,12 @@ struct fex_gen_config<glXGetCurrentReadDrawable> {};
 template<>
 struct fex_gen_config<glXChooseFBConfig> : fexgen::custom_host_impl {};
 template<>
+struct fex_gen_param<glXChooseFBConfig, -1, GLXFBConfig*> : fexgen::ptr_passthrough {};
+template<>
 struct fex_gen_config<glXGetFBConfigs> : fexgen::custom_host_impl {};
 template<>
+struct fex_gen_param<glXGetFBConfigs, -1, GLXFBConfig*> : fexgen::ptr_passthrough {};
+template<>
 struct fex_gen_config<glXCreatePbuffer> {};
 template<>
 struct fex_gen_config<glXCreateGLXPixmap> : fexgen::custom_host_impl {};