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

[Greg Chicares]

On 2017-03-08 22:53, Vadim Zeitlin wrote:
> 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.

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

> 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...

The first step was sliced bread. Oh, wait, maybe that's US specific.
(Here, "bread" comes in convenient, uniform slices, much like "cheese".)

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

> 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>

Yes, I mis-described it. In this case, there had been no change to the files
in question since the first bad commit, so it still did what I wanted.

I'm leery of running git-checkout with a named SHA1--I did it once, and it
detached my head, or something like that. But thanks for showing the example
of piping git-show into git-apply -R.

> 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,

Yes, I can see no other explanation.

> 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:

[drastically abbreviated]

> -class ce_skin_name
> +class LMI_SO ce_skin_name
> -class datum_string
> +class LMI_SO datum_string
> -class mc_enum
> +class LMI_SO mc_enum
> -class tn_range
> +class LMI_SO tn_range

That is, of course, guaranteed to work. Probably we could just
  s/class/class LMI_SO/
globally and then the problem could never arise again; but that's
exactly what we want to avoid doing, because the goal is to keep
shared-library internals hidden.

IIRC, when we used mingw.org's tools, we kept auto-import turned
off, and these classes didn't need to be visible outside a DLL.
(Except perhaps for ce_skin_name, which is new--but the others
are very old.) If I have remembered correctly, that seems to
suggest that they really should not need to be "exported".

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

>  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

If the visibility attributes are correct, then they should work just
as well without optimization or debug support.

> 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

Trying that here, I get:

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'
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x18):
 undefined reference to `mc_enum<rounding_style>::all_strings() const'
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x1c):
 undefined reference to `mc_enum<rounding_style>::cardinality() const'
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x20):
 undefined reference to `mc_enum<rounding_style>::enforce_proscription()'
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x24):
 undefined reference to `mc_enum<rounding_style>::ordinal() const'
rounding_document.o:rounding_document.cpp:(.rdata$_ZTV7mc_enumI14rounding_styleE[__ZTV7mc_enumI14rounding_styleE]+0x28):
 undefined reference to `mc_enum<rounding_style>::str(int) const'

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

> 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

The explanation here seems to be that preferences_model uses datum_string.
Given that datum_boolean (e.g.) remains encapsulated, datum_string should
also remain so.

> 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.

Perhaps I should finally start doing routine GTK builds here, with ELF
visibility attributes. At the moment, using MinGW-w64 only, I'm trying
to maintain encapsulation that I can no longer test. I still think it's
a good idea to maintain the strongest possible physical separation at
shared-library barriers. I've put considerable effort into designing
lmi this way, and I don't want to throw it overboard piece by piece.