[Bug c++/104872] New: Memory corruption in Coroutine with POD type

[Bug c++/104872] New: Memory corruption in Coroutine with POD type

[benni.buch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

            Bug ID: 104872
           Summary: Memory corruption in Coroutine with POD type
           Product: gcc
           Version: 11.2.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: benni.buch at gmail dot com
  Target Milestone: ---

The bug affects all versions of GCC 11 and trunk (upcoming version 12).

The following minimal example causes a crash during memory freeing. However,
the error seems to be caused by a previous incorrect memory access. At least
that is what the log output of contructors, the assignment and the destructor
suggests.

Tested with Compiler Explorer: https://godbolt.org/z/WKf57cPzn

```cpp
#include <coroutine>
#include <iostream>
#include <string_view>


using namespace std::literals;


class logging_string{
public:
    logging_string(std::string_view text) :text_(text) {
        std::cout << "       view: " << this << " " << text_ << std::endl;
    }

    logging_string(logging_string&& other) {
        std::cout << "       move: " << this << " <= " << &other << " new <= "
<< other.text_ << std::endl;
        text_ = std::move(other.text_);
    }

    ~logging_string(){
        std::cout << "   destruct: " << this << " " << text_ << std::endl;
    }

    logging_string& operator=(logging_string&& other){
        std::cout << "move-assign: " << this << " <= " << &other << " " <<
text_ << " <= " << other.text_ << std::endl;
        text_ = std::move(other.text_);
        return *this;
    }

private:
    std::string text_;
};

struct wrapper{
//     wrapper() = default;
//     wrapper(std::string_view text) :filename(text) {}
//     wrapper(wrapper&&) = default;
//     wrapper& operator=(wrapper&&) = default;
//     ~wrapper() = default;

    logging_string filename;
};


struct generator{
    struct promise_type;
    using handle_type = std::coroutine_handle<promise_type>;

    struct promise_type{
        wrapper value{"default"sv};
        std::exception_ptr exception;

        generator get_return_object(){
            return handle_type::from_promise(*this);
        }

        std::suspend_always initial_suspend(){
            return {};
        }

        std::suspend_always final_suspend()noexcept{
            return {};
        }

        void unhandled_exception(){ exception = std::current_exception(); }

        std::suspend_always yield_value(wrapper&& new_value){
            value = std::move(new_value);
            return {};
        }

        void return_void(){}
    };

    generator(handle_type h)
        : handle_(h)
        {}

    generator(generator&& other)
        : handle_(other.handle_)
    {
        other.handle_ = nullptr;
    }

    generator& operator=(generator&& other){
        handle_ = other.handle_;
        other.handle_ = nullptr;
        return *this;
    }

    ~generator(){
        if(handle_){
            handle_.destroy();
        }
    }

    explicit operator bool(){
        fill();
        return !handle_.done();
    }

    wrapper operator()(){
        fill();
        full_ = false;
        return std::move(handle_.promise().value);
    }

private:
    handle_type handle_;
    bool full_ = false;

    void fill(){
        if(!full_){
            handle_();
            if(handle_.promise().exception){
                std::rethrow_exception(handle_.promise().exception);
            }
            full_ = true;
        }
    }
};


static generator generate(){
    co_yield {"generate"sv};
}


int main(){
    auto gen = generate();
    (void)static_cast<bool>(gen);
}
```

Crashes with output:

```text
       view: 0xea2ec0 default
       view: 0xea2f20 generate
move-assign: 0xea2ec0 <= 0xea2f00 default <= generate
   destruct: 0xea2f00 
   destruct: 0xea2f20 generate
   destruct: 0xea2ec0 generate
free(): invalid size
Aborted (core dumped)
```

The move-assign looks very strange because the address of the "other" object is
not one of the previously created objects!

If you uncomment the wrapper functions in the minimal example, the program runs
fine and generates the expected output:

```text
       view: 0x129aec0 default
       view: 0x129af00 generate
move-assign: 0x129aec0 <= 0x129af00 default <= generate
   destruct: 0x129af00 
   destruct: 0x129aec0 generate
```

Also the move-assign looks as expected now.

You get the same valid output (regardless of whether the wrapper functions are
defined or not) when compiling with trunk of clang and msvc. (Tested via
Compiler Explorer.)

I also tested clang trunk with "-stdlib=libstdc++" which also works fine. So
the bug is probably in the g++ compiler, not in the libstdc++ library.

[Bug c++/104872] Memory corruption in Coroutine with POD type

[benni.buch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

--- Comment #1 from Benjamin Buch <benni.buch at gmail dot com> ---
More minimal version:

https://godbolt.org/z/aEv13e38a

```cpp
#include <coroutine>
#include <iostream>
#include <string_view>


using namespace std::literals;


class logging_string{
public:
    logging_string(std::string_view text) :text_(text) {
        std::cout << "       view: " << this << " " << text_ << std::endl;
    }

    logging_string(logging_string&& other) {
        std::cout << "       move: " << this << " <= " << &other << " new <= "
<< other.text_ << std::endl;
        text_ = std::move(other.text_);
    }

    ~logging_string(){
        std::cout << "   destruct: " << this << " " << text_ << std::endl;
    }

    logging_string& operator=(logging_string&& other){
        std::cout << "move-assign: " << this << " <= " << &other << " " <<
text_ << " <= " << other.text_ << std::endl;
        text_ = std::move(other.text_);
        return *this;
    }

private:
    std::string text_;
};

struct wrapper{
//     wrapper() = default;
//     wrapper(std::string_view text) :filename(text) {}
//     wrapper(wrapper&&) = default;
//     wrapper& operator=(wrapper&&) = default;
//     ~wrapper() = default;

    logging_string filename;
};


struct generator{
    struct promise_type;
    using handle_type = std::coroutine_handle<promise_type>;

    struct promise_type{
        wrapper value{"default"sv};

        generator get_return_object(){
            return handle_type::from_promise(*this);
        }

        std::suspend_always initial_suspend(){
            return {};
        }

        std::suspend_always final_suspend()noexcept{
            return {};
        }

        void unhandled_exception(){}

        std::suspend_always yield_value(wrapper&& new_value){
            value = std::move(new_value);
            return {};
        }
    };

    generator(handle_type h)
        : handle(h)
        {}

    ~generator(){
        handle.destroy();
    }

    handle_type handle;
};


static generator generate(){
    co_yield {"generate"sv};
}


int main(){
    auto gen = generate();
    gen.handle();
}
```

[Bug c++/104872] Memory corruption in Coroutine with POD type

[benni.buch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

--- Comment #2 from Benjamin Buch <benni.buch at gmail dot com> ---
To workaround it is enough define the wrapper constructor to build a string.

```cpp
wrapper(std::string text): filename(std::move(text)) {}
```

https://godbolt.org/z/9za7hfjs8

```cpp
#include <coroutine>
#include <iostream>


using namespace std::literals;


class logging_string{
public:
    logging_string(std::string text): text_(std::move(text)) {
        std::cout << "       view: " << this << " " << text_ << std::endl;
    }

    logging_string(logging_string&& other) {
        std::cout << "       move: " << this << " <= " << &other << " new <= "
<< other.text_ << std::endl;
        text_ = std::move(other.text_);
    }

    ~logging_string(){
        std::cout << "   destruct: " << this << " " << text_ << std::endl;
    }

    logging_string& operator=(logging_string&& other){
        std::cout << "move-assign: " << this << " <= " << &other << " " <<
text_ << " <= " << other.text_ << std::endl;
        text_ = std::move(other.text_);
        return *this;
    }

private:
    std::string text_;
};

struct wrapper{
//     wrapper(std::string text): filename(std::move(text)) {}

    logging_string filename;
};


struct generator{
    struct promise_type;
    using handle_type = std::coroutine_handle<promise_type>;

    struct promise_type{
        wrapper value{"default"s};

        generator get_return_object(){
            return handle_type::from_promise(*this);
        }

        std::suspend_always initial_suspend(){
            return {};
        }

        std::suspend_always final_suspend()noexcept{
            return {};
        }

        void unhandled_exception(){}

        std::suspend_always yield_value(wrapper&& new_value){
            value = std::move(new_value);
            return {};
        }
    };

    generator(handle_type h)
        : handle(h)
        {}

    ~generator(){
        handle.destroy();
    }

    handle_type handle;
};


static generator generate(){
    co_yield {"generate"s};
}


int main(){
    auto gen = generate();
    gen.handle();
}
```

[Bug c++/104872] Memory corruption in Coroutine with POD type

[benni.buch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

--- Comment #3 from Benjamin Buch <benni.buch at gmail dot com> ---
Bug is still present in GCC 12.1 and current trunk.

[Bug c++/104872] Memory corruption in Coroutine with POD type

[benni.buch at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

--- Comment #4 from Benjamin Buch <benni.buch at gmail dot com> ---
Can you please increase the priority? P3 seems too low for the wrong code. With
ICE I could understand that, but here the code seems to be compiled
successfully and then crashes when running the program. This is security
relevant!

Bug is still present in GCC 12.2 and current trunk.

[Bug c++/104872] Memory corruption in Coroutine with POD type

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
P2 is used to mark regressions that already exist on released versions, P1 is
for regressions that are new on trunk and not in any release yet. Everything
else is P3.

Changing it would serve no real purpose, it wouldn't make it any more or less
likely for somebody to work on it.

[Bug c++/104872] Memory corruption in Coroutine with POD type

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2022-11-02
             Status|UNCONFIRMED                 |NEW

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
       view: 0x60f000000050 default
       view: 0x60f0000000a0 generate
move-assign: 0x60f000000050 <= 0x60f000000080 default <= generate
   destruct: 0x60f000000080 
   destruct: 0x60f0000000a0 generate
   destruct: 0x60f000000050 generate
=================================================================
==624938==ERROR: AddressSanitizer: attempting free on address which was not
malloc()-ed: 0x60f0000000b0 in thread T0
    #0 0x7f559baa6898 in operator delete(void*)
/home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:152
    #1 0x403260 in logging_string::~logging_string() /tmp/coro.C:21
    #2 0x4034e9 in wrapper::~wrapper() /tmp/coro.C:33
    #3 0x403695 in generator::promise_type::~promise_type() /tmp/coro.C:44
    #4 0x402b60 in generate /tmp/coro.C:81
    #5 0x402d90 in generate /tmp/coro.C:79
    #6 0x403815 in
std::__n4861::coroutine_handle<generator::promise_type>::destroy() const
/home/jwakely/gcc/13/include/c++/13.0.0/coroutine:242
    #7 0x4034cd in generator::~generator() /tmp/coro.C:72
    #8 0x402e45 in main /tmp/coro.C:87
    #9 0x7f559b4b850f in __libc_start_call_main (/lib64/libc.so.6+0x2950f)
(BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)
    #10 0x7f559b4b85c8 in __libc_start_main_impl (/lib64/libc.so.6+0x295c8)
(BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)
    #11 0x402294 in _start (/tmp/a.out+0x402294)

0x60f0000000b0 is located 112 bytes inside of 168-byte region
[0x60f000000040,0x60f0000000e8)
allocated by thread T0 here:
    #0 0x7f559baa5e58 in operator new(unsigned long)
/home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x40237f in generate /tmp/coro.C:81
    #2 0x402e2d in main /tmp/coro.C:85
    #3 0x7f559b4b850f in __libc_start_call_main (/lib64/libc.so.6+0x2950f)
(BuildId: 85c438f4ff93e21675ff174371c9c583dca00b2c)

SUMMARY: AddressSanitizer: bad-free
/home/jwakely/src/gcc/gcc/libsanitizer/asan/asan_new_delete.cpp:152 in operator
delete(void*)
==624938==ABORTING


Probably another dup of PR 98401 or one of the other similar coro bugs.