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

[Vadim Zeitlin]

On Wed, 8 Mar 2017 04:25:32 +0000 Greg Chicares <address@hidden> wrote:

GC> [Vadim--The mysticity is my only problem; maybe you can explain it?]

 I'm not sure if this is going to satisfy you, but my explanation is that
MinGW auto-import heuristics don't seem to work well when explicit DLL
import declarations are also used and so it's unwise to rely on
auto-importing in "so_test" build where LMI_SO has a non-empty expansion.

GC> I run this test less often than I should, and today it failed when I
GC> ran an extended series of tests in preparation for declaring a release
GC> candidate:
GC> 
GC> /opt/lmi/src/lmi[0]$make $coefficiency all build_type=so_test 
USE_SO_ATTRIBUTES=1
GC> ...
GC> skeleton.o: In function `~ce_skin_name':
GC> /opt/lmi/src/lmi/ce_skin_name.hpp:46: undefined reference to `vtable for 
ce_skin_name'
GC> skeleton.o: In function `~datum_string':
GC> /opt/lmi/src/lmi/datum_string.hpp:44: undefined reference to `vtable for 
datum_string'
GC> /opt/lmi/src/lmi/datum_string.hpp:44: undefined reference to `vtable for 
datum_string'
GC> collect2: error: ld returned 1 exit status
GC> /opt/lmi/src/lmi/workhorse.make:794: recipe for target 'skeleton.dll' failed

 I can indeed reproduce this if I revert 
51473f535f22e9036e96fc6be1ec96dc222ba517


<git digression>

GC> But that didn't seem to help, so I instructed my machine to solve the
GC> problem by brute force[0]. (It took less than five minutes to decide
GC> that 84cd25c was the problem.)

 Yes, sometimes I really feel that git spoils me, it's faster to just use
it to find the problem instead of spending time to think about it. If
robots are really going to take our jobs in the future, I'll reminiscence
in my memoires that Git was the first step on this road...

GC> so, there being no git command to revert selected files...
GC> 
GC> 
http://git.661346.n2.nabble.com/Revert-a-single-commit-in-a-single-file-td6064050.html
GC> | I am afraid that it would lead to encouraging people to record a
GC> | horribly broken history
GC> 
GC> ...I found a command to do it anyway:
GC> 
GC> /opt/lmi/src/lmi[0]$for z in preferences_model.?pp; do git show 84cd25c:$z 
> $z; done

 This doesn't really revert the changes to these files done in 84cd25c, but
revert the files to their state in this commit, which could be done simpler
with just

        $ git checkout 84cd25c -- preferences_model.?pp

To really revert changes to some files you could have used "git show $SHA1
-- some_files | git apply -R".

</git digression>


GC> and with that change everything "worked"--now the test:
GC> 
GC> /opt/lmi/src/lmi[0]$make $coefficiency all build_type=so_test 
USE_SO_ATTRIBUTES=1 2>&1 |less -S
GC> 
GC> is clean.

 I can confirm that 51473f535f22e9036e96fc6be1ec96dc222ba517 works, but I
think it works by chance only, i.e. it somehow avoids triggering whichever
bug prevents auto-importing from working without it, and the proper thing
to do would be to revert 51473f535f22e9036e96fc6be1ec96dc222ba517 (i.e.
unrevert the original change) and apply the following patch instead:
---------------------------------- >8 --------------------------------------
commit a401a23ba29f302143367b5ce2931689f88d0f5f
Author: Vadim Zeitlin <address@hidden>
Date:   Wed Mar 8 23:29:10 2017 +0100

    Add missing LMI_SO to declarations of classes in lmi DLL

    When using explicit DLL import/export declarations, i.e. in the "so_test"
    build kind, all DLL-exported classes should be using LMI_SO, otherwise using
    them might still work due to auto-importing, but this seems to be very 
fragile
    and can stop working even due to completely unrelated changes, so avoid
    relying on it.

diff --git a/ce_skin_name.hpp b/ce_skin_name.hpp
index 2f64306..cf2f01e 100644
--- a/ce_skin_name.hpp
+++ b/ce_skin_name.hpp
@@ -43,7 +43,7 @@
 /// to end users, but skin names are more esoteric and it is less
 /// confusing to show them as file names rather than apparent phrases.

-class ce_skin_name
+class LMI_SO ce_skin_name
     :public mc_enum_base
     ,private boost::equality_comparable<ce_skin_name,ce_skin_name>
     ,private boost::equality_comparable<ce_skin_name,std::string>
diff --git a/datum_string.hpp b/datum_string.hpp
index 5457a5b..ba57a77 100644
--- a/datum_string.hpp
+++ b/datum_string.hpp
@@ -34,7 +34,7 @@

 // Implicitly-declared special member functions do the right thing.

-class datum_string
+class LMI_SO datum_string
     :public datum_base
     ,private boost::equality_comparable<datum_string,datum_string>
 {
diff --git a/mc_enum.hpp b/mc_enum.hpp
index f9a35fd..b183388 100644
--- a/mc_enum.hpp
+++ b/mc_enum.hpp
@@ -92,7 +92,7 @@ class LMI_SO mc_enum_base
 /// explained in the documentation for class mc_enum_data.

 template<typename T>
-class mc_enum
+class LMI_SO mc_enum
     :public mc_enum_base
     ,private boost::equality_comparable<mc_enum<T>,mc_enum<T>>
     ,private boost::equality_comparable<mc_enum<T>,T>
diff --git a/tn_range.hpp b/tn_range.hpp
index 5e32a99..f563a76 100644
--- a/tn_range.hpp
+++ b/tn_range.hpp
@@ -228,7 +228,7 @@ class LMI_SO tn_range_base
 /// the right thing.

 template<typename Number, typename Trammel>
-class tn_range
+class LMI_SO tn_range
     :public tn_range_base
     ,private boost::totally_ordered    <tn_range<Number,Trammel>>
     ,private boost::equality_comparable<tn_range<Number,Trammel>,Number>
---------------------------------- >8 --------------------------------------

 This adds two more LMI_SO which fix the other errors I initially had
because I started building with "optimization_flag=-O0 debug_flag=''" on
make command line, hoping that it would build faster. As often, my attempts
to save time turned out to waste a lot more of it however as I got

rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x10):
 undefined reference to `mc_enum<rounding_style>::read(std::istream&)'
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'
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]'
collect2.exe: error: ld returned 1 exit status
/opt/lmi/src/git/workhorse.make:794: recipe for target 'skeleton.dll' failed

and spent time trying to understand why was I getting it when you were not.
When I finally understood that it was due to a missing LMI_SO and added it,
I also got

skeleton.o:skeleton.cpp:(.text$_ZN12datum_stringD1Ev[__ZN12datum_stringD1Ev]+0xe):
 undefined reference to `vtable for datum_string'
skeleton.o:skeleton.cpp:(.rdata$_ZTV8tn_rangeIi19nonnegative_trammelIiEE[__ZTV8tn_rangeIi19nonnegative_trammelIiEE]+0x10):
 undefined reference to `tn_range<int, nonnegative_trammel<int> 
>::read(std::istream&)'
C:/opt/lmi/MinGW-4_9_1/bin/../lib/gcc/i686-w64-mingw32/4.9.1/../../../../i686-w64-mingw32/bin/ld.exe:
 skeleton.o: bad reloc address 0x10 in section 
`.rdata$_ZTV8tn_rangeIi19nonnegative_trammelIiEE[__ZTV8tn_rangeIi19nonnegative_trammelIiEE]'
collect2.exe: error: ld returned 1 exit status
/opt/lmi/src/git/workhorse.make:794: recipe for target 'skeleton.dll' failed

which explains the other LMI_SO I added.


 So, with this patch and without 51473f535f22e9036e96fc6be1ec96dc222ba517,
things still link and seem to work correctly for me. However it still
doesn't answer your question about what could be done to prevent the
occurrence of such problems in the future. I thought the answer would be to
use --disable-auto-import in LDFLAGS in the USE_SO_ATTRIBUTES case, but
unfortunately this doesn't work at all: if I do it, I get tons of link
errors about "undefined reference to `typeinfo for char const*'" and many
other built-in or standard library symbols, so apparently linking with
libstdc++ relies on auto-import. If this is correct, I'm not sure what else
could be done other than, indeed, running this test more often (which would
be best suited by setting up continuous integration for lmi, as I'd really
like to do). But maybe there is some way around this, I'll try to look at
this again later.

 Regards,
VZ