microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.05k stars 1.48k forks source link

special_math.cpp: Statically linked library contains and depends on Boost symbols #362

Open codicodi opened 4 years ago

codicodi commented 4 years ago

Description Boost symbols embedded in static library can conflict with user code and/or custom boost installations.

Example 1 In this example one can override behaviour of standard library function using custom boost symbol.

PS G:\dev\test> type .\test.cpp
#include <cmath>
#include <cstdio>

namespace boost::math {
template<typename T>
T zeta(T)
{
    return 0;
}
}

int main()
{
    auto a = boost::math::zeta(10.0);
    auto b = std::riemann_zeta(10.0);
    std::printf("boost = %f\nstd   = %f\n", a, b);
    return 0;
}
PS G:\dev\test> cl -EHsc -std:c++17 -MDd -nologo .\test.cpp
test.cpp
PS G:\dev\test> .\test.exe
boost = 0.000000
std   = 1.000995
PS G:\dev\test> cl -EHsc -std:c++17 -MTd -nologo .\test.cpp
test.cpp
PS G:\dev\test> .\test.exe
boost = 0.000000
std   = 0.000000
PS G:\dev\test> cl
Microsoft (R) C/C++ Optimizing Compiler Version 19.24.28314 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

Playing with different compiler flags I also got following output once:

boost = 1.000995
std   = 1.000995

but I forgot exact combination and wasn't able to reproduce it later.

Example 2 In this example one can link to boost symbols contained in STL.

PS G:\dev\test> type .\test.cpp
#include <cmath>
#include <cstdio>

namespace boost::math {
template<typename T>
T zeta(T);

extern template double zeta<double>(double);
}

int main()
{
    auto a = boost::math::zeta(10.0);
    std::printf("boost = %f\n", a);
    return 0;
}
PS G:\dev\test> cl -EHsc -std:c++17 -MT -nologo .\test.cpp
test.cpp
PS G:\dev\test> .\test.exe
boost = 1.000995
PS G:\dev\test> cl -EHsc -std:c++17 -MD -nologo .\test.cpp
test.cpp
test.obj : error LNK2019: unresolved external symbol "double __cdecl boost::math::zeta<double>(double)" (??$zeta@N@math@boost@@YANN@Z) referenced in function main
test.exe : fatal error LNK1120: 1 unresolved externals

Expected behavior STL shouldn't expose any third-party symbols to user code in order to avoid conflicts.

StephanTLavavej commented 4 years ago

I agree that this is a bug. It's also a potential source of ODR violations, since users will often be using versions of Boost other than the exact one we used to build with, and Boost doesn't attempt to provide ABI stability.

@CaseyCarter, I think we may have to use bcp to rename the Boost namespace when building.

AlexGuteniev commented 4 years ago

I would recommend using bcp to make a renamed copy of sources, and put them somewhere in the repository.

This way the version of Boost will be fixed, instead of whatever .\vcpkg.exe install boost-math:x86-windows boost-math:x64-windows would obtain. So no worries about Boost roadmap.

Also it will simplify joining by removing a half of the build instructions.

Finally, it would help me. Because on my main project there is older boost in the repository, and somehow vcpkg-integrated boost 1.7x affects build in very interesting (probably ABI-breaking) way by linking in. Not sure if it is misconfiguration or vcpkg bug, currently working around by vcpkg integrate install / vcpkg integrate remove

CaseyCarter commented 4 years ago

Also it will simplify joining by removing a half of the build instructions.

We've been wavering back and forth on how to best handle this bug. Boost is pretty big: extracting boost::math with BCP produces a directory with 135MB of sources, whereas our entire repo right now is ~441MB (without submodules). We'd rather not carry those sources if we can avoid it.

"Just include the sources" does solve the three major issues we have:

  1. this one (BCP makes it trivial to change the top-level namespace to _Boost),
  2. it breaks the STL<=>boost dependency cycle which makes it hard to deploy the STL usefully in vcpkg, and
  3. it makes the sources from boost::math that the special_math test wants access to available in a convenient known location.
StephanTLavavej commented 4 years ago

I would also strongly prefer to avoid checking Boost sources into our tree. Extracting Boost and running bcp as part of our build step would be preferable, if possible.

AlexGuteniev commented 2 years ago

Does this bug still exist?

CaseyCarter commented 2 years ago

Yes it does.