Re: [lmi] MinGW 3.4.5, stdio formatting functions and multiple definitions

[Greg Chicares]

[I gather that I should answer this message first.]

On 2014-12-06 23:00Z, Vadim Zeitlin wrote:
> 
>  There is a problem with the use of std::sprintf() and other formatting
> functions with MinGW 3.4.5. There is a comment obliquely referring to this
> problem in workhorse.make:

[on the next line, "This library" refers to boost regex]

>       # This library also seems to require
>       #   -Wl,--allow-multiple-definition
>       # for reasons unknown, at least with MinGW gcc-3.4.5 .
> 
> and I'd like to explain (to some degree) these reasons as well as discuss
> how could we avoid them.

[snip minimal example]

> IOW, std::sprintf() can't be used in more than one file in the entire
> program when its MinGW-specific version is used. The choice of MinGW
> version, rather than Microsoft one, used by default, is due to the use of
> "-posix", as per http://article.gmane.org/gmane.comp.gnu.mingw.user/27539
> also referenced from workhorse.make.

Yes, and the purpose is only to get a correct snprintf()
(not sprintf(), which lmi deliberately doesn't use):

  # Use a correct snprintf() implementation:
  #   http://article.gmane.org/gmane.comp.gnu.mingw.user/27539
  cxx_standard += -posix

But we can't apply '-posix' only to snprintf() and not to sprintf().

> And I can confirm that any of the ways
> to choose it mentioned there (e.g. -std=c++98, -D__USE_MINGW_ANSI_STDIO=1
> and so on) is sufficient to reproduce the bug, i.e. it's not really
> "-posix" fault.
> 
>  I can also say that the problem does *not* happen in any of the following
> cases:
> 
> (1) Including <stdio.h> and using ::sprintf() instead of std::sprintf().
> 
> (2) Adding "using namespace std;" directive and then using std::sprintf()
>     without explicitly qualifying it.
> 
> (3) Using a custom function with a using declaration for it inside a
>     namespace paralleling the way the standard headers do it with sprintf.
> 
> The combination of (2) and (3) makes me pretty sure that it's some sombre
> compiler bug because there is no way "using namespace std" + "sprintf"
> could behave differently from "std::sprintf" on purpose. Yet, in the former
> case a static sprintf() is emitted into the object file when compiling
> while in the latter one, an extern function is generated.
> 
>  Unfortunately I didn't find any way to work, or even hack, around this
> bug and I don't know what else to try (I've already tried a few things I
> would be ashamed to mention in a polite company of C++ programmers, but
> they didn't work anyhow).

I was about to suggest approaching it that way, but I guess it's doomed.

>  But I don't think we should leave it like this neither. While lmi itself
> doesn't use std::sprintf() anywhere, boost.regex library does and this is
> what originally costed me a lot of grief as I couldn't understand why I was
> getting link errors when I started using it in the GUI test. It finally
> turned out (and yes, I could have found this sooner, but I'm also pretty
> confident that I could have wasted even more time on this as well...) that
> to use it, I had to add --allow-multiple-definition linker option, as the
> code already using boost.regex did -- and this is what I did for my patch. 
> However, again, this can't be the right solution. Even if we prominently
> document that using boost.regex requires this linker option (and I don't
> even see where/how this could be done), it's just a bad idea to use it in
> the first place as it could make other errors and potentially result in
> run-time problems which would be much more difficult to debug than
> link-time ones.

I think I should nevertheless commit your about-dialog patch with celerity.
But I agree that we should fix the underlying problem, and remove the
'--allow-multiple-definition' kludge already in place.

>  Hence I'd like to suggest patching boost.regex to use just sprintf()
> instead of std::sprintf(). This is not ideal neither, but the worst problem
> I see with this is maintenance: normally, we would need to update this
> patch when we upgrade to a newer version of boost.regex and in this
> particular case this problem probably will never happen as we will
> hopefully update to a newer compiler (which doesn't have this bug at all)
> before/together with upgrading boost.regex.

That problem will never happen. Not "probably never": no, never.
We cannot upgrade to a newer boost version as long as we use
this practically unmaintained compiler. The MinGW project lead
uses gcc-3.4.5 himself, and advises everyone not to use the
very latest version that they released a couple years ago
because it is apparently broken in a way that cannot be fixed.

>  So far I've just quickly tested that doing this indeed works, i.e. we can
> omit -Wl,--allow-multiple-definition and everything still links (and runs)
> successfully if we patch boost.regex to use ::sprintf(). Is it worth it to
> prepare a real patch to lmi doing this

Yes. I suggest, in 'install_miscellanea.make':

 .PHONY: boost
 boost: $(file_list)
+       -[ -e $(stem).patch ] && $(PATCH) --directory=scratch --strip=1 < 
$(stem).patch
        @$(MKDIR) $(third_party_include_dir)/boost/
        $(CP) --force --preserve --recursive scratch/$(stem)/boost/* 
$(third_party_include_dir)/boost/

(where the added line is already used for 'xmlwrapp', and already
adapts itself to the target name). Then, if you create a patch
'boost_1_33_1.patch' that handles sprintf appropriately, using
the procedure documented under the rubric
  # Patches were generated according to this advice:
in the same makefile (which I believe is just the canonical way
linux patches are created), then it should all just work.

And the patch will never be applied for any boost version but 1.33.1 .
It "expires" of its own accord without any action on our part as soon
as we upgrade boost.

> and maybe also looking at mpatrol
> which also currently uses --allow-multiple-definition (but probably for
> some other reason and, anyhow, using it for compilation of third party code
> doesn't bother me that much, it's only using it for our own code that does)?
> I'd really like to do it because using --allow-multiple-definition is IMO a
> rather dangerous hack, but I'll wait for your reply before continuing with
> this.

No, I think working on mpatrol is not a good use of your time.
Our use of it became broken years ago, and if it were easy to
fix, we'd have fixed it.