From 7a703e1176a55ba6bfc67ede05f27134fb9517a0 Mon Sep 17 00:00:00 2001 From: Tony Wasserka Date: Tue, 11 Jun 2024 16:55:23 +0200 Subject: [PATCH] Library Forwarding/GL: Enable stricter pointer parameter checks --- ThunkLibs/Generator/analysis.cpp | 14 ++++++++++---- ThunkLibs/libGL/libGL_Host.cpp | 12 ++++++------ ThunkLibs/libGL/libGL_interface.cpp | 10 ++++++++++ 3 files changed, 26 insertions(+), 10 deletions(-) diff --git a/ThunkLibs/Generator/analysis.cpp b/ThunkLibs/Generator/analysis.cpp index 16001a7859..3e2a6341cf 100644 --- 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 index 6b24c5f891..5e683a55d8 100644 --- 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 -T* RelocateArrayToGuestHeap(T* Data, int NumItems) { +guest_layout RelocateArrayToGuestHeap(T* Data, int NumItems) { if (!Data) { - return nullptr; + return guest_layout {.data = 0}; } guest_layout 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 MapToGuestVisualInfo(Display* HostDisplay, XVi return GuestRet; } -GLXFBConfig* fexfn_impl_libGL_glXChooseFBConfig(Display* Display, int Screen, const int* Attributes, int* NumItems) { +guest_layout 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 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 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 index c26c741b22..87e007ed91 100644 --- a/ThunkLibs/libGL/libGL_interface.cpp +++ b/ThunkLibs/libGL/libGL_interface.cpp @@ -45,6 +45,10 @@ struct fex_gen_type : fexgen::opaque_type {}; template<> struct fex_gen_type> : 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> : fexgen::opaque_type {}; template<> @@ -89,6 +93,8 @@ struct fex_gen_config {}; template<> struct fex_gen_config : fexgen::custom_host_impl {}; template<> +struct fex_gen_param : fexgen::ptr_passthrough {}; +template<> struct fex_gen_config : fexgen::custom_host_impl {}; template<> struct fex_gen_param : fexgen::ptr_passthrough {}; @@ -179,8 +185,12 @@ struct fex_gen_config {}; template<> struct fex_gen_config : fexgen::custom_host_impl {}; template<> +struct fex_gen_param : fexgen::ptr_passthrough {}; +template<> struct fex_gen_config : fexgen::custom_host_impl {}; template<> +struct fex_gen_param : fexgen::ptr_passthrough {}; +template<> struct fex_gen_config {}; template<> struct fex_gen_config : fexgen::custom_host_impl {};