[Bug c++/104872] New: Memory corruption in Coroutine with POD type
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
#include
#include
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;
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(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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872
--- Comment #1 from Benjamin Buch ---
More minimal version:
https://godbolt.org/z/aEv13e38a
```cpp
#include
#include
#include
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;
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872
--- Comment #2 from Benjamin Buch ---
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
#include
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;
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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872 --- Comment #4 from Benjamin Buch --- 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
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104872 --- Comment #3 from Benjamin Buch --- Bug is still present in GCC 12.1 and current trunk.
[Bug c++/114076] New: list-initialization with assignment expression triggers wrong template instanciation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114076
Bug ID: 114076
Summary: list-initialization with assignment expression
triggers wrong template instanciation
Product: gcc
Version: 13.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: ---
I come from this Bug Report to Visual C++:
- https://github.com/microsoft/STL/issues/4417
I think that GCC has a similar bug here.
```cpp
template
struct holder {
holder() = default;
constexpr ~holder() {
static_assert(sizeof(T) || true);
}
};
struct Incomplete;
struct Class {
Class();
~Class();
holder a{};
holder b = {};
//holder c = holder{};
};
int main() {
[[maybe_unused]] Class v;
}
```
- https://godbolt.org/z/ds3WYK55f
Cases a and b can both be compiled with Clang. GCC rejects b, which is wrong in
my opinion.
If I understand it correctly, then cases a and b should result in absolutely
identical initializations. The destructor of holder should not be instantiated.
In case c, the rejection seems to me to be correct, since here the temporary
value must be destroyed by a destructor call.
Among other things, this error affects unique_ptr.
[Bug c++/114076] list-initialization with assignment expression triggers wrong template instanciation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114076
--- Comment #3 from Benjamin Buch ---
I [created an overview](https://stackoverflow.com/a/78101462/4821621) with all
cases that currently work on StackOverflow. I think that all these cases should
be valid. For a properly formated version with links to Compiler Explorer
please use my stackoverflow post. Here is the relevant part
GitHub user
[frederick-vs-ja](https://github.com/microsoft/STL/issues/4417#issuecomment-1960976417)
has reduced the problem in the MS STL Bug Report to a minimal example.
```cpp
template
struct Holder {
Holder() = default;
constexpr ~Holder() { static_assert(sizeof(T) || true); }
};
struct Incomplete;
struct Class {
Class();
~Class();
Holder v{};
};
int main() { [[maybe_unused]] Class v; }
```
I have extended this example with some variants and tested the compilers with
them. Syntactically, there are 7 ways to call the default constructor of
Holder:
- `Holder a;` default-initialization
- `Holder b{};` direct-list-initialization
- `Holder c = {};` copy-list-initialization
- `Holder d = Holder();` copy-direct-initialization
- `Holder e = {Holder()};` copy-list-initialization
- `Holder f = Holder{};` copy-direct-initialization
- `Holder g = {Holder{}};` copy-list-initialization
Then we can change `Holder` in two ways. (Thanks to Github user
[fsb4000](https://github.com/microsoft/STL/issues/4417#issuecomment-1959604006)
in the MS STL bug report!)
First we can remove the explicit constructor definition, so the compiler
generates this constructor implicitly:
```cpp
template
struct Holder {
constexpr ~Holder() { static_assert(sizeof(T) || true); }
};
```
Second we can remove the constexpr from the destructor:
```cpp
template
struct Holder {
Holder() = default;
~Holder() { static_assert(sizeof(T) || true); }
};
```
A third relevant change can be applied to `B`. (Thanks to [Jiang
An](https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114076#c1) in the GCC bug
report!) We can make it a template, by giving it a default template argument:
```cpp
template
struct Class {
Class();
~Class();
Holder v{};
};
```
So we have 8 code examples with 7 kinds of initialization. I created a table
with what works with which compiler. I tested with MSVC 19.38, GCC 13.2 and
clang 17.0.1. Each table cell links to the live code, where the trunk versions
of the compilers are used additionally.
- Case 1: *Implicit `Holder` ctor*, *non-`constexpr` dtor*, *non-template
`Class`*
- Case 2: Explicit `Holder` ctor, *non-`constexpr` dtor*, *non-template
`Class`*
- Case 3: *Implicit `Holder` ctor*, `constexpr` dtor, *non-template `Class`*
- Case 4: Explicit `Holder` ctor, `constexpr` dtor, *non-template `Class`*
- Case 5: *Implicit `Holder` ctor*, *non-`constexpr` dtor*, template `Class`
- Case 6: Explicit `Holder` ctor, *non-`constexpr` dtor*, template `Class`
- Case 7: *Implicit `Holder` ctor*, `constexpr` dtor, template `Class`
- Case 8: Explicit `Holder` ctor, `constexpr` dtor, template `Class`
The order is: ✅/❌ MSVC ; ✅/❌ GCC ; ✅/❌ clang
||Case a|Case b|Case c|Case d|Case e|Case f|Case g|
|-|-|-|-|-|-|-|-|
|Case 1:|✅✅✅|✅✅✅|✅✅✅|✅❌✅|✅❌✅|✅❌✅|✅❌✅|
|Case 2:|✅✅✅|✅✅✅|✅❌✅|✅❌✅|✅❌✅|✅❌✅|✅❌✅|
|Case 3:|✅✅✅|❌✅✅|❌✅✅|❌❌❌|❌❌❌|❌❌❌|❌❌❌|
|Case 4:|✅✅✅|❌✅✅|❌❌✅|❌❌❌|❌❌❌|❌❌❌|❌❌❌|
|Case 5:|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|
|Case 6:|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|✅✅✅|
|Case 7:|✅✅✅|✅✅✅|✅✅✅|✅✅❌|✅✅❌|✅✅❌|✅✅❌|
|Case 8:|✅✅✅|✅✅✅|✅✅✅|✅✅❌|✅✅❌|✅✅❌|✅✅❌|
Cases d, e, f and g can apparently be considered as one case.
[Bug c++/104850] Instantiating a destructor for a template class too early, before the calling destructor is seen - rejects valid code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104850 Benjamin Buch changed: What|Removed |Added CC||benni.buch at gmail dot com --- Comment #6 from Benjamin Buch --- I think this is a sub-case of Bug 104850
[Bug c++/104850] Instantiating a destructor for a template class too early, before the calling destructor is seen - rejects valid code
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104850 --- Comment #7 from Benjamin Buch --- Sorry wrong number; Bug 114076
[Bug c++/114564] New: Accessing template Base via template Derived fails
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114564
Bug ID: 114564
Summary: Accessing template Base via template Derived fails
Product: gcc
Version: 13.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: ---
```cpp
template
struct Base {};
template
struct Derived: Base {
Derived(Derived::Base obj);
};
```
Accessing Base via Derived works with clang and MSVC, but not with GCC.
Live:
- https://godbolt.org/z/Wv3EK3e71
[Bug c++/114076] list-initialization with assignment expression triggers wrong template instanciation
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=114076 --- Comment #4 from Benjamin Buch --- I just tried the Compiler Explorer links from my overview and saw that there is no change in the trunk for GCC. https://stackoverflow.com/a/78101462/4821621 The fact that all three major compilers behave differently here is a problem for portability. Would be nice if someone could take a look at this ;-)
[Bug libstdc++/109299] New: wrong warning on std::wstring with -O2 -std=c++20 -D_FORTIFY_SOURCE=2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109299
Bug ID: 109299
Summary: wrong warning on std::wstring with -O2 -std=c++20
-D_FORTIFY_SOURCE=2
Product: gcc
Version: 12.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: benni.buch at gmail dot com
Target Milestone: ---
I run into this by using googletest with CMake in release mode.
```
#include
static std::wstring foo(std::wstring text = {}) {
text.resize(42);
return text;
}
int main() {
foo();
}
```
```
$ g++ -O2 -std=c++20 -D_FORTIFY_SOURCE=2 main.cpp
In file included from /usr/include/features.h:486,
from
/usr/include/x86_64-linux-gnu/c++/12/bits/os_defines.h:39,
from
/usr/include/x86_64-linux-gnu/c++/12/bits/c++config.h:655,
from /usr/include/c++/12/string:38,
from main.cpp:1:
In function ‘wchar_t* wmemcpy(wchar_t*, const wchar_t*, size_t)’,
inlined from ‘static constexpr std::char_traits::char_type*
std::char_traits::copy(char_type*, const char_type*, std::size_t)’ at
/usr/include/c++/12/bits/char_traits.h:558:16,
inlined from ‘constexpr std::__cxx11::basic_string<_CharT, _Traits,
_Alloc>::basic_string(std::__cxx11::basic_string<_CharT, _Traits, _Alloc>&&)
[with _CharT = wchar_t; _Traits = std::char_traits; _Alloc =
std::allocator]’ at /usr/include/c++/12/bits/basic_string.h:675:23,
inlined from ‘std::wstring foo(std::wstring)’ at main.cpp:5:12,
inlined from ‘int main()’ at main.cpp:9:8:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:42:10: warning: call to
‘__wmemcpy_chk_warn’ declared with attribute warning: wmemcpy called with
length bigger than size of destination buffer [-Wattribute-warning]
42 | return __glibc_fortify_n (wmemcpy, __n, sizeof (wchar_t),
| ^
```
On stackoverflow.com you can already find a few discussion about it:
- https://stackoverflow.com/questions/75689057
user17732522 reduced it to:
```
#include
wchar_t* _M_dataplus;
int _M_string_length;
wchar_t _M_local_buf[4];
wchar_t _M_local_buf2[4];
void g();
void f() {
g();
_M_string_length = 4;
if (_M_dataplus == _M_local_buf)
wmemcpy(_M_local_buf2, _M_local_buf, _M_string_length + 1);
}
```
```
$ g++ -O1 -D_FORTIFY_SOURCE=2 -c main.cpp
In file included from /usr/include/features.h:486,
from
/usr/include/x86_64-linux-gnu/bits/libc-header-start.h:33,
from /usr/include/wchar.h:27,
from main.cpp:1:
In function ‘wchar_t* wmemcpy(wchar_t*, const wchar_t*, size_t)’,
inlined from ‘void f()’ at main.cpp:15:16:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:42:10: warning: call to
‘__wmemcpy_chk_warn’ declared with attribute warning: wmemcpy called with
length bigger than size of destination buffer [-Wattribute-warning]
42 | return __glibc_fortify_n (wmemcpy, __n, sizeof (wchar_t),
| ^
```
[Bug tree-optimization/109299] wrong warning on std::wstring with -O2 -std=c++20 -D_FORTIFY_SOURCE=2
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109299 Benjamin Buch changed: What|Removed |Added Version|12.0|12.1.0 --- Comment #3 from Benjamin Buch --- You are right, I use GCC 12.1.0, sorry! I also ran reproduce it via godbolt.org starting from version 12.1 until current trunk. https://godbolt.org/z/Y9frhGr86
[Bug libstdc++/104928] std::counting_semaphore on Linux can sleep forever
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 Benjamin Buch changed: What|Removed |Added CC||benni.buch at gmail dot com --- Comment #8 from Benjamin Buch --- I came here from https://www.reddit.com/r/cpp/comments/1htews2 Shouldn't this bug have a P1 priority? Wrong code is critical after all.
[Bug libstdc++/104928] std::counting_semaphore on Linux can sleep forever
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 --- Comment #10 from Benjamin Buch --- Okay, I understand this. https://gcc.gnu.org/onlinedocs/libstdc++/manual/status.html#status.iso.2020 defines P1135R6 is implemented since libstdc++ 11.1 and the feature test macro __cpp_lib_semaphore >= 201907L is defined accordingly while the implementation is known to produce wrong code for nearly 2 years and three major releases now. It brings us into a situation again, where users have to detect compiler and compiler version to check whether they can use std::counting_semaphore instead of just checking the feature test macro. In my option this is a very bad and hard to acceptable situation, even in an experimental mode. Especially since C++20 already has a wide adoption these days. So I ask you to undefine the feature test macro and set the implementation state to partial. Also I propose to introduce -std=c++XXpreview flags for all (new) experimental language modes instead of -std=c++XX which misleadingly indicates that the mode is ready to use. The documentation might name the modes as experimental, but I think its much more user-friendly to make the flag itself expressive. (MSVC also starts doing this with the upcoming release 2022 17.13 for C++23. There will be a c++23preview mode.) I understand that this might break existing code. Nevertheless in my opinion its better to loudly fail at compile time that silently doing the wrong thing at runtime. We all know that software contains bug. That's normal. To actively inform the users is important and user-friendly. Sometimes even more important than fixing the bug itself. ;-)
[Bug libstdc++/104928] std::counting_semaphore on Linux can sleep forever
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 --- Comment #12 from Benjamin Buch --- The point is that the great majority of users do not read the full documentation to find the section that mentions the experimental character of the latest C++ modes. To be explicit about this, by just using a better name would be very helpful. Of course user will continue to use those, but they would know, that the modes are experimental and have not the same priority as non-experimental modes. Intuitive interfaces are better then large documentation that only a few users read and also only if they have a problem.
[Bug libstdc++/104928] std::counting_semaphore on Linux can sleep forever
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 --- Comment #13 from Benjamin Buch --- According to the CLI interface I opened a backwards compatible feature request: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118301
[Bug c++/118301] New: Feature request: CLI parament std with explicit experimental values
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118301 Bug ID: 118301 Summary: Feature request: CLI parament std with explicit experimental values Product: gcc Version: 15.0 Status: UNCONFIRMED Severity: normal Priority: P3 Component: c++ Assignee: unassigned at gcc dot gnu.org Reporter: benni.buch at gmail dot com Target Milestone: --- I'm coming from: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104928 Currently you can set the C++ standard via -std=c++17/20/23... While C++17 is officially supported, C++20 and higher are still experimental which means even serious bugs like wrong code generation are not handled with the same priority as similar bugs in C++17 and lower. Even professional users are already using C++20 and higher in production because they are not aware that these modes are still experimental and don't get the same level of support. You can blame the users for not reading the documentation. On the other hand you can argue, that GCCs (and clangs) CLI interface is a bit misleading, because stable and testing modes look exactly the same. MSVC will introduce a c++23preview argument with 2022 17.13. I really like this idea, because it makes the experimental character very clear to the user. Changing GCCs current interface would be a bad idea, because it would break a lot of existing code. Therefore I propose to introduce c++20/23...preview arguments that have the same effect as the current non-preview arguments. The current non-preview arguments should continue to work, but print a warning to use the preview arguments instead.
