Skip to content

Commit a4291d3

Browse files
committed
fix emscripten, more windows security fixes, docs
1 parent 6d49c64 commit a4291d3

3 files changed

Lines changed: 96 additions & 49 deletions

File tree

src/libprojectM/Renderer/PlatformGLResolver.cpp

Lines changed: 20 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
#include <cstring>
1717
#include <cstdint>
1818
#include <cstdio>
19+
20+
#ifdef _WIN32
1921
#include <cstdlib>
2022
#include <limits>
23+
#endif
2124

2225
#ifdef _WIN32
2326
#include <windows.h>
@@ -99,6 +102,8 @@ auto HasSpaceSeparatedToken(const char* list, const char* token) -> bool
99102
return false;
100103
}
101104

105+
#ifndef __EMSCRIPTEN__
106+
102107
/**
103108
* @brief Heuristically determines whether a name looks like an OpenGL-style extension symbol.
104109
*
@@ -140,17 +145,8 @@ auto IsLikelyExtensionName(const char* name) -> bool
140145
return false;
141146
}
142147

143-
144-
#ifndef __EMSCRIPTEN__
145-
146148
/**
147149
* @brief Returns true if a GL/EGL symbol name is likely to be an extension entry point.
148-
*
149-
* Per EGL_KHR_get_all_proc_addresses (and its client variant), eglGetProcAddress
150-
* is not required to return addresses for non-extension EGL or client API functions
151-
* unless the extension is advertised. When the extension is not available, a
152-
* conservative policy is to query eglGetProcAddress only for extension-style
153-
* entry points (ARB/EXT/KHR/etc.), while resolving core symbols via direct exports.
154150
*/
155151
auto ShouldUseEglGetProcAddressForName(const char* name) -> bool
156152
{
@@ -228,6 +224,7 @@ auto GLResolver::CheckGLRequirementsUnlocked() -> GLContextCheckResult
228224

229225
auto GLResolver::Initialize(UserResolver resolver, void* userData) -> bool
230226
{
227+
231228
// Prevent concurrent Initialize()
232229
std::unique_lock<std::mutex> lock(m_mutex);
233230

@@ -382,11 +379,15 @@ auto GLResolver::GetProcAddress(const char* name) const -> void*
382379
// Only hold m_mutex while reading internal state.
383380
std::unique_lock<std::mutex> lock(m_mutex);
384381

382+
if (!m_loaded && !m_initializing)
383+
{
384+
LOG_ERROR(std::string("[GLResolver] GetProcAddress called without initialization."));
385+
return nullptr;
386+
}
387+
385388
// Local state copy to use once the lock is released.
386389
const auto state = m_state;
387390

388-
const bool loaded = m_loaded;
389-
390391
// Gate to detected backend.
391392
const auto currentContext = ProbeCurrentContext();
392393

@@ -430,14 +431,7 @@ auto GLResolver::GetProcAddress(const char* name) const -> void*
430431
}
431432
break;
432433
default:
433-
if (!loaded)
434-
{
435-
LOG_ERROR(std::string("[GLResolver] Backend is not initialized."));
436-
}
437-
else
438-
{
439-
LOG_ERROR(std::string("[GLResolver] Initialization did not choose a valid backend."));
440-
}
434+
LOG_ERROR(std::string("[GLResolver] Initialization did not choose a valid backend."));
441435
return nullptr;
442436
}
443437

@@ -448,6 +442,8 @@ auto GLResolver::GetProcAddress(const char* name) const -> void*
448442
return resolved;
449443
}
450444

445+
#ifndef __EMSCRIPTEN__
446+
451447
lock.lock();
452448

453449
// Global symbol table (works if the process already linked/loaded GL libs).
@@ -499,6 +495,7 @@ auto GLResolver::GetProcAddress(const char* name) const -> void*
499495
return FunctionToSymbol(proc);
500496
}
501497
}
498+
#endif
502499
return nullptr;
503500
}
504501

@@ -671,7 +668,7 @@ void GLResolver::ResolveProviderFunctions()
671668
constexpr int kEglExtensions = 0x3055; // EGL_EXTENSIONS
672669
constexpr int kEglSuccess = 0x3000; // EGL_SUCCESS
673670
constexpr int kEglBadDisplay = 0x3008; // EGL_BAD_DISPLAY
674-
constexpr EglDisplay kEglNoDisplay = nullptr; // EGL_NO_DISPLAY
671+
EglDisplay kEglNoDisplay = nullptr; // EGL_NO_DISPLAY
675672

676673
void* querySym = nullptr;
677674
if (m_eglLib.IsOpen())
@@ -1230,7 +1227,9 @@ auto GLResolver::ResolveUnlocked(const char* name, const ResolverState& state) -
12301227
// wglGetProcAddress can return special sentinel values (1,2,3,-1) for core symbols.
12311228
// Treat those as invalid and allow fallback to GetProcAddress/dlsym paths.
12321229
const std::uintptr_t raw = FunctionToInteger(proc);
1233-
if (raw != 1u && raw != 2u && raw != 3u && raw != std::numeric_limits<std::uintptr_t>::max())
1230+
// FunctionToInteger returns 0 on non-representable conversions (e.g. function pointers wider than void*).
1231+
// Treat that as invalid and allow fallback to exported symbols.
1232+
if (raw != 0u && raw != 1u && raw != 2u && raw != 3u && raw != std::numeric_limits<std::uintptr_t>::max())
12341233
{
12351234
// Prefer exports from opengl32.dll for core OpenGL 1.1 entry points.
12361235
// In the wild, wglGetProcAddress may return a non-null pointer for some core symbols

src/libprojectM/Renderer/PlatformGLResolver.hpp

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <memory>
99
#include <mutex>
1010
#include <string>
11+
#include <thread>
1112

1213
namespace libprojectM
1314
{
@@ -82,24 +83,36 @@ using UserResolver = void* (*)(const char* name, void* userData);
8283
* - EGL: eglGetProcAddress
8384
* - Queried for all symbols only when EGL_KHR_get_all_proc_addresses or
8485
* EGL_KHR_client_get_all_proc_addresses is advertised.
85-
* - Otherwise queried only for extension-style names.
86+
* - Otherwise queried only for extension-style names during the provider step.
87+
* - As a last resort (after exports/global lookups fail), a best-effort call
88+
* to eglGetProcAddress may be attempted even for non-extension names to support
89+
* stacks that expose core client API entry points only via eglGetProcAddress.
8690
* - GLX: glXGetProcAddressARB / glXGetProcAddress
8791
* - Queried only for glX* or extension-style names.
8892
* - WGL: wglGetProcAddress
8993
* - Filters sentinel values; prefers exported symbols for core OpenGL 1.1 entry points.
9094
* 3) Global symbol scope lookup (dlsym(RTLD_DEFAULT) / GetProcAddress on already-loaded modules)
9195
* 4) Direct exports from explicitly opened libraries (EGL/GL/GLX)
96+
* 5) EGL only: Try to resolve function via eglGetProcAddress() as a fallback.
9297
*
9398
* Resolution order (Emscripten/WebGL):
9499
* 1) User resolver callback (if provided)
95100
* 2) emscripten_webgl2_get_proc_address / emscripten_webgl_get_proc_address
96101
* (prefers the current context major version)
97102
*
103+
* ## Loader Flow (Emscripten)
104+
*
105+
* GLResolver::Initialize(userResolver?, userData?)
106+
* |
107+
* +-- Has userResolver
108+
* +-- yes: Store user resolver, will use user resolver + emscripten_webgl*_get_proc_address fallback.
109+
* +-- no: Do not initialize GLAD at all, rely on WebGL static linking.
110+
*
98111
* ## Loader Flow (non-Emscripten)
99112
*
100113
* GLResolver::Initialize(userResolver?, userData?)
101114
* |
102-
* +-- OpenNativeLibraries() (best-effort)
115+
* +-- OpenNativeLibraries()
103116
* | +-- open EGL library
104117
* | | Windows: libEGL.dll | EGL.dll
105118
* | | macOS: libEGL.dylib | libEGL.1.dylib | EGL
@@ -131,7 +144,7 @@ using UserResolver = void* (*)(const char* name, void* userData);
131144
* |
132145
* +-- DetectBackend() (prefers EGL, then WGL/CGL/GLX depending on platform)
133146
* |
134-
* +-- gladLoadGL(...) / gladLoadGLES2(...) (GLAD calls back into GLResolver::GetProcAddress)
147+
* +-- gladLoadGL(...) / gladLoadGLES2(...) (GLAD calls back into GLResolver::GetProcAddress)
135148
* |
136149
* +-- CheckGLRequirementsUnlocked()
137150
* | Desktop GL: OpenGL >= 3.3
@@ -145,15 +158,17 @@ using UserResolver = void* (*)(const char* name, void* userData);
145158
*
146159
* GLResolver::GetProcAddress(name)
147160
* |
148-
* +-- validates the currently-bound context matches the detected backend
161+
* +-- Validates the currently-bound context matches the detected backend.
149162
* |
150-
* +-- ResolveUnlocked(name, state)
151-
* | +-- user resolver (if any)
152-
* | +-- provider getProcAddress (backend-aware)
153-
* | EGL -> eglGetProcAddress (extension-only unless EGL_KHR_*_get_all_proc_addresses)
154-
* | GLX -> glXGetProcAddress* (glX* and extension-style only)
155-
* | WGL -> wglGetProcAddress (filters sentinels; may prefer exported symbols)
156-
* | WebGL-> emscripten_webgl*_get_proc_address (Emscripten only)
163+
* +-- User resolver (if any)
164+
* |
165+
* +-- Provider getProcAddress (backend-aware)
166+
* | EGL -> eglGetProcAddress (extension-only unless EGL_KHR_*_get_all_proc_addresses)
167+
* | GLX -> glXGetProcAddress* (glX* and extension-style only)
168+
* | WGL -> wglGetProcAddress (filters sentinels; may prefer exported symbols)
169+
* | WebGL-> emscripten_webgl*_get_proc_address (Emscripten only)
170+
* |
171+
* +-- If Emscripten -> All available lookups have been tried, return nullptr.
157172
* |
158173
* +-- DynamicLibrary::FindGlobalSymbol(name)
159174
* |
@@ -162,8 +177,10 @@ using UserResolver = void* (*)(const char* name, void* userData);
162177
* | m_glLib.GetSymbol(name)
163178
* | m_glxLib.GetSymbol(name)
164179
* |
180+
* +-- Late best-effort eglGetProcAddress() fallback (EGL only).
181+
* |
165182
* \/
166-
* nullptr
183+
* nullptr
167184
*/
168185
class GLResolver
169186
{
@@ -195,9 +212,7 @@ class GLResolver
195212
* This method must be called at least once before GetProcAddress() is used.
196213
*
197214
* Thread-safety: Initialize() is internally synchronized. Intended usage is to initialize
198-
* once during startup before any resolution occurs. GetProcAddress() assumes
199-
* initialization has completed successfully (per API contract) and does not wait
200-
* for an in-flight Initialize() call.
215+
* once during startup before any resolution occurs. GetProcAddress() should not be called until Initialize() has completed.
201216
*
202217
* May be called multiple times; initialization work is performed only when needed.
203218
*

src/libprojectM/Renderer/PlatformLoader.hpp

Lines changed: 46 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ class DynamicLibrary
276276
DynamicLibrary(const DynamicLibrary&) = delete;
277277
DynamicLibrary& operator=(const DynamicLibrary&) = delete;
278278

279-
inline auto Open(const char* const*, std::string& reason) -> bool
279+
inline auto Open(const char* const*, std::string&) -> bool
280280
{
281281
return false;
282282
}
@@ -300,7 +300,7 @@ class DynamicLibrary
300300
return nullptr;
301301
}
302302

303-
inline auto SetCloseOnDestruct(bool enabled) -> void
303+
inline auto SetCloseOnDestruct(bool) -> void
304304
{
305305
}
306306

@@ -328,7 +328,13 @@ class DynamicLibrary
328328
public:
329329
DynamicLibrary() = default;
330330

331-
~DynamicLibrary() = default;
331+
~DynamicLibrary()
332+
{
333+
if (m_closeOnDestruct)
334+
{
335+
Close();
336+
}
337+
}
332338

333339
DynamicLibrary(const DynamicLibrary&) = delete;
334340
auto operator=(const DynamicLibrary&) -> DynamicLibrary& = delete;
@@ -487,21 +493,36 @@ class DynamicLibrary
487493
// explicit full path improves dependency resolution by allowing the loader to search
488494
// the DLL's directory for its dependent DLLs (LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR).
489495
// This keeps the search path restricted without changing process-wide state.
490-
const std::string appFull = buildAppDirPath(name);
491-
if (!appFull.empty())
496+
const bool isSystemOpenGL32 = (_stricmp(name, "opengl32.dll") == 0);
497+
498+
// For system DLLs like opengl32.dll, never prefer the application directory.
499+
// Loading system DLLs from app-local paths enables DLL preloading/hijacking.
500+
std::string appFull;
501+
if (!isSystemOpenGL32)
492502
{
493-
handle = tryLoadEx(appFull.c_str(), LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
494-
if (handle == nullptr && ::GetLastError() == ERROR_INVALID_PARAMETER)
503+
appFull = buildAppDirPath(name);
504+
if (!appFull.empty())
495505
{
496-
// Older OS loader: use altered search path for absolute paths to improve
497-
// dependent DLL resolution.
498-
handle = tryLoadExplicitPathFallback(appFull.c_str());
506+
handle = tryLoadEx(appFull.c_str(), LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS);
507+
if (handle == nullptr && ::GetLastError() == ERROR_INVALID_PARAMETER)
508+
{
509+
// Older OS loader: use altered search path for absolute paths to improve
510+
// dependent DLL resolution.
511+
handle = tryLoadExplicitPathFallback(appFull.c_str());
512+
}
499513
}
500514
}
501515

502516
if (handle == nullptr)
503517
{
504-
handle = tryLoadEx(name, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
518+
if (isSystemOpenGL32)
519+
{
520+
handle = tryLoadEx(name, LOAD_LIBRARY_SEARCH_SYSTEM32);
521+
}
522+
else
523+
{
524+
handle = tryLoadEx(name, LOAD_LIBRARY_SEARCH_APPLICATION_DIR | LOAD_LIBRARY_SEARCH_SYSTEM32);
525+
}
505526
}
506527

507528
if (handle == nullptr && ::GetLastError() == ERROR_INVALID_PARAMETER)
@@ -641,6 +662,17 @@ class DynamicLibrary
641662
return m_handle;
642663
}
643664

665+
/**
666+
* @brief Controls whether the library is closed in the destructor.
667+
*
668+
* The default is false to avoid unloading GL/driver libraries during process teardown.
669+
* Enable only for short-lived helper loads where unloading is safe and desired.
670+
*/
671+
auto SetCloseOnDestruct(bool enabled) -> void
672+
{
673+
m_closeOnDestruct = enabled;
674+
}
675+
644676
/**
645677
* @brief Resolves a symbol from this library.
646678
* @param name The symbol name.
@@ -742,8 +774,9 @@ class DynamicLibrary
742774
}
743775

744776
private:
745-
LibHandle m_handle{}; //!< Library handle used to access the system library.
746-
std::string m_loadedName; //< Successfully opened library name.
777+
LibHandle m_handle{}; //!< Library handle used to access the system library.
778+
std::string m_loadedName; //< Successfully opened library name.
779+
bool m_closeOnDestruct{false}; //!< If true, Close() is called from the destructor.
747780
};
748781

749782
#endif // #ifdef __EMSCRIPTEN__

0 commit comments

Comments
 (0)