mpark / variant

C++17 `std::variant` for C++11/14/17
https://mpark.github.io/variant
Boost Software License 1.0
659 stars 88 forks source link

Crash under MSVC (VS 2015 update 3) #41

Closed twadrianlis closed 5 years ago

twadrianlis commented 6 years ago

When we have a global object that contains a variant instance that stores a std::unique_ptr the variant creation crashes the program. I tried to illustrate the minimal example and provide a stacktrace of the crash. Am I missing something? I don't think the code does anything illegal?

Crash occurs on MSVC (GCC does not crash);

Callstack:

0000000000000000() (Unknown Source:0)
tests.exe!mpark::detail::visitation::alt::visit_alt<mpark::detail::dtor,mpark::detail::destructor<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > >,1> & __ptr64>(mpark::detail::dtor && visitor, mpark::detail::destructor<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > >,1> & <vs_0>) Line 1212 (d:\dev\contrib\variant.hpp:1212)
tests.exe!mpark::detail::destructor<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > >,1>::destroy() Line 1461 (d:\dev\contrib\variant.hpp:1461)
tests.exe!mpark::detail::assignment<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > > >::emplace<1,std::unique_ptr<int,std::default_delete<int> > >(std::unique_ptr<int,std::default_delete<int> > && <args_0>) Line 1606 (d:\dev\contrib\variant.hpp:1606)
tests.exe!`mpark::detail::assignment<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > > >::assign_alt<1,std::unique_ptr<int,std::default_delete<int> >,std::unique_ptr<int,std::default_delete<int> > >'::`7'::<unnamed-type-impl>::operator()(std::integral_constant<bool,1> __formal) Line 1639 (d:\dev\contrib\variant.hpp:1639)
tests.exe!mpark::detail::assignment<mpark::detail::traits<int,std::unique_ptr<int,std::default_delete<int> > > >::assign_alt<1,std::unique_ptr<int,std::default_delete<int> >,std::unique_ptr<int,std::default_delete<int> > >(mpark::detail::alt<1,std::unique_ptr<int,std::default_delete<int> > > & a, std::unique_ptr<int,std::default_delete<int> > && arg) Line 1650 (d:\dev\contrib\variant.hpp:1650)
tests.exe!mpark::detail::impl<int,std::unique_ptr<int,std::default_delete<int> > >::assign<1,std::unique_ptr<int,std::default_delete<int> > >(std::unique_ptr<int,std::default_delete<int> > && arg) Line 1765 (d:\dev\contrib\variant.hpp:1765)
tests.exe!mpark::variant<int,std::unique_ptr<int,std::default_delete<int> > >::operator=<std::unique_ptr<int,std::default_delete<int> >,0,1,std::unique_ptr<int,std::default_delete<int> >,0>(std::unique_ptr<int,std::default_delete<int> > && arg) Line 1982 (d:\dev\contrib\variant.hpp:1982)
tests.exe!Bar::Bar() Line 214 (d:\dev\tests\core\ConfigTests.cpp:214)
tests.exe!Foo::Foo() Line 221 (d:\dev\tests\core\ConfigTests.cpp:221)
tests.exe!`dynamic initializer for 'instance''() Line 223 (d:\dev\tests\core\ConfigTests.cpp:223)
ucrtbased.dll!00007fffdfb20479() (Unknown Source:0)
tests.exe!__scrt_common_main_seh() Line 223 (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:223)
tests.exe!__scrt_common_main() Line 296 (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:296)
tests.exe!mainCRTStartup() Line 17 (f:\dd\vctools\crt\vcstartup\src\startup\exe_main.cpp:17)
kernel32.dll!00007ff80f091fe4() (Unknown Source:0)
ntdll.dll!00007ff80f79efb1() (Unknown Source:0)

Repro:

using UniquePointer = std::unique_ptr<int>;
using VariantType = mpark::variant<int, UniquePointer>;

struct Bar
{
    Bar()
    {
        auto ptr = std::make_unique<int>();
        v = std::move( ptr ); // this crashes
    }
    VariantType v;
};

struct Foo {
    Foo()
    {
    }
    Bar store;
} instance; // global instance
mpark commented 6 years ago

Interesting... thanks for the report. Looks scary! I'll try to get around to looking at this soon!

erakis commented 6 years ago

I'm getting the same problem too. This is annoying as I'm stuck. Before using mpark::variant I was using strict-variant but I was getting error for nested variant, so I switch to mpark but again I'm stuck.

I'm also using Visual Studio 2015 Update 3.

mpark commented 6 years ago

Took some time to look into this today. It seems like the offending commit is 4bfed80, where I made the jump tables to be global variables. It looks like the jump tables aren't initialized while the instance global is being created. Looks like v1.2.2 works, since it doesn't have this commit.

Will need to think about how to fix this...

erakis commented 6 years ago

@mpark Thank you so much for your time. I appreciate it. I'm on a big project and being stuck each time I try a variant lib is not a good thing :(

  1. Why it is working with GCC ?

  2. By curiosity, is this problem present on VS 2017 ?

  3. If I downgrade to v1.2.2, what will I loose ? Will I get other compilation error ? Degrade the performance, missing functionality ?

  4. How many time do you estimate to fix this problem on the new version ?

mpark commented 6 years ago

Reduced test case:

struct Foo {
  Foo() { v.emplace<0>(1); }
  mpark::variant<int, std::vector<int>> v;
} foo;
mpark commented 6 years ago

@purell:

  1. I'm not sure yet. The initialization order rules are quite complicated, and i don't know if MSVC is actually doing anything wrong.
  2. I don't know. I don't have that environment set up currently. UPDATE: @K-ballo says that it's not an issue on VS2017.
  3. Refer to the changes described in https://github.com/mpark/variant/releases/tag/v1.3.0
  4. Probably not for a few weeks. I'm currently on vacation, and I made some time tonight but I can't promise anything for the rest.
rleigh-codelibre commented 6 years ago

I'm getting this error as well with 1.3.0 variant.hpp:

    0000000000000000()  Unknown
    tiff.exe!mpark::detail::visitation::alt::visit_alt_at<<lambda_a58e77b7b349863bb1db735b5431a7ff>,mpark::detail::assignment<mpark::detail::traits<VTYPES > > & __ptr64,mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1> >(unsigned __int64 index, mpark::detail::assignment<mpark::detail::traits<VTYPES > >::generic_assign::__l11::<lambda_a58e77b7b349863bb1db735b5431a7ff> && visitor, mpark::detail::assignment<mpark::detail::traits<VTYPES > > & <vs_0>, mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1> && <vs_1>) Line 1203 C++
    tiff.exe!mpark::detail::assignment<mpark::detail::traits<VTYPES > >::generic_assign<mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1> >(mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1> && that) Line 1673  C++
    tiff.exe!mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1>::operator=(mpark::detail::move_assignment<mpark::detail::traits<VTYPES >,1> && that) Line 1700    C++
    tiff.exe!mpark::detail::copy_assignment<mpark::detail::traits<VTYPES >,1>::operator=(mpark::detail::copy_assignment<mpark::detail::traits<VTYPES >,1> && __that) Line 618   C++
    [External Code] 
    tiff.exe!mpark::variant<VTYPES >::operator=(mpark::variant<VTYPES > && __that) Line 618 C++
        tiff.exe!ome::files::VariantPixelBuffer::createBuffer(const boost::detail::multi_array::extent_gen<9> & range, ome::xml::model::enums::PixelType pixeltype, const boost::general_storage_order<9> & storage) Line 323   C++
    tiff.exe!ome::files::VariantPixelBuffer::VariantPixelBuffer() Line 126  C++
    tiff.exe!TIFFVariantTest::getPNGData(unsigned int xsize, unsigned int ysize, ome::xml::model::enums::PixelType pixeltype, ome::files::tiff::PlanarConfiguration planarconfig) Line 979  C++
    tiff.exe!`anonymous namespace'::pixel_tests() Line 1717 C++
    tiff.exe!`dynamic initializer for 'pixel_params''() Line 1754   C++
    [External Code] 

with a simple assignment here. I can try to reduce it, but it looks the same as the above. Works fine on MacOS X and Linux, fails with VS2015.

VTYPES are std::shared_ptr<ome::files::PixelBuffer<signed char> >,std::shared_ptr<ome::files::PixelBuffer<short> >,std::shared_ptr<ome::files::PixelBuffer<int> >,std::shared_ptr<ome::files::PixelBuffer<unsigned char> >,std::shared_ptr<ome::files::PixelBuffer<unsigned short> >,std::shared_ptr<ome::files::PixelBuffer<unsigned int> >,std::shared_ptr<ome::files::PixelBuffer<bool> >,std::shared_ptr<ome::files::PixelBuffer<float> >,std::shared_ptr<ome::files::PixelBuffer<double> >,std::shared_ptr<ome::files::PixelBuffer<std::complex<float> > >,std::shared_ptr<ome::files::PixelBuffer<std::complex<double> > > i.e. a list of shared pointers of different types; while a bit horrible, it's shouldn't be causing problems.

Retesting with 1.2.2, it works correctly and there's no segfault.

Edit: Just to comment on the use of global jump tables, my code is constructing and assigning the variant inside a static (googletest test discovery and registration) before main() is entered.

mpark commented 5 years ago

Fixed by 1896bd5f7031b4b81abf6c07595848198aec215a