Interconnect: test emitting signals across shared object boundaries.

mosra · mosra · commit 5d2faa1fbeb2 · 2019-06-26T18:42:08.000+02:00
This worked "as is" a month ago when there was no
-fvisibility-inlines-hidden, but that flag broke it. Now we need to
explicitly export the inline functions as well.

Added a verification test as well as additional docs explaining the
situation.
diff --git a/doc/corrade-changelog.dox b/doc/corrade-changelog.dox
@@ -169,6 +169,8 @@ namespace Corrade {
     [mosra/corrade#53](https://github.com/mosra/corrade/issues/53))
 -   The Emscripten CMake toolchain now sets the `EMSCRIPTEN` variable to help
     3rd party projects (see [mosra/toolchains#7](https://github.com/mosra/toolchains/pull/7))
+-   Clarified interaction of `-fvisibility-inlines-hidden` and/or
+    `-Wl,-Bsymbolic-functions` GCC/Clang flags and @ref Interconnect::Emitter
 
 @subsection corrade-changelog-latest-bugfixes Bug fixes
 
diff --git a/src/Corrade/Interconnect/Emitter.h b/src/Corrade/Interconnect/Emitter.h
@@ -178,6 +178,44 @@ called from outside the class.
 
 <b></b>
 
+@m_class{m-block m-warning}
+
+@par Connecting slots across shared objects
+    If you're planning to use signal/slot connections across shared objects, be
+    aware that in combination with GCC/Clang `-fvisibility-inlines-hidden`
+    option (enabled by default when you use the `CORRADE_USE_PEDANTIC_FLAGS`
+    @ref corrade-cmake "CMake property"), address of given signal inside a
+    shared library will differ from its address outside of it. Copied from GCC
+    documentation:
+@par
+    <blockquote>
+    This switch declares that the user does not attempt to compare pointers
+    to inline methods where the addresses of the two functions were taken in
+    different shared objects.
+    </blockquote>
+@par
+    There are three possible solutions:
+@par
+    1.  Compile without `-fvisibility-inlines-hidden`. Not recommended, as you
+        usually have no control over externally specified flags. This flag is
+        not available on MSVC or MinGW and while MSVC "just works", MinGW
+        doesn't.
+    2.  Make the signal definition non-inline by moving its definition to a
+        `*.cpp` file. Potentially very verbose, but works everywhere including
+        MinGW.
+    3.  Annotating the function with an export macro such as
+        @ref CORRADE_VISIBILITY_INLINE_MEMBER_EXPORT from
+        @ref Corrade/Utility/VisibilityMacros.h that puts its visibility back
+        to default. Doesn't work on MinGW either.
+@par
+    Similar issue happens with the `-Wl,-Bsymbolic-functions` option passed to
+    the linker. This flag is unfortunately present by default when building
+    Ubuntu packages and has to be explicitly removed in order to make the
+    signal connections work correctly. See the `package/debian/rules` file for
+    an example how to do it.
+
+<b></b>
+
 @m_class{m-block m-danger}
 
 @par MSVC and identical function merging
diff --git a/src/Corrade/Interconnect/Test/CMakeLists.txt b/src/Corrade/Interconnect/Test/CMakeLists.txt
@@ -27,8 +27,13 @@ corrade_add_test(InterconnectTest Test.cpp LIBRARIES CorradeInterconnect)
 corrade_add_test(InterconnectStateMachineTest StateMachineTest.cpp LIBRARIES CorradeInterconnect)
 corrade_add_test(InterconnectBenchmark Benchmark.cpp LIBRARIES CorradeInterconnect)
 
+add_library(InterconnectTestEmitterLibrary ${SHARED_OR_STATIC} EmitterLibrary.cpp)
+target_link_libraries(InterconnectTestEmitterLibrary PUBLIC CorradeInterconnect)
+corrade_add_test(InterconnectLibraryTest LibraryTest.cpp LIBRARIES CorradeInterconnect InterconnectTestEmitterLibrary)
+
 set_target_properties(
     InterconnectTest
     InterconnectStateMachineTest
     InterconnectBenchmark
+    InterconnectLibraryTest
     PROPERTIES FOLDER "Corrade/Interconnect/Test")
diff --git a/src/Corrade/Interconnect/Test/EmitterLibrary.cpp b/src/Corrade/Interconnect/Test/EmitterLibrary.cpp
@@ -0,0 +1,42 @@
+/*
+    This file is part of Corrade.
+
+    Copyright © 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016,
+                2017, 2018, 2019 Vladimír Vondruš <mosra@centrum.cz>
+
+    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 "EmitterLibrary.h"
+
+namespace Corrade { namespace Interconnect { namespace Test {
+
+void EmitterLibrary::fireInlineThroughAFunction() {
+    fireInline();
+}
+
+void EmitterLibrary::fireNonInlineThroughAFunction() {
+    fireNonInline();
+}
+
+auto EmitterLibrary::fireNonInline() -> Signal {
+    return emit(&EmitterLibrary::fireNonInline);
+}
+
+}}}
diff --git a/src/Corrade/Interconnect/Test/EmitterLibrary.h b/src/Corrade/Interconnect/Test/EmitterLibrary.h
@@ -0,0 +1,57 @@
+#ifndef Corrade_Interconnect_Test_EmitterLibrary_h
+#define Corrade_Interconnect_Test_EmitterLibrary_h
+/*
+    This file is part of Corrade.
+
+    Copyright © 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016,
+                2017, 2018, 2019 Vladimír Vondruš <mosra@centrum.cz>
+
+    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 "Corrade/Interconnect/Emitter.h"
+
+#ifndef CORRADE_BUILD_STATIC
+    #ifdef InterconnectTestEmitterLibrary_EXPORTS
+        #define CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_EXPORT CORRADE_VISIBILITY_EXPORT
+    #else
+        #define CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_EXPORT CORRADE_VISIBILITY_IMPORT
+    #endif
+    #define CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_INLINE_MEMBER_EXPORT CORRADE_VISIBILITY_INLINE_MEMBER_EXPORT
+#else
+    #define CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_EXPORT CORRADE_VISIBILITY_STATIC
+    #define CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_INLINE_MEMBER_EXPORT CORRADE_VISIBILITY_STATIC
+#endif
+
+namespace Corrade { namespace Interconnect { namespace Test {
+
+struct CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_EXPORT EmitterLibrary: Emitter {
+    void fireInlineThroughAFunction();
+    void fireNonInlineThroughAFunction();
+
+    CORRADE_INTERCONNECT_TESTEMITTERLIBRARY_INLINE_MEMBER_EXPORT Signal fireInline() {
+        return emit(&EmitterLibrary::fireInline);
+    }
+
+    Signal fireNonInline();
+};
+
+}}}
+
+#endif
diff --git a/src/Corrade/Interconnect/Test/LibraryTest.cpp b/src/Corrade/Interconnect/Test/LibraryTest.cpp
@@ -0,0 +1,68 @@
+/*
+    This file is part of Corrade.
+
+    Copyright © 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016,
+                2017, 2018, 2019 Vladimír Vondruš <mosra@centrum.cz>
+
+    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 "Corrade/Interconnect/Test/EmitterLibrary.h"
+#include "Corrade/TestSuite/Tester.h"
+
+namespace Corrade { namespace Interconnect { namespace Test { namespace {
+
+struct LibraryTest: TestSuite::Tester {
+    explicit LibraryTest();
+
+    void test();
+};
+
+LibraryTest::LibraryTest() {
+    addTests({&LibraryTest::test});
+}
+
+void LibraryTest::test() {
+    EmitterLibrary e;
+
+    int fired = 1;
+    connect(e, &EmitterLibrary::fireInline, [&fired]() { fired *= 2; });
+    connect(e, &EmitterLibrary::fireNonInline, [&fired]() { fired *= 3; });
+
+    e.fireNonInline();
+    CORRADE_COMPARE(fired, 3);
+
+    e.fireInline();
+    CORRADE_COMPARE(fired, 6);
+
+    e.fireNonInlineThroughAFunction();
+    CORRADE_COMPARE(fired, 18);
+
+    {
+        #ifdef __MINGW32__
+        CORRADE_EXPECT_FAIL("Inline member functions are duplicated inside and outside of the DLL when under MinGW.");
+        #endif
+        e.fireInlineThroughAFunction();
+        CORRADE_COMPARE(fired, 36);
+    }
+}
+
+}}}}
+
+CORRADE_TEST_MAIN(Corrade::Interconnect::Test::LibraryTest)
