Skip to content

Move from destroyed object in tuple_cat of nested tuples #23180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
llvmbot opened this issue Mar 5, 2015 · 11 comments
Closed

Move from destroyed object in tuple_cat of nested tuples #23180

llvmbot opened this issue Mar 5, 2015 · 11 comments
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@llvmbot
Copy link
Member

llvmbot commented Mar 5, 2015

Bugzilla Link 22806
Resolution FIXED
Resolved on Apr 15, 2016 13:06
Version 3.6
OS MacOS X
Reporter LLVM Bugzilla Contributor
CC @eugenis,@mclow

Extended Description

This bug concerns libc++ trunk (git-svn ID 230867).

In the following code, Tracked{} is destroyed and is then moved-from.

std::tuple_cat(std::make_tuple(std::make_tuple(Tracked{})));

Here's a self-contained test program:

#include <cassert>
#include <tuple>

struct Tracked {
    enum class State { CONSTRUCTED, MOVED_FROM, DESTROYED };
    State state;

    Tracked() : state{State::CONSTRUCTED} { }

    Tracked(Tracked const& t) : state{State::CONSTRUCTED} {
        assert(t.state != State::MOVED_FROM && "copying a moved-from object");
        assert(t.state != State::DESTROYED && "copying a destroyed object");
    }

    Tracked(Tracked&& t) : state{State::CONSTRUCTED} {
        assert(t.state != State::MOVED_FROM && "double moving from an object");
        assert(t.state != State::DESTROYED && "moving from a destroyed object");
        t.state = State::MOVED_FROM;
    }

    ~Tracked() {
        assert(state != State::DESTROYED && "double-destroying an object");
        state = State::DESTROYED;
    }
};

int main() {
    std::tuple_cat(std::make_tuple(std::make_tuple(Tracked{})));
}

Compiling and then running this will produce:

Assertion failed: (t.state != State::DESTROYED && "moving from a destroyed object")

I don't have the time to investigate this further right now, unfortunately.

@mclow
Copy link
Contributor

mclow commented Mar 5, 2015

More extensive test.
It's clear that the problem occurs in tuple_cat, with an rvalue argument.

#include <iostream>
#include <cassert>
#include <tuple>

struct Tracked {
    enum class State { CONSTRUCTED, MOVED_FROM, DESTROYED };
    State state;

    Tracked() : state{State::CONSTRUCTED} { std::cout << "Constructing " << (void*) this << std::endl; }

    Tracked(Tracked const& t) : state{State::CONSTRUCTED} {
        std::cout << "Copy-constructing " <<  (void*) this << " from " << (void *) &t << std::endl; 
        assert(t.state != State::MOVED_FROM && "copying a moved-from object");
        assert(t.state != State::DESTROYED  && "copying a destroyed object");
    }

    Tracked(Tracked&& t) : state{State::CONSTRUCTED} {
        std::cout << "Move-constructing " <<  (void*) this << " from " << (void *) &t << std::endl; 
        assert(t.state != State::MOVED_FROM && "double moving from an object");
        assert(t.state != State::DESTROYED  && "moving from a destroyed object");
        t.state = State::MOVED_FROM;
    }

    ~Tracked() {
        std::cout << "Destroying " << (void*) this << std::endl; 
        assert(state != State::DESTROYED && "double-destroying an object");
        state = State::DESTROYED;
    }
};

int main() {
    Tracked v;
    auto t1 = std::make_tuple(v);
//  auto t1 = std::make_tuple(Tracked{});
    {
        auto t2 = std::make_tuple(t1);
//      auto t2 = std::make_tuple(std::move(t1));
        {
            std::cout << "  Creating t3" << std::endl;
//          auto t3 = std::tuple_cat(t2);
            auto t3 = std::tuple_cat(std::move(t2)); // move
            std::cout << "  Destroying t3" << std::endl;
        }
    }
}

@mclow
Copy link
Contributor

mclow commented Mar 5, 2015

It appears also that it's only a problem in the single argument case.

If you change
auto t2 = std::make_tuple(t1);
to:
auto t2 = std::make_tuple(t1, t1);
or:
auto t2 = std::make_tuple(3, t1);
or:
auto t2 = std::make_tuple(t1, 3);

Then the assert does not fire.

@mclow
Copy link
Contributor

mclow commented Mar 5, 2015

Created http://reviews.llvm.org/D8084 with a possible solution for this.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 7, 2015

We are incorrectly calling the "tuple-like" copy/move constructor std::tuple<std::tuple>::tuple(std::tuple&&) on line 664.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2015

Minimal Reproducer
I've attached a minimal reproducer that demonstrates the bug.

Louis, Is it alright If I commit your "Tracked" class into our test suite? It seems like a useful thing to have around.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2015

Sure, go ahead. It's very useful to detect improper usage of perfect forwarding in tricky code. Here's the (more complete) version I use in Hana: http://goo.gl/ighmKQ.
The std::cerr << ...; are commented out because it generates a ton of output. It's useful to enable them when you found a bug and want to track it down.

@llvmbot
Copy link
Member Author

llvmbot commented Mar 29, 2015

The following also triggers an assertion on my machine:


int main() {
    auto empty = std::make_tuple();
    auto singleton = std::make_tuple(Tracked{});
    std::tuple_cat(empty, std::make_tuple(singleton));
}

@llvmbot
Copy link
Member Author

llvmbot commented Aug 31, 2015

Possible fix up for review as D12502. See http://reviews.llvm.org/D12502 .

@llvmbot
Copy link
Member Author

llvmbot commented Oct 14, 2015

*** Bug llvm/llvm-bugzilla-archive#25169 has been marked as a duplicate of this bug. ***

@llvmbot
Copy link
Member Author

llvmbot commented Apr 15, 2016

Finally fixed in r266461.

@llvmbot
Copy link
Member Author

llvmbot commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#25169

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

2 participants