From 24f7d45364510a4521a2cb70ed5600a7a83bb142 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sat, 19 Oct 2019 00:27:19 +0200 Subject: [PATCH] Implement globals shared across DLLs on Windows. --- doc/changelog.dox | 3 +- src/Magnum/Audio/Context.cpp | 39 ++++++++++++++-- .../Test/GlobalStateAcrossLibrariesALTest.cpp | 7 +-- src/Magnum/CMakeLists.txt | 6 +++ src/Magnum/GL/Context.cpp | 41 ++++++++++++++--- .../Test/GlobalStateAcrossLibrariesGLTest.cpp | 7 +-- .../Implementation/WindowsWeakSymbol.cpp | 43 ++++++++++++++++++ src/Magnum/Implementation/WindowsWeakSymbol.h | 44 +++++++++++++++++++ 8 files changed, 167 insertions(+), 23 deletions(-) create mode 100644 src/Magnum/Implementation/WindowsWeakSymbol.cpp create mode 100644 src/Magnum/Implementation/WindowsWeakSymbol.h diff --git a/doc/changelog.dox b/doc/changelog.dox index 091458c6e..fde1b6bd6 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -350,8 +350,7 @@ See also: @ref Corrade::Containers::Array instead of being deleted - Global state used by @ref GL::Context and @ref Audio::Context is no longer duplicated when Magnum is built statically and linked to more than one - dynamic library or executable. Currently that holds only for Linux and - macOS, Windows support will come next. + dynamic library or executable. - The [base-qt](https://github.com/mosra/magnum-bootstrap/tree/base-qt) bootstrap project is fixed to use OpenGL 4.1 on macOS instead of being stuck on 2.1 (see [mosra/magnum-bootstrap#22](https://github.com/mosra/magnum-bootstrap/issues/22)) diff --git a/src/Magnum/Audio/Context.cpp b/src/Magnum/Audio/Context.cpp index f14113e82..d099b258f 100644 --- a/src/Magnum/Audio/Context.cpp +++ b/src/Magnum/Audio/Context.cpp @@ -41,6 +41,10 @@ #include "Magnum/Audio/Extensions.h" +#if defined(CORRADE_TARGET_WINDOWS) && defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS_RT) +#include "Magnum/Implementation/WindowsWeakSymbol.h" +#endif + namespace Magnum { namespace Audio { namespace { @@ -113,14 +117,16 @@ std::vector Context::deviceSpecifierStrings() { return list; } -#ifndef MAGNUM_BUILD_STATIC -/* (Of course) can't be in an unnamed namespace in order to export it below */ +#if !defined(MAGNUM_BUILD_STATIC) || defined(CORRADE_TARGET_WINDOWS) +/* (Of course) can't be in an unnamed namespace in order to export it below + (except for Windows, where we do extern "C" so this doesn't matter) */ namespace { #endif /* Unlike GL, this isn't thread-local. Would need to implement ALC_EXT_thread_local_context first */ -#if defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS) +#if !defined(MAGNUM_BUILD_STATIC) || (defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS)) +#ifdef MAGNUM_BUILD_STATIC /* On static builds that get linked to multiple shared libraries and then used in a single app we want to ensure there's just one global symbol. On Linux it's apparently enough to just export, macOS needs the weak attribute. @@ -134,11 +140,36 @@ CORRADE_VISIBILITY_EXPORT #endif #endif Context* currentContext = nullptr; +#else +/* On Windows the symbol is exported unmangled and then fetched via + GetProcAddress() to emulate weak linking. */ +extern "C" CORRADE_VISIBILITY_EXPORT Context* magnumAudioUniqueCurrentContext = nullptr; +#endif -#ifndef MAGNUM_BUILD_STATIC +#if !defined(MAGNUM_BUILD_STATIC) || defined(CORRADE_TARGET_WINDOWS) } #endif +/* Windows don't have any concept of weak symbols, instead GetProcAddress() on + GetModuleHandle(nullptr) "emulates" the weak linking as it's guaranteed to + pick up the same symbol of the final exe independently of the DLL it was + called from. To avoid #ifdef hell in code below, the currentContext is + redefined to return a value from this uniqueness-ensuring function. */ +#if defined(CORRADE_TARGET_WINDOWS) && defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS_RT) +namespace { + +Context*& windowsCurrentContext() { + /* A function-local static to ensure it's only initialized once without any + race conditions among threads */ + static Context** const uniqueGlobals = reinterpret_cast(Magnum::Implementation::windowsWeakSymbol("magnumAudioUniqueCurrentContext")); + return *uniqueGlobals; +} + +} + +#define currentContext windowsCurrentContext() +#endif + bool Context::hasCurrent() { return currentContext; } Context& Context::current() { diff --git a/src/Magnum/Audio/Test/GlobalStateAcrossLibrariesALTest.cpp b/src/Magnum/Audio/Test/GlobalStateAcrossLibrariesALTest.cpp index 78ac07a7c..b6d96ee08 100644 --- a/src/Magnum/Audio/Test/GlobalStateAcrossLibrariesALTest.cpp +++ b/src/Magnum/Audio/Test/GlobalStateAcrossLibrariesALTest.cpp @@ -44,12 +44,7 @@ GlobalStateAcrossLibrariesALTest::GlobalStateAcrossLibrariesALTest() { void GlobalStateAcrossLibrariesALTest::test() { Context context; CORRADE_VERIFY(Context::hasCurrent()); - { - #ifdef CORRADE_TARGET_WINDOWS - CORRADE_EXPECT_FAIL("Deduplication of global data across shared libraries isn't implemented on Windows yet."); - #endif - CORRADE_COMPARE(currentContextInALibrary(), &context); - } + CORRADE_COMPARE(currentContextInALibrary(), &context); } }}}} diff --git a/src/Magnum/CMakeLists.txt b/src/Magnum/CMakeLists.txt index 71ab8a4c5..00fd8c3bd 100644 --- a/src/Magnum/CMakeLists.txt +++ b/src/Magnum/CMakeLists.txt @@ -66,6 +66,12 @@ set(Magnum_HEADERS set(Magnum_PRIVATE_HEADERS Implementation/ImageProperties.h) +# Functionality specific to static Windows builds +if(CORRADE_TARGET_WINDOWS AND NOT CORRADE_TARGET_WINDOWS_RT AND MAGNUM_BUILD_STATIC) + list(APPEND Magnum_SRCS Implementation/WindowsWeakSymbol.cpp) + list(APPEND Magnum_PRIVATE_HEADERS Implementation/WindowsWeakSymbol.h) +endif() + # Files shared between main library and math unit test library set(MagnumMath_SRCS Math/Angle.cpp diff --git a/src/Magnum/GL/Context.cpp b/src/Magnum/GL/Context.cpp index cac04a3ee..5c8e30b3f 100644 --- a/src/Magnum/GL/Context.cpp +++ b/src/Magnum/GL/Context.cpp @@ -65,6 +65,10 @@ #include "Magnum/GL/Implementation/TransformFeedbackState.h" #endif +#if defined(CORRADE_TARGET_WINDOWS) && defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS_RT) +#include "Magnum/Implementation/WindowsWeakSymbol.h" +#endif + namespace Magnum { namespace GL { namespace { @@ -473,8 +477,9 @@ Containers::ArrayView Extension::extensions(Version version) { CORRADE_ASSERT_UNREACHABLE(); /* LCOV_EXCL_LINE */ } -#ifndef MAGNUM_BUILD_STATIC -/* (Of course) can't be in an unnamed namespace in order to export it below */ +#if !defined(MAGNUM_BUILD_STATIC) || defined(CORRADE_TARGET_WINDOWS) +/* (Of course) can't be in an unnamed namespace in order to export it below + (except for Windows, where we do extern "C" so this doesn't matter) */ namespace { #endif @@ -485,8 +490,7 @@ CORRADE_THREAD_LOCAL /* On static builds that get linked to multiple shared libraries and then used in a single app we want to ensure there's just one global symbol. On Linux it's apparently enough to just export, macOS needs the weak attribute. - Windows not handled yet, as it needs a workaround using DllMain() and - GetProcAddress(). */ + Windows handled differently below. */ CORRADE_VISIBILITY_EXPORT #ifdef __GNUC__ __attribute__((weak)) @@ -496,8 +500,35 @@ CORRADE_VISIBILITY_EXPORT #endif Context* currentContext = nullptr; -#ifndef MAGNUM_BUILD_STATIC +#if !defined(MAGNUM_BUILD_STATIC) || defined(CORRADE_TARGET_WINDOWS) +} +#endif + +/* Windows can't have a symbol both thread-local and exported, moreover there + isn't any concept of weak symbols. Exporting thread-local symbols can be + worked around by exporting a function that then returns a reference to a + non-exported thread-local symbol; and finally GetProcAddress() on + GetModuleHandle(nullptr) "emulates" the weak linking as it's guaranteed to + pick up the same symbol of the final exe independently of the DLL it was + called from. To avoid #ifdef hell in code below, the currentContext is + redefined to return a value from this uniqueness-ensuring function. */ +#if defined(CORRADE_TARGET_WINDOWS) && defined(MAGNUM_BUILD_STATIC) && !defined(CORRADE_TARGET_WINDOWS_RT) +extern "C" CORRADE_VISIBILITY_EXPORT Context*& magnumGLUniqueCurrentContext() { + return currentContext; +} + +namespace { + +Context*& windowsCurrentContext() { + /* A function-local static to ensure it's only initialized once without any + race conditions among threads */ + static Context*&(*const uniqueGlobals)() = reinterpret_cast(Magnum::Implementation::windowsWeakSymbol("magnumGLUniqueCurrentContext")); + return uniqueGlobals(); } + +} + +#define currentContext windowsCurrentContext() #endif bool Context::hasCurrent() { return currentContext; } diff --git a/src/Magnum/GL/Test/GlobalStateAcrossLibrariesGLTest.cpp b/src/Magnum/GL/Test/GlobalStateAcrossLibrariesGLTest.cpp index da26ad784..70b488a53 100644 --- a/src/Magnum/GL/Test/GlobalStateAcrossLibrariesGLTest.cpp +++ b/src/Magnum/GL/Test/GlobalStateAcrossLibrariesGLTest.cpp @@ -42,12 +42,7 @@ GlobalStateAcrossLibrariesGLTest::GlobalStateAcrossLibrariesGLTest() { void GlobalStateAcrossLibrariesGLTest::test() { CORRADE_VERIFY(GL::Context::hasCurrent()); - { - #ifdef CORRADE_TARGET_WINDOWS - CORRADE_EXPECT_FAIL("Deduplication of global data across shared libraries isn't implemented on Windows yet."); - #endif - CORRADE_COMPARE(currentContextInALibrary(), &GL::Context::current()); - } + CORRADE_COMPARE(currentContextInALibrary(), &GL::Context::current()); } }}}} diff --git a/src/Magnum/Implementation/WindowsWeakSymbol.cpp b/src/Magnum/Implementation/WindowsWeakSymbol.cpp new file mode 100644 index 000000000..619cc606c --- /dev/null +++ b/src/Magnum/Implementation/WindowsWeakSymbol.cpp @@ -0,0 +1,43 @@ +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 + Vladimír Vondruš + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included + in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. +*/ + +#include "WindowsWeakSymbol.h" + +#include + +#define WIN32_LEAN_AND_MEAN 1 +#define VC_EXTRALEAN +#include + +namespace Magnum { namespace Implementation { + +void* windowsWeakSymbol(const char* name) { + /* FARPROC?! I want either a function pointer or a variable pointer */ + void* address = reinterpret_cast(GetProcAddress(GetModuleHandleA(nullptr), name)); + CORRADE_INTERNAL_ASSERT(address); + return address; +} + +}} diff --git a/src/Magnum/Implementation/WindowsWeakSymbol.h b/src/Magnum/Implementation/WindowsWeakSymbol.h new file mode 100644 index 000000000..ac6ecc31f --- /dev/null +++ b/src/Magnum/Implementation/WindowsWeakSymbol.h @@ -0,0 +1,44 @@ +#ifndef Magnum_Implementation_WindowsWeakSymbol_h +#define Magnum_Implementation_WindowsWeakSymbol_h +/* + This file is part of Magnum. + + Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 + Vladimír Vondruš + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included + in all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. +*/ + +#include + +/* This is a copy of Corrade/Utility/Implementation/windowsWeakSymbol.{h,cpp}, + just adapted for Magnum. Yeah, it's only in order to avoid including + windows.h in all the code. */ + +#if !defined(CORRADE_TARGET_WINDOWS) || !defined(MAGNUM_BUILD_STATIC) || defined(CORRADE_TARGET_WINDOWS_RT) +#error this file is only meant to be used in non-RT Windows static builds +#endif + +namespace Magnum { namespace Implementation { + +void* windowsWeakSymbol(const char* name); + +}} + +#endif