Re: [lmi] Mystic fix for so-attributes linker error

[Vadim Zeitlin]

On Thu, 9 Mar 2017 02:46:59 +0000 Greg Chicares <address@hidden> wrote:

GC> That is most helpful. I had forgotten that, unlike the mingw.org toolchain,
GC> MinGW-w64 pretty much forces us to enable that kludge, as your attempt to
GC> turn it off (below) demonstrates. These LMI_SO attributes should make
GC> everything work in the absence of that kludge, but then we can't use
GC> MinGW-w64's libstdc++.

 After some more tests, I realized that we could use -static-libstdc++ gcc
option to avoid this problem. However disabling auto import still doesn't
work, with
---------------------------------- >8 --------------------------------------
diff --git a/workhorse.make b/workhorse.make
index d4f28a4..5878aa3 100644
--- a/workhorse.make
+++ b/workhorse.make
@@ -675,6 +675,7 @@ endif
 
 ifneq (,$(USE_SO_ATTRIBUTES))
   actually_used_lmi_so_attributes = -DLMI_USE_SO_ATTRIBUTES 
$(lmi_so_attributes)
+  LDFLAGS += -Wl,--disable-auto-import -static-libstdc++
 endif
 
 REQUIRED_CPPFLAGS = \
---------------------------------- >8 --------------------------------------
I get the following errors:

main_wx_test.o: In function `~DocManagerEx':
/opt/lmi/src/git/docmanager_ex.hpp:47: undefined reference to `vtable for 
DocManagerEx'
/opt/lmi/src/git/docmanager_ex.hpp:47: undefined reference to `vtable for 
DocManagerEx'
main_wx_test.o: In function `~Skeleton':
/opt/lmi/src/git/skeleton.hpp:57: undefined reference to `vtable for Skeleton'
/opt/lmi/src/git/skeleton.hpp:57: undefined reference to `vtable for Skeleton'
/opt/lmi/src/git/skeleton.hpp:57: undefined reference to `vtable for Skeleton'
/opt/lmi/src/git/skeleton.hpp:57: undefined reference to `vtable for Skeleton'
/opt/lmi/src/git/skeleton.hpp:57: undefined reference to `vtable for Skeleton'
main_wx_test.o:/opt/lmi/src/git/skeleton.hpp:57: more undefined references to 
`vtable for Skeleton' follow
collect2.exe: error: ld returned 1 exit status
/opt/lmi/src/git/workhorse.make:791: recipe for target 'wx_test.exe' failed
make[1]: *** [wx_test.exe] Error 1
make[1]: Target 'all' not remade because of errors.

On one hand, this is somewhat surprising because lmi_wx_shared.exe does get
linked successfully and I don't see any important differences between the
command used for linking it and for linking wx_test.exe. But OTOH it looks
perfectly normal because both of these vtables are inside skeleton.dll and
are not DLL-exported from it, so using them should fail without
auto-import. I.e. the surprising part is actually that linking
lmi_wx_shared.exe succeeds, not that linking wx_test.exe fails.

 I feel like I'm still missing something here, so I'll return to this
later, unless you think it's not worth it considering the other ideas
discussed below.


GC> The first step was the Jacquard loom, sans lequelle vous n'auriez rien qui
GC> fit ressouvenir de la gloire de la France [to misquote Arouet].

 Oh, I didn't know this one nor about the duck at all. What a wonderful
story.


GC> I'm leery of running git-checkout with a named SHA1--I did it once, and it
GC> detached my head, or something like that.

 Luckily, with Git, detaching head is less fatal than it was historically
in France. In fact, doing it is quite convenient if you want to test a
particular commit and it's enough to checkout the branch you want to get
back to (i.e. "master" in lmi case as there are no others) to unguillotine
oneself.


GC> That is, of course, guaranteed to work. Probably we could just
GC>   s/class/class LMI_SO/
GC> globally

 I don't think this can work because there are also classes which are not
part of liblmi.dll but are compiled into skeleton.dll, e.g. DocManagerEx
appearing in the error messages above. Using LMI_SO with this class would
be a mistake as it would end up being DLL-imported on use and even on
definition (because skeleton.dll is compiled with -DLMI_USE_SO), which is
definitely wrong.

 In fact, it's really not clear to me how can a single LMI_SO work with
more than one DLL. I believe we really need per-DLL macros, i.e. add
LMI_SKELETON_SO governed by LMI_SKELETON_{BUILD,USE}_SO, just as the
existing LMI_SO value is determined by LMI_{BUILD,USE}_SO. Or am I missing
something here?

GC> and then the problem could never arise again; but that's
GC> exactly what we want to avoid doing, because the goal is to keep
GC> shared-library internals hidden.

 Yes, I agree that we wouldn't want to use LMI_SO on all classes
indiscriminately anyhow. But even if we did want it, it wouldn't work
anyhow currently.

GC> I used to have various versions of two non-free compilers, but they
GC> wouldn't work with C++11. Do you have other compilers with which
GC> you could test this, without MinGW-w64's compulsory 'auto-import'?

 I use only static libraries in my MSVC build precisely in order not to
have to deal with these DLL complications. But I could change/add a DLL
build which could be used to check this, of course. This would be a good
test, I think, because MSVC doesn't do any auto-linking and is perfectly
picky about mixed up DLL import/export declarations. Should I do this?

 But I also thought of something else: we could also change the default
visibility to hidden for the Unix builds. This should expose all the
problems due to missing/wrong LMI_SO usage too because ELF linker doesn't
do (nor even could do) any auto-linking neither. Should I do this instead
(or in addition to)?

 Of course, please keep in mind that testing either of those approaches
will almost certainly result in errors right now, that I will want to fix
and submit patches fixing them to you. So there is also a question of
knowing whether you want to have to deal with discussing/applying them
right now. I do think that the danger of breaking anything is very small
here, i.e. if it still compiles and links, it really should work in exactly
the same way as before too.


GC> > make command line, hoping that it would build faster. As often, my 
attempts
GC> > to save time turned out to waste a lot more of it however as I got
GC> > 
GC> > 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x10):
 undefined reference to `mc_enum<rounding_style>::read(std::istream&)'
GC> > 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x14):
 undefined reference to `mc_enum<rounding_style>::write(std::ostream&) const'
GC> > 
C:/opt/lmi/MinGW-4_9_1/bin/../lib/gcc/i686-w64-mingw32/4.9.1/../../../../i686-w64-mingw32/bin/ld.exe:
 rounding_document.o: bad reloc address 0x14 in section 
`.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]'
GC> > collect2.exe: error: ld returned 1 exit status
GC> > /opt/lmi/src/git/workhorse.make:794: recipe for target 'skeleton.dll' 
failed
GC> 
GC> Trying that here, I get:
GC> 
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x10):
 undefined reference to `mc_enum<rounding_style>::read(std::istream&)'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x14):
 undefined reference to `mc_enum<rounding_style>::write(std::ostream&) const'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x18):
 undefined reference to `mc_enum<rounding_style>::all_strings() const'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x1c):
 undefined reference to `mc_enum<rounding_style>::cardinality() const'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x20):
 undefined reference to `mc_enum<rounding_style>::enforce_proscription()'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x24):
 undefined reference to `mc_enum<rounding_style>::ordinal() const'
GC> 
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x28):
 undefined reference to `mc_enum<rounding_style>::str(int) const'

 It's really strange that we're not getting the same results. I have no
idea why I saw only the first 2 symbols and this mysterious "bad reloc"
error afterwards which, apparently, prevented me from seeing all the other
ones.

GC> but what that suggests to me is that mc_enum is appropriately hidden,
GC> but rounding_document.o accesses mc_enum's internals inappropriately.
GC> In that case, breaking encapsulation globally would be a shame.

 AFAICS the problem is that mc_enum_base is DLL-exported but mc_enum<>
itself is not. If we want to encapsulate all the methods above in
liblmi.dll, then we shouldn't export mc_enum_base from it neither. But
currently virtual methods such as all_strings(), declared with LMI_SO in
the base class, but defined without it in the derived one, seem to create
problems.

GC> Perhaps I should finally start doing routine GTK builds here, with ELF
GC> visibility attributes.

 Notice that currently this wouldn't help because we don't use
-fvisibility=hidden yet.

GC> At the moment, using MinGW-w64 only, I'm trying to maintain
GC> encapsulation that I can no longer test. I still think it's
GC> a good idea to maintain the strongest possible physical separation at
GC> shared-library barriers. I've put considerable effort into designing
GC> lmi this way, and I don't want to throw it overboard piece by piece.

 FWIW I agree, but with an important caveat: if it's important, it should
be actually used, preferably by default. I.e. I think that we should either
make USE_SO_ATTRIBUTES=1 build the default or drop it completely (the
latter would be undesirable but IMO still better than having half-tested
almost never used build mode which is just confusing).

 Why don't we use USE_SO_ATTRIBUTES=1 (and, ideally, --disable-auto-import)
by default?
VZ