From cec78fdc58b193ff70fc1c159c2c04e07e709dd4 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 29 Sep 2013 01:08:15 +0200 Subject: [PATCH 1/7] bughunt: extract a call sequence leading to a strange init error with Clang Observations: - the initial observation was that we get two instances of the config rules service - obviously this it is *not* the initialisation of a static variable accessed from multiple compilation units. But the access from two compilation units is crucial - we can exclude the effect of all other initialisation. It *is* in SingletonSubclass - we can exclude the effect of dynamic linking. Even two translation units linked statically exhibit the same problem rebuild this test case in the research area, to be able to verify with various compilers --- research/clang-static-init-1.cpp | 38 +++++++++++++++++++++++++++ research/clang-static-init-2.cpp | 36 ++++++++++++++++++++++++++ research/clang-static-init.hpp | 44 ++++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+) create mode 100644 research/clang-static-init-1.cpp create mode 100644 research/clang-static-init-2.cpp create mode 100644 research/clang-static-init.hpp diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp new file mode 100644 index 000000000..c4869fd65 --- /dev/null +++ b/research/clang-static-init-1.cpp @@ -0,0 +1,38 @@ +#include "lib/test/run.hpp" +#include "proc/hh.hpp" + +#include "proc/config-resolver.hpp" + +#include + +using proc::ConfigResolver; + +using ::test::Test; +using std::cout; +using std::endl; + + +namespace proc { +namespace test { + + class StaticInstance_test : public Test + { + virtual void + run (Arg) + { + ConfigResolver& ref1 = ConfigResolver::instance(); + + ConfigResolver& sub2 = fabricate(); + + cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; + + } + + }; + + /** Register this test class... */ + LAUNCHER (StaticInstance_test, "unit bug"); + + + +}} // namespace proc::test diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp new file mode 100644 index 000000000..ea107ee96 --- /dev/null +++ b/research/clang-static-init-2.cpp @@ -0,0 +1,36 @@ + +#include "proc/hh.hpp" + +#include + +using proc::ConfigResolver; +using std::cout; +using std::endl; + + +namespace proc { +namespace test { + + + int Subject::cnt = 0; + + Subject::Subject() + { + ++cnt; + std::cout << "Subject("< fab2; + } + + + ConfigResolver& + fabricate() + { + return ConfigResolver::instance(); + } + + +}} // namespace proc::test diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp new file mode 100644 index 000000000..c27e5ddf7 --- /dev/null +++ b/research/clang-static-init.hpp @@ -0,0 +1,44 @@ + +#include "lib/error.hpp" + +#include "proc/config-resolver.hpp" + +namespace proc { +namespace test { + + template + struct Holder + { + static T* instance; + + T& + get() + { + if (!instance) + { + instance = new T(); + } + return *instance; + } + }; + + + template + T* Holder::instance; + + struct Subject + { + static int cnt; + + Subject(); + + }; + + + proc::ConfigResolver& fabricate(); + + + + +}} // namespace proc::test + From 10a511d29ca51a4b38bc9df85eb4f9554e7a0ec1 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 30 Sep 2013 00:42:27 +0200 Subject: [PATCH 2/7] bughunt: build testcase statically linked ...but still dynamically linking against the core lib But the actual template instantiations happen now within the two compilation units, which are linked statically. When looking into the symbol table, we can see that the static field is emitted two times readelf -W target/clang-static-init -s | c++filt |less --- research/SConscript | 2 ++ research/clang-static-init-1.cpp | 40 +++++++++++++------------------- research/clang-static-init-2.cpp | 5 ++-- research/clang-static-init.hpp | 4 +--- 4 files changed, 21 insertions(+), 30 deletions(-) diff --git a/research/SConscript b/research/SConscript index cde733bf9..282a4b1e4 100644 --- a/research/SConscript +++ b/research/SConscript @@ -9,10 +9,12 @@ Import('env core support_lib') envR = env.Clone() +envR.Append(CPPPATH='research') # envR.Append(CCFLAGS=' -O3 ') # build additional test and administrative tools.... experiments = [ envR.Program('try', ['try.cpp'] + support_lib) #### to try out some feature... + , envR.Program('clang-static-init', ['clang-static-init-1.cpp', 'clang-static-init-2.cpp'] + core) ] # diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index c4869fd65..8b8cbcb7e 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -1,7 +1,7 @@ #include "lib/test/run.hpp" -#include "proc/hh.hpp" #include "proc/config-resolver.hpp" +#include "clang-static-init.hpp" #include @@ -12,27 +12,19 @@ using std::cout; using std::endl; -namespace proc { -namespace test { +int +main (int, char**) + { + cout << "\n.gulp.\n"; + + ConfigResolver& ref1 = ConfigResolver::instance(); + + ConfigResolver& sub2 = test::fabricate(); + + cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; + + + return 0; + } + - class StaticInstance_test : public Test - { - virtual void - run (Arg) - { - ConfigResolver& ref1 = ConfigResolver::instance(); - - ConfigResolver& sub2 = fabricate(); - - cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; - - } - - }; - - /** Register this test class... */ - LAUNCHER (StaticInstance_test, "unit bug"); - - - -}} // namespace proc::test diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp index ea107ee96..2bf169019 100644 --- a/research/clang-static-init-2.cpp +++ b/research/clang-static-init-2.cpp @@ -1,5 +1,5 @@ -#include "proc/hh.hpp" +#include "clang-static-init.hpp" #include @@ -8,7 +8,6 @@ using std::cout; using std::endl; -namespace proc { namespace test { @@ -33,4 +32,4 @@ namespace test { } -}} // namespace proc::test +} // namespace test diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index c27e5ddf7..65d0a6ef7 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -3,7 +3,6 @@ #include "proc/config-resolver.hpp" -namespace proc { namespace test { template @@ -40,5 +39,4 @@ namespace test { -}} // namespace proc::test - +} // namespace test From 72bd94e1414fe78518245ae523cceb99c4def349 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 30 Sep 2013 00:58:39 +0200 Subject: [PATCH 3/7] bughunt: replace the ConfigResolver with a direct instantiation of Singleton -> MISS thus not lib::Singleton is the culprit, it must be lib::SingletonSubclass --- research/clang-static-init-1.cpp | 8 ++++---- research/clang-static-init-2.cpp | 8 ++++---- research/clang-static-init.hpp | 7 +++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index 8b8cbcb7e..366b53a83 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -1,25 +1,25 @@ #include "lib/test/run.hpp" -#include "proc/config-resolver.hpp" #include "clang-static-init.hpp" #include -using proc::ConfigResolver; using ::test::Test; using std::cout; using std::endl; +using namespace test; int main (int, char**) { cout << "\n.gulp.\n"; - ConfigResolver& ref1 = ConfigResolver::instance(); + Factory fab1; + Subject& ref1 = fab1(); - ConfigResolver& sub2 = test::fabricate(); + Subject& sub2 = test::fabricate(); cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp index 2bf169019..b9a95f1b2 100644 --- a/research/clang-static-init-2.cpp +++ b/research/clang-static-init-2.cpp @@ -3,7 +3,6 @@ #include -using proc::ConfigResolver; using std::cout; using std::endl; @@ -21,14 +20,15 @@ namespace test { namespace { - Holder fab2; +// Holder fab2; + Factory fab2; } - ConfigResolver& + Subject& fabricate() { - return ConfigResolver::instance(); + return fab2(); } diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index 65d0a6ef7..6a0b13da9 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -1,7 +1,8 @@ #include "lib/error.hpp" -#include "proc/config-resolver.hpp" +#include "lib/singleton-subclass.hpp" + namespace test { @@ -33,8 +34,10 @@ namespace test { }; + typedef lib::Singleton Factory; - proc::ConfigResolver& fabricate(); + + Subject& fabricate(); From 0e8a1f1e08e4c24d77d4ffa03df7cc19cff75cc5 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Mon, 30 Sep 2013 02:09:31 +0200 Subject: [PATCH 4/7] bughunt: use a single SingletonSub instance in two compilation units --> HIT basically this reproduces the problem in a simplified setup. Especially note that we're going through a single instance of the factory, yet still this single instance 'sees' two different locations of the class static variable --- research/clang-static-init-1.cpp | 4 +--- research/clang-static-init-2.cpp | 7 ++++--- research/clang-static-init.hpp | 5 ++++- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index 366b53a83..027c4ef1f 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -16,9 +16,7 @@ main (int, char**) { cout << "\n.gulp.\n"; - Factory fab1; - Subject& ref1 = fab1(); - + Subject& ref1 = fab(); Subject& sub2 = test::fabricate(); cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp index b9a95f1b2..8b4c375ba 100644 --- a/research/clang-static-init-2.cpp +++ b/research/clang-static-init-2.cpp @@ -20,15 +20,16 @@ namespace test { namespace { -// Holder fab2; - Factory fab2; + lib::singleton::UseSubclass typeID; } + Factory fab(typeID); + Subject& fabricate() { - return fab2(); + return fab(); } diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index 6a0b13da9..cd4682381 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -34,7 +34,10 @@ namespace test { }; - typedef lib::Singleton Factory; + typedef lib::SingletonSub Factory; + + extern Factory fab; + Subject& fabricate(); From 675b2070ce4f82433e02906fa30964717f939bc1 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 6 Oct 2013 17:37:01 +0200 Subject: [PATCH 5/7] bughunt: attempt to rebuild the problematic structure stand-alone... --> MISS this is only a stripped-down version of the basic singleton, without the indirection. Unsurprisingly, this doesn't exhibit the problem yet --- research/SConscript | 2 +- research/clang-static-init-1.cpp | 4 +--- research/clang-static-init-2.cpp | 6 +++--- research/clang-static-init.hpp | 32 +++++++++++++++++++++++--------- 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/research/SConscript b/research/SConscript index 282a4b1e4..11f58d5c9 100644 --- a/research/SConscript +++ b/research/SConscript @@ -14,7 +14,7 @@ envR.Append(CPPPATH='research') # build additional test and administrative tools.... experiments = [ envR.Program('try', ['try.cpp'] + support_lib) #### to try out some feature... - , envR.Program('clang-static-init', ['clang-static-init-1.cpp', 'clang-static-init-2.cpp'] + core) + , envR.Program('clang-static-init', ['clang-static-init-1.cpp', 'clang-static-init-2.cpp']) ] # diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index 027c4ef1f..7170aa999 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -1,11 +1,9 @@ -#include "lib/test/run.hpp" #include "clang-static-init.hpp" #include -using ::test::Test; using std::cout; using std::endl; @@ -16,7 +14,7 @@ main (int, char**) { cout << "\n.gulp.\n"; - Subject& ref1 = fab(); + Subject& ref1 = fab.get(); Subject& sub2 = test::fabricate(); cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp index 8b4c375ba..80cba771e 100644 --- a/research/clang-static-init-2.cpp +++ b/research/clang-static-init-2.cpp @@ -20,16 +20,16 @@ namespace test { namespace { - lib::singleton::UseSubclass typeID; + // } - Factory fab(typeID); + AccessPoint fab; Subject& fabricate() { - return fab(); + return fab.get(); } diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index cd4682381..f0694fd6b 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -1,12 +1,10 @@ -#include "lib/error.hpp" - -#include "lib/singleton-subclass.hpp" - namespace test { - template + template class Fac + > struct Holder { static T* instance; @@ -16,15 +14,31 @@ namespace test { { if (!instance) { - instance = new T(); + instance = Fac::create(); } return *instance; } }; + template class F + > + T* Holder::instance; + + + template - T* Holder::instance; + struct Factory + { + static T* + create() + { + return new T(); + } + }; + + struct Subject { @@ -34,9 +48,9 @@ namespace test { }; - typedef lib::SingletonSub Factory; + typedef Holder AccessPoint; - extern Factory fab; + extern AccessPoint fab; From 4bd9eeb8ee81d57072c874c2a7b2d1b758abb6ce Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 6 Oct 2013 18:50:53 +0200 Subject: [PATCH 6/7] bughunt: re-create the whole 2nd layer with a configurable product type --> HIT Now we've reprduced the problematic situation in isolation --- research/clang-static-init-1.cpp | 14 +--- research/clang-static-init-2.cpp | 24 ++++--- research/clang-static-init.hpp | 106 +++++++++++++++++++++++++++---- 3 files changed, 109 insertions(+), 35 deletions(-) diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index 7170aa999..020f469b3 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -1,26 +1,18 @@ #include "clang-static-init.hpp" -#include -using std::cout; -using std::endl; - -using namespace test; - int main (int, char**) { - cout << "\n.gulp.\n"; + cout << "\nStart Testcase: invoking two instances of the configurable singleton factory...\n"; - Subject& ref1 = fab.get(); - Subject& sub2 = test::fabricate(); + test::Subject& ref1 = test::fab.get(); + test::Subject& sub2 = test::fabricate(); cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; return 0; } - - diff --git a/research/clang-static-init-2.cpp b/research/clang-static-init-2.cpp index 80cba771e..c23468837 100644 --- a/research/clang-static-init-2.cpp +++ b/research/clang-static-init-2.cpp @@ -1,29 +1,33 @@ #include "clang-static-init.hpp" -#include - -using std::cout; -using std::endl; namespace test { - int Subject::cnt = 0; + int Subject::creationCount = 0; Subject::Subject() { - ++cnt; - std::cout << "Subject("< shall_build_a_Subject_instance; } - AccessPoint fab; + /** + * instance of the singleton factory + * @note especially for this example we're using just \em one + * shared instance of the factory. + * Yet still, the two (inlined) calls to the get() function + * access different addresses for the embedded singleton instance + */ + AccessPoint fab(shall_build_a_Subject_instance); Subject& diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index f0694fd6b..6597e8b05 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -1,54 +1,132 @@ +#include + +using std::cout; + namespace test { - - template class Fac + + + + /* === Layer-1: a singleton factory based on a templated static variable === */ + + template class Fac ///< Policy: actual factory to create the instance > struct Holder { - static T* instance; + static I* instance; - T& + I& get() { if (!instance) { - instance = Fac::create(); + cout << "Singleton Factory: invoke Fabrication ---> instance="<<&instance<<"...\n"; + + instance = Fac::create(); } return *instance; } }; - - template class F > - T* Holder::instance; + I* Holder::instance; - template + + template struct Factory { - static T* + static C* create() { - return new T(); + return new C(); } }; + + + /* === Layer-2: configurable product type === */ + + template + struct Adapter + { + typedef I* FactoryFunction (void); + + static FactoryFunction* factoryFunction; + + + template + static I* + concreteFactoryFunction() + { + return static_cast (Factory::create()); + } + + + template + struct AdaptedConfigurableFactory + { + static X* + create() + { + return (*factoryFunction)(); + } + }; + }; + + /** storage for the per-type shared function pointer to the concrete factory */ + template + typename Adapter::FactoryFunction* Adapter::factoryFunction; + + + + template + struct TypeInfo { }; + + + + /** + * Singleton factory with the ability to configure the actual product type C + * only at the \em definition site. Users get to see only the interface type T + */ + template + struct ConfigurableHolder + : Holder::template AdaptedConfigurableFactory> + { + /** define the actual product type */ + template + ConfigurableHolder (TypeInfo) + { + Adapter::factoryFunction = &Adapter::template concreteFactoryFunction; + } + }; + + + + + + /* === Actual usage: Test case fabricating Subject instances === */ + struct Subject { - static int cnt; + static int creationCount; Subject(); }; - typedef Holder AccessPoint; + typedef ConfigurableHolder AccessPoint; extern AccessPoint fab; From cdb3d3045a7f7949ff22b09e1934911e2c83d260 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 6 Oct 2013 18:58:32 +0200 Subject: [PATCH 7/7] BUG: Clang shows a problem when accessing templated static variable through separate compilation units this is really creepy: the same(!) instance of the singleton factory sees different addresses of the class static variable, depending on the compilation unit. Please note that the type of the concrete factory function is *erased* when exiting the constructor function of ConfigurableHolder --- research/clang-static-init-1.cpp | 16 +++++++++++++--- research/clang-static-init.hpp | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/research/clang-static-init-1.cpp b/research/clang-static-init-1.cpp index 020f469b3..6c473de41 100644 --- a/research/clang-static-init-1.cpp +++ b/research/clang-static-init-1.cpp @@ -2,16 +2,26 @@ #include "clang-static-init.hpp" +test::Subject& +localFunction() +{ + return test::fab.get(); +} + int main (int, char**) { - cout << "\nStart Testcase: invoking two instances of the configurable singleton factory...\n"; + cout << "\nStart Testcase: invoking two instances of the configurable singleton factory...\n\n"; test::Subject& ref1 = test::fab.get(); - test::Subject& sub2 = test::fabricate(); + test::Subject& sub2 = test::fabricate(); ///NOTE: invoking get() from within another compilation unit reveales the problem + test::Subject& sub3 = localFunction(); - cout << "sub1="<< &ref1 << " sub2="<< &sub2 <<"\n"; + cout << "sub1=" << &ref1 + << "\nsub2="<< &sub2 + << "\nsub3="<< &sub3 + << "\n"; return 0; diff --git a/research/clang-static-init.hpp b/research/clang-static-init.hpp index 6597e8b05..272e76d08 100644 --- a/research/clang-static-init.hpp +++ b/research/clang-static-init.hpp @@ -22,7 +22,7 @@ namespace test { { if (!instance) { - cout << "Singleton Factory: invoke Fabrication ---> instance="<<&instance<<"...\n"; + cout << "Singleton Factory: invoke Fabrication ---> address of static instance variable: "<<&instance<<"...\n"; instance = Fac::create(); }