Re: [lmi] boost::filesystem anomaly?

[Greg Chicares]

On 2016-06-11 17:47, Vadim Zeitlin wrote:
> On Sat, 11 Jun 2016 17:03:00 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2016-06-11 15:48, Vadim Zeitlin wrote:
> ...
> GC> > trouble is that there are 2 copies of boost_filesystem library in the
> GC> > lmi_wx executable. Because of this, fs::no_check used above is not the
> GC> > same no_check object that the no_check defined in path_posix_windows.cpp
> GC> > that the code in path::m_path_append() compares the checker with, hence 
> the
> GC> > check for Windows paths is completely skipped, resulting in the bug.
> GC> 
> GC> I'm really glad you found that,
> 
>  Actually, I found wrong, sorry. There is a single function in liblmi.dll,
> but the trouble is that there is the real function and its import library
> trampoline, which, of course, has a different address. And someone had
> found it more than 10 years before me, see
> 
>       https://sourceforge.net/p/mingw/mailman/message/16207966/

Following the links leads to:

http://lists.boost.org/boost-users/2005/12/15632.php
| A major upgrade to the filesystem library now on the i18n branch will get
| merged into CVS HEAD in a few days. Once that happens it should hopefully be
| easier to track down the Mingw problems if they still exist.

We're using this:

  /opt/lmi/third_party/src/boost/libs/filesystem/src[0]$ls -l

...
  -rw-r--r-- 1 earl None 30136 Oct 26  2005 operations_posix_windows.cpp


and the message quoted above is dated 2005-12-05, a few weeks later.

This follow-up message
  http://lists.boost.org/boost-users/2005/12/15643.php
includes a patch that looks like it might solve our problem:
  http://lists.boost.org/boost-users/att-15643/patch_native.diff
but it looks like someone else already found that...

> GC> Okay, I'll wait for your fix.
> 
>  The problem is that I have 2 ideas about how to fix this and I'm not sure
> which one would be least worst (I can't really call neither of them "best"
> with straight face). The first one is to define BOOST_FILESYSTEM_DYN_LINK
> and BOOST_FILESYSTEM_SOURCE when building liblmi.dll and also define the
> former when building lmi_wx target. This should solve the problem because
> a pointer to a DLL-exported function really does point to it (and not to
> the trampoline used to jump to it), but is rather brittle and a little
> ugly.
> 
>  The second one is to modify Boost.Filesystem sources to add an accessor
> for no_check, e.g. something similar to this patch:
> 
>       http://lists.boost.org/boost-users/att-15643/patch_native.diff
> 
> linked from a thread about the same problem on Boost mailing list. This is
> arguably simpler, but more error-prone as it would still be possible to use
> no_check directly (and wrongly).
> 
>  On balance, as much as I don't like modifying lmi build system, I think
> the first solution is preferable. What do you think?

I think we should wait and see whether we need to fix this at all.
If we don't, then our time is better spent on modernization: a current
release of boost would remove this problem.

> GC> # These object files are used in both an application and a shared
> GC> # library that it links to, only for builds that use shared-library
> GC> # 'attributes'. This workaround is used merely because we don't yet
> GC> # build these objects as a library. TODO ?? The duplication is not
> GC> # correct: it validates linking, but the linked applications don't
> GC> # run correctly.
[...snip discussion...]
>  Yes, I saw this too and had the same thought and arrived to the same
> conclusion. I still don't understand the comment nor the meaning and
> intended usage of USE_SO_ATTRIBUTES in lmi more globally...

I can explain the last part. A long, long time ago, MinGW implemented
direct linking to DLLs with ld's '-l' option. Even though I had a whole
gigabyte of RAM...
  http://permalink.gmane.org/gmane.comp.gnu.mingw.user/14753
the reduction in RAM use was important on machines in the office that
were less bounteously endowed, and that's why lmi uses direct linking.
But I've carefully preserved and maintained the shared-library attributes
because they should work even with msw compilers that don't link DLLs
directly, and because visibility attributes should benefit ELF.

> GC> >  BTW, if we updated to a newer (and more std::filesystem-compatible)
> GC> > version of the Boost.Filesystem library which doesn't have the concept 
> of
> GC> > "checkers" any more, we wouldn't have to spend time on dealing with this
> GC> > problem...
> GC> 
> GC> I think we should replace boost::regex with std::regex first (and test
> GC> the replacement).
> 
>  FWIW replacing boost::regex with std::regex is trivial, they have exactly
> the same API. However a couple of regexes will need to be adjusted because
> C++11 std::regex doesn't support Perl extended syntax for some reason. A
> simple grep finds only 4 occurrences of it and 3 of them use the syntax
> which is supported by ECMAScript regexes and hence C++11, the only problem
> is the use of "(?-s)" to turn off the "s" flag in regex_test.cpp, so this
> test would need to be modified to avoid using it, but it shouldn't be a
> problem.

Since you know how to grep for perl regexes, I think I'd better leave
this task to you.

> GC> Then we'd be using only one non-header boost library,
> GC> filesystem--unless they've changed other header-only libraries we use
> GC> to require separate compilation.
> 
>  No, they didn't. But while I'm all for getting rid of Boost.Regex, there
> is no particular urgency to it. Updating Boost.Filesystem would, however,
> fix (well, avoid) the problem that we're currently facing.

As this comment in 'objects.make' explains...

# Boost filesystem and regex libraries. The other boost libraries that
# lmi requires are implemented entirely in headers.
#
# As for listing the object files here, the regex author says:
#   http://groups.google.com/group/boost-list/msg/7f925ca50d69384b
# | add the libs/regex/src/*.cpp files to your project

...lmi compiles the sources for two libraries. AIUI, it's really not
feasible to update boost piecemeal, so it doesn't seem efficient to
upgrade boost::regex (and update 'objects.make' accordingly) only to
replace it with std::regex. That's why I was thinking that we should
switch to std::regex before we consider updating boost.