[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #54 from Iain Sandoe --- (In reply to Martin Liška from comment #53) > > Yes, I forgot to mention that. > > I hope you are right and someone will make these backports in 10.1, 10.2, > > 10.3, 11.1, 11.2. > > Note one can't rewrite history, but as written, pull the gcc-11 branch and > it's fixed there. so, to be clear it will be fixed in 11.3 when that is released (and I have a set of backports on my TODO for 11.x and 10.x, so also we could expect this issues to be fixed in 10.4). There is no easy solution to the general problem - which is finding time to do all the various jobs ;)
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #53 from Martin Liška --- > Yes, I forgot to mention that. > I hope you are right and someone will make these backports in 10.1, 10.2, > 10.3, 11.1, 11.2. Note one can't rewrite history, but as written, pull the gcc-11 branch and it's fixed there.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #52 from Martin Liška --- It's fixed on master with r12-3350-g88974974d8188cf1 and it *got* backported to gcc-11 branch.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #51 from Mkkt Bkkt --- (In reply to Avi Kivity from comment #50) > Your reproducer does pass in trunk. So perhaps just a missing backport. Yes, I forgot to mention that. I hope you are right and someone will make these backports in 10.1, 10.2, 10.3, 11.1, 11.2. Or is this a naive idea of GCC development process? If so, what should I do to make these backports appear?
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #50 from Avi Kivity --- Your reproducer does pass in trunk. So perhaps just a missing backport.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Mkkt Bkkt changed: What|Removed |Added CC||valera.mironow at gmail dot com --- Comment #49 from Mkkt Bkkt --- I don't think undefined sanitizer works with coroutines in GCC 11.2.0. I have a code that doesn't work. Error description: https://gist.github.com/MBkkt/e50520096933eab997a16f54d402919b Branch for reproduce: https://github.com/YACLib/YACLib/tree/mbkkt/issue_to_gcc_ubsan Generally: I suspend coroutine not in initial/final state Then I resume it After resume, coroutine have valid addresses of stack objects. But dtor of objects on coroutine stack will be called on invalid addresses. Minimal example: https://godbolt.org/z/5h33Maeqf
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Iain Sandoe changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #48 from Iain Sandoe --- (In reply to Jakub Jelinek from comment #47) > On the #c42 testcase the false positive warning is gone with > r12-3529-g70ee703c479081ac2ea67eb67041551216e66783 > which has been backported in > r11-9062-g17e4e6e33d13e0cf09c76cba06c5fc20deab8bb4 to 11.x. > Is there any problem left with sanitizers vs. coroutines? I suppose probably (realistically), but nothing reported at this time I have the two test cases in my regular testing tree (on Darwin18) where: Schedule of variations: unix/-fsanitize=undefined,address PASS: g++.dg/coroutines/pr95137-1.C (test for excess errors) PASS: g++.dg/coroutines/pr95137-2.C (test for excess errors) PASS: g++.dg/coroutines/pr95137-2.C execution test > Even if yes, it would be better to track that in separate PRs, because this > one got overly long and mixes many different issues. concur, this has become a long thread. closing as fixed - I have a backport for 10.x in my queue.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #47 from Jakub Jelinek --- On the #c42 testcase the false positive warning is gone with r12-3529-g70ee703c479081ac2ea67eb67041551216e66783 which has been backported in r11-9062-g17e4e6e33d13e0cf09c76cba06c5fc20deab8bb4 to 11.x. Is there any problem left with sanitizers vs. coroutines? Even if yes, it would be better to track that in separate PRs, because this one got overly long and mixes many different issues.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Barnabás Pőcze changed: What|Removed |Added CC||pobrn at protonmail dot com --- Comment #46 from Barnabás Pőcze --- (In reply to stream009 from comment #42) > I got strange compile error when I use coroutine with UBSAN. > > The weird thing is error is reported in compile time not runtime. > The code compile fine without UBSAN. > [...] > === begin compile error === > : In function 'void _Z5errori.actor(error(int)::_Z5errori.frame*)': > :21:9: warning: '' may be used uninitialized > [-Wmaybe-uninitialized] >21 | co_return; > | ^ > :21:9: note: '' was declared here >21 | co_return; > | ^ > === end compile error === I am not familiar with the internals of gcc at all, but it appears that this is due to incorrect code generation. When ubsan is not used, the generated gimple looks like this: if (_13 == 0) goto ; else goto ; : _14 = _ptr->__p; result::promise_type::return_void (_14); goto final.suspend; : _15 = _ptr->__p; result::promise_type::return_void (_15); when ubsan is used, however: if (_8 == 0) goto ; else goto ; : D.9938 = _ptr->__p; .UBSAN_NULL (D.9938, 4B, 0); result::promise_type::return_void (D.9938); goto final.suspend; : .UBSAN_NULL (D.9938, 4B, 0); result::promise_type::return_void (D.9938); `D.9938` is not initialized, and I guess hence the warning. --- I have noticed that this does not happen on f6f2d6cfec1c2fe9570b98211be58329d8d7749b, so out of curiosity I tried to bisect gcc: > git bisect start > # new: [f6f2d6cfec1c2fe9570b98211be58329d8d7749b] Daily bump. > git bisect new f6f2d6cfec1c2fe9570b98211be58329d8d7749b > # old: [7ca388565af176bd4efd4f8db1e5e9e11e98ef45] Update ChangeLog and > version files for release > git bisect old 7ca388565af176bd4efd4f8db1e5e9e11e98ef45 > # old: [250f234988b6231669a720c52101d3686d645072] testsuite: Fix up > gcc.target/s390/zero-scratch-regs-1.c > git bisect old 250f234988b6231669a720c52101d3686d645072 > # old: [79513dc0b2d980bfd1b109d0d502de487c02b894] compiler: don't pad > zero-sized trailing field in results struct > git bisect old 79513dc0b2d980bfd1b109d0d502de487c02b894 > # new: [1b62cddcf091fb8cadf575246a7d3ff778650a6b] Fix ipa-modref pure/const > discovery > git bisect new 1b62cddcf091fb8cadf575246a7d3ff778650a6b > # new: [247bac507e63b32d4dc23ef1c55f300aafea24c6] libstdc++: Simplify > std::basic_regex::assign > git bisect new 247bac507e63b32d4dc23ef1c55f300aafea24c6 > # new: [d5f8abe1d3f718a75cbff0a453c1d961be5939b7] Use on-demand ranges in > ssa_name_has_boolean_range before querying nonzero bits. > git bisect new d5f8abe1d3f718a75cbff0a453c1d961be5939b7 > # new: [7d79c3ebc3f3f6f8aecf83726c97474ae5cfe957] Don't record string > concatenation data for 'RESERVED_LOCATION_P' > git bisect new 7d79c3ebc3f3f6f8aecf83726c97474ae5cfe957 > # new: [8137be3958be4e5421c283cce3e5b50dbb80b84e] mips: Fix macro typo > git bisect new 8137be3958be4e5421c283cce3e5b50dbb80b84e > # old: [caef5203d64e61da506909d58890035af32a6239] Fix internal error on > pointer-to-pointer binding in LTO mode > git bisect old caef5203d64e61da506909d58890035af32a6239 > # new: [cc1e28878a228b6c4a0872e56d97ac88971b7725] libstdc++: Check for TLS > support on mingw cross-compilers > git bisect new cc1e28878a228b6c4a0872e56d97ac88971b7725 > # new: [70ee703c479081ac2ea67eb67041551216e66783] coroutines: Make proxy vars > for the function arg copies. > git bisect new 70ee703c479081ac2ea67eb67041551216e66783 > # old: [bd55fa102715c7442c050b193dadfdb5337e2377] Fix PR ada/101970 > git bisect old bd55fa102715c7442c050b193dadfdb5337e2377 > # old: [f008fd3a480e3718436156697ebe7eeb47841457] c++: Fix > __is_*constructible/assignable for templates [PR102305] > git bisect old f008fd3a480e3718436156697ebe7eeb47841457 > # old: [de07cff96abd43f6f65dcf333958899c2ec42598] c++: empty union member > activation during constexpr [PR102163] > git bisect old de07cff96abd43f6f65dcf333958899c2ec42598 > # skip: [c5a735fa9df7eca4666c8da5e51ed9c5ab7cc81a] coroutines: Expose > implementation state to the debugger. > git bisect skip c5a735fa9df7eca4666c8da5e51ed9c5ab7cc81a > # only skipped commits left to test > # possible first new commit: [70ee703c479081ac2ea67eb67041551216e66783] > coroutines: Make proxy vars for the function arg copies. > # possible first new commit: [c5a735fa9df7eca4666c8da5e51ed9c5ab7cc81a] > coroutines: Expose implementation state to the debugger. Unfortunately, when I got to c5a735fa9df7eca4666c8da5e51ed9c5ab7cc81a, it did not build; I am not sure where I had gone wrong: > g++ -std=c++11 -fno-PIE -c
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #45 from Rafael Avila de Espindola --- (In reply to niek from comment #43) > Does this mean (and could you please reconfirm) that bug 95317 has > disappeared in trunk (which will become GCC 12)? Hi, I am not working on a project using coroutines right now, so I can't easily do any test other than trying the reduced testcase as you did.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #44 from Iain Sandoe --- folks, despite that this particular problem is of concern - I have been working on changes that are needed for correctness (with or without sanitisers!) as any specific fix for this PR would depend on those. One change that has been applied recently altered the way in which coroutine frame references are represented (as DECL_VALUE_EXPRs instead of explicitly). I believe that this change alone also fixed some other issues - and might well have fixed any specific instance). There are still a number of other correctness fixes in my queue (actually covered by one patch) - and I would like to get that in before concentrating on the sanitiser support.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #43 from niek --- (In reply to Rafael Avila de Espindola from comment #29) > Created attachment 48771 [details] > Testcase without lambda coroutines > > I modified the testcase to also build with clang and not depend on async > lambdas. This still reproduces the problem with gcc with undefined behaviour > sanitizer, but works with with clang and sanitizers and gcc with valgrind. Hi Rafael, I tried your testcase on GCC trunk today (2021-09-28), and the testcase now seems to pass (!), even with asan and ubsan enabled. I tested with Compiler Explorer, with options -std=c++20 -g -fcoroutines -fsanitize=address,undefined and with the "compile to binary" and "execute the code" options. The only thing is that I had to include the header; gcc was complaining about not finding std::exchange. Does this mean (and could you please reconfirm) that bug 95317 has disappeared in trunk (which will become GCC 12)?
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
stream009 changed:
What|Removed |Added
CC||stream009 at gmail dot com
--- Comment #42 from stream009 ---
I got strange compile error when I use coroutine with UBSAN.
The weird thing is error is reported in compile time not runtime.
The code compile fine without UBSAN.
GCC version: 11.1
compile option: -std=c++20 -Wall -Wextra -pedantic-errors -fcoroutines
-fsanitize=undefined
compiler explorer link: https://godbolt.org/z/Yva6b1YTz
It seems error happen only coroutine that doesn't return value (promise has
return_void() defined). I don't get error with coroutine that return value.
(https://godbolt.org/z/3jcvYPcqa)
Let me know if I should report this as separate bug.
=== begin source ===
#include
struct result
{
struct promise_type
{
std::suspend_never initial_suspend() { return {}; }
std::suspend_never final_suspend() noexcept { return {}; }
result get_return_object() { return {}; }
void return_void() {}
void unhandled_exception() {}
};
};
result error(int i)
{
if (i == 0) {
co_return;
}
}
=== end source ===
=== begin compile error ===
: In function 'void _Z5errori.actor(error(int)::_Z5errori.frame*)':
:21:9: warning: '' may be used uninitialized
[-Wmaybe-uninitialized]
21 | co_return;
| ^
:21:9: note: '' was declared here
21 | co_return;
| ^
=== end compile error ===
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Richard Biener changed: What|Removed |Added Status|NEW |ASSIGNED
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Richard Biener changed: What|Removed |Added Target Milestone|10.3|10.4 --- Comment #41 from Richard Biener --- GCC 10.3 is being released, retargeting bugs to GCC 10.4.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #40 from Iain Sandoe --- (In reply to niek from comment #39) > I just tested this on the nightly build of GCC 11. Unfortunately, the issue > is still there... > > @Richard Biener > Would it be a good idea to attach this bug's target milestone to GCC 11.1? > (instead of, or in addition to, GCC 10.3) the fix would be made on master first anyway and then back ported; that's not the problem - the problem is (as always) spare time to address issues.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #39 from niek --- I just tested this on the nightly build of GCC 11. Unfortunately, the issue is still there... @Richard Biener Would it be a good idea to attach this bug's target milestone to GCC 11.1? (instead of, or in addition to, GCC 10.3) kind regards, Niek
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #38 from Avi Kivity --- I do not have a patch, and unfortunately, it will take me several months at the most optimistic least to get up to speed with gcc internals to fix this. I've switched to clang, but I'd really like to switch back, I miss gcc's better support for C++20 and also just irrationally prefer it. I'll cross my fingers and toes for more luck with gcc 11.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #37 from Iain Sandoe --- (In reply to Avi Kivity from comment #36) > A reminder that coroutines are crippled without this fixed, as it is > standard practice these days to use sanitizers. Although I have taken the PR, please don't let that stop you providing a patch if you have one. My volunteer-time is very limited and it's quite likely that I won't get to this until some time during stage #3.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #36 from Avi Kivity --- A reminder that coroutines are crippled without this fixed, as it is standard practice these days to use sanitizers.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #35 from Iain Sandoe --- (In reply to niek from comment #34) > Any progress on this issue? > > (The issue is still present in gcc-trunk...) I have a couple of ideas, but very short of time to try them out, sorry about that.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 niek changed: What|Removed |Added CC||niekb at scintilla dot utwente.nl --- Comment #34 from niek --- Dear Iain, Any progress on this issue? (The issue is still present in gcc-trunk...) kind regards, Niek
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Richard Biener changed: What|Removed |Added Target Milestone|10.2|10.3 --- Comment #33 from Richard Biener --- GCC 10.2 is released, adjusting target milestone.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #32 from Iain Sandoe --- (In reply to Rafael Avila de Espindola from comment #31) > Hi Iain, > > Any update on this? If there is any way I can help, please let me know. It > has been a decade since I looked into gcc, but I should still be able to > test patches or implement small side changes. I was working on a wider problem, now need to re-analyze this. A quick look says we have a similar issue to one already solved (that was duplicate use of a dtor). When we switch sanitize off, the gimple generated correctly re-loads the this pointer for both the catch and finally clauses. When sanitize=undefined is on, it seems to conclude (incorrectly) that a temp loaded for the catch clause can be re-used for the finally clause. Not sure at present where the error is creeping in. (so from my quick test, your case passes without -fsanitize=undefined, the DTORs run correctly - no abort() .. and it fails with the sanitiser engaged because the DTOR gets a nonsense this ptr.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #31 from Rafael Avila de Espindola --- Hi Iain, Any update on this? If there is any way I can help, please let me know. It has been a decade since I looked into gcc, but I should still be able to test patches or implement small side changes.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #30 from Iain Sandoe --- (In reply to Rafael Avila de Espindola from comment #29) > Created attachment 48771 [details] > Testcase without lambda coroutines > > I modified the testcase to also build with clang and not depend on async > lambdas. This still reproduces the problem with gcc with undefined behaviour > sanitizer, but works with with clang and sanitizers and gcc with valgrind. thanks, I think I have a fix for this locally - but not ready to post just yet.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Rafael Avila de Espindola changed: What|Removed |Added Attachment #48723|0 |1 is obsolete|| --- Comment #29 from Rafael Avila de Espindola --- Created attachment 48771 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48771=edit Testcase without lambda coroutines I modified the testcase to also build with clang and not depend on async lambdas. This still reproduces the problem with gcc with undefined behaviour sanitizer, but works with with clang and sanitizers and gcc with valgrind.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Rafael Avila de Espindola changed: What|Removed |Added Attachment #48579|0 |1 is obsolete|| --- Comment #28 from Rafael Avila de Espindola --- Created attachment 48723 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48723=edit new testcase This testcase still fails with gcc 50ff02b534195c12298c64311d03a8b2d2dc261f $ ~/gcc/inst/bin/g++ -std=gnu++20 -fcoroutines -g coroutines_test.cc -o t && valgrind ./t ... ==105582== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0) % ~/gcc/inst/bin/g++ -std=gnu++20 -fcoroutines -g coroutines_test.cc -fsanitize=undefined -o t && ./t /home/espindola/scylla/t/coroutines_test.cc:793: runtime error: member call on misaligned address 0x0020d854 for type 'struct future', which requires 8 byte alignment ...
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #27 from CVS Commits --- The releases/gcc-10 branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:800dac8fca3cf75512913e380df339fa2253ba76 commit r10-8274-g800dac8fca3cf75512913e380df339fa2253ba76 Author: Iain Sandoe Date: Thu Jun 11 14:11:14 2020 +0100 coroutines: Ensure distinct DTOR trees [PR95137]. Part of the PR notes that there are UBSAN fails for the coroutines test suite. These are primarily related to the use of the same DTOR tree in the two edges from the await block. Fixed by building a new tree for each. gcc/cp/ChangeLog: PR c++/95137 * coroutines.cc (expand_one_await_expression): Build separate DTOR trees for the awaitable object on the destroy and resume paths. (cherry picked from commit 006f28aefeb3be575239beddc7febe56dff463a2)
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #26 from Iain Sandoe --- (In reply to Rafael Avila de Espindola from comment #25) > (In reply to CVS Commits from comment #24) > > The master branch has been updated by Iain D Sandoe : > I can confirm that the reduced testcase is now fixed. the patch was intended to address the UBSAN failure identified - fixing the asan ones was incidental. >On the original test I still get runtime error: member call on misaligned >address 0x0003 for type > 'struct future', which requires 8 byte alignment > > I will try reducing the testcase again and upload the results. thanks, that's very helpful. On the test cases in the testsuite, I saw three fails that looked like false positives (because the asan logic doesn't know about things being moved into the coroutine frame). However, the error you report above seems more likely related to some incorrect dtor (there are some known issues there too...) In general, debug-related support is pretty much non-existent in coroutines impls (other than in MSVC). I have some ideas about how to make this better on GCC - and I suspect that will help deal with support for the sanitisers.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #25 from Rafael Avila de Espindola --- (In reply to CVS Commits from comment #24) > The master branch has been updated by Iain D Sandoe : Thanks! I can confirm that the reduced testcase is now fixed. On the original test I still get runtime error: member call on misaligned address 0x0003 for type 'struct future', which requires 8 byte alignment I will try reducing the testcase again and upload the results.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #24 from CVS Commits --- The master branch has been updated by Iain D Sandoe : https://gcc.gnu.org/g:006f28aefeb3be575239beddc7febe56dff463a2 commit r11-1129-g006f28aefeb3be575239beddc7febe56dff463a2 Author: Iain Sandoe Date: Tue Jun 9 19:17:14 2020 +0100 coroutines: Ensure distinct DTOR trees [PR95137]. Part of the PR notes that there are UBSAN fails for the coroutines test suite. These are primarily related to the use of the same DTOR tree in the two edges from the await block. Fixed by building a new tree for each. gcc/cp/ChangeLog: PR c++/95137 * coroutines.cc (expand_one_await_expression): Build separate DTOR trees for the awaitable object on the destroy and resume paths.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #23 from Iain Sandoe --- (In reply to Avi Kivity from comment #22) > @Iain: if you can publish your patches somewhere we can test them with our > codebase and report. > > (if you can publish them on releases/gcc-10 that's even better). working on collating them ready to post - and I expect them to be appropriate for back-port to 10.x.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #22 from Avi Kivity --- @Iain: if you can publish your patches somewhere we can test them with our codebase and report. (if you can publish them on releases/gcc-10 that's even better).
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Iain Sandoe changed: What|Removed |Added Target Milestone|--- |10.2
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #21 from Iain Sandoe --- (In reply to Rafael Avila de Espindola from comment #20) > The attached testcase also fails with just -fsanitize=undefined. I have > tested with gcc version > > gcc (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1) thanks Rafael, I confirm this is repeatable for me on both master and 10.1 (and, also that patches in my queue fix it for at least master [on Darwin]) doing some wider testing now.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #20 from Rafael Avila de Espindola --- The attached testcase also fails with just -fsanitize=undefined. I have tested with gcc version gcc (GCC) 10.1.1 20200507 (Red Hat 10.1.1-1)
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
Rafael Avila de Espindola changed:
What|Removed |Added
Attachment #48547|0 |1
is obsolete||
--- Comment #19 from Rafael Avila de Espindola ---
Created attachment 48579
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48579=edit
Single file testcase
$ g++ -g -fcoroutines -std=gnu++20 coroutines_test.cc -o t && valgrind ./t
no errors, exit value 42
$ g++ -g -fcoroutines -std=gnu++20 coroutines_test.cc -o t -fsanitize=address
-fsanitize=undefined && ./t
coroutines_test.cc:561:26: runtime error: member access within misaligned
address 0x002231e6 for type 'struct promise_base', which requires 8 byte
alignment
0x002231e6: note: pointer points here
5c 5d c3 cc 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 58 48 89 7d 88
48 8d 5d 90 49 89 df
^
AddressSanitizer:DEADLYSIGNAL
=
==112180==ERROR: AddressSanitizer: SEGV on unknown address 0x002231ee (pc
0x0021adf2 bp 0x7ffdf0604570 sp 0x7ffdf06044f0 T0)
==112180==The signal is caused by a WRITE memory access.
#0 0x21adf2 in seastar::internal::future_base::detach_promise()
/home/espindola/scylla/t/coroutines_test.cc:561
#1 0x21ac94 in seastar::internal::future_base::clear()
/home/espindola/scylla/t/coroutines_test.cc:556
#2 0x21acd2 in seastar::internal::future_base::~future_base()
/home/espindola/scylla/t/coroutines_test.cc:559
#3 0x21e3d4 in seastar::future<>::~future()
/home/espindola/scylla/t/coroutines_test.cc:645
#4 0x21d1e1 in seastar::internal::awaiter<>::~awaiter()
/home/espindola/scylla/t/coroutines_test.cc:862
#5 0x215d02 in main::{lambda()#1}::operator()() const [clone .actor]
/home/espindola/scylla/t/coroutines_test.cc:1172
#6 0x214d3b in operator() /home/espindola/scylla/t/coroutines_test.cc:1169
#7 0x216ab8 in apply /home/espindola/scylla/t/coroutines_test.cc:92
#8 0x216b8a in apply >
/home/espindola/scylla/t/coroutines_test.cc:97
#9 0x216d10 in operator() /home/espindola/scylla/t/coroutines_test.cc:680
#10 0x21765e in
satisfy_with_result_of::then_impl_nrvo, seastar::future<> >&&)> mutable:: >
/home/espindola/scylla/t/coroutines_test.cc:793
#11 0x216fee in operator() /home/espindola/scylla/t/coroutines_test.cc:678
#12 0x218881 in run_and_dispose
/home/espindola/scylla/t/coroutines_test.cc:376
#13 0x2143c2 in seastar::reactor::run_tasks(seastar::reactor::task_queue&)
/home/espindola/scylla/t/coroutines_test.cc:1130
#14 0x21463e in seastar::reactor::run()
/home/espindola/scylla/t/coroutines_test.cc:1138
#15 0x216852 in main /home/espindola/scylla/t/coroutines_test.cc:1173
#16 0x7faee1994041 in __libc_start_main (/lib64/libc.so.6+0x27041)
#17 0x21190d in _start (/home/espindola/scylla/t/t+0x21190d)
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV /home/espindola/scylla/t/coroutines_test.cc:561
in seastar::internal::future_base::detach_promise()
==112180==ABORTING
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
--- Comment #18 from Rafael Avila de Espindola ---
(In reply to Avi Kivity from comment #17)
> Is that the test were a lambda coroutine is called from future::then()? In
> that case it's a real use-after-free.
It was reduced from that to just
#include
#include
using namespace seastar;
int main(int argc, char** argv) {
seastar::app_template app;
app.run(argc, argv, [] () -> future<> {
future<> xyz = make_ready_future<>().then([] {});
co_await std::move(xyz);
});
return 0;
}
Which doesn't have a user after free.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #17 from Avi Kivity --- Is that the test were a lambda coroutine is called from future::then()? In that case it's a real use-after-free.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
--- Comment #16 from Rafael Avila de Espindola ---
> @Rafael: Can you please append output with:
> export UBSAN_OPTIONS="print_stacktrace=1"
I also added halt_on_error=1:abort_on_error=1:
It is
../tests/unit/coroutines_test.cc:11:5: runtime error: member call on misaligned
address 0x41b58ab3 for type 'struct awaiter', which requires 8 byte
alignment
0x41b58ab3: note: pointer points here
Reactor stalled for 261 ms on shard 0.
Backtrace:
/lib64/libasan.so.6+0x00045bad
...
#0 0xf3605a in main::{lambda()#1}::operator()() const [clone .actor]
../tests/unit/coroutines_test.cc:11
#1 0xf3b20b in std::__n4861::coroutine_handle::resume() const
/usr/include/c++/10/coroutine:126
#2 0xf3b5ed in
seastar::internal::coroutine_traits_base<>::promise_type::run_and_dispose()
../include/seastar/core/coroutine.hh:104
#3 0x11c8892 in seastar::reactor::run_tasks(seastar::reactor::task_queue&)
../src/core/reactor.cc:2151
#4 0x11cc8db in seastar::reactor::run_some_tasks()
../src/core/reactor.cc:2566
#5 0x11d1aa0 in seastar::reactor::run() ../src/core/reactor.cc:2721
#6 0xf4ef5e in seastar::app_template::run_deprecated(int, char**,
std::function&&) ../src/core/app-template.cc:202
#7 0xf4cf00 in seastar::app_template::run(int, char**,
std::function ()>&&) ../src/core/app-template.cc:115
#8 0xf4d3fa in seastar::app_template::run(int, char**,
std::function ()>&&) ../src/core/app-template.cc:130
#9 0xf36a70 in main ../tests/unit/coroutines_test.cc:8
#10 0x7fac0533d041 in __libc_start_main (/lib64/libc.so.6+0x27041)
#11 0xf3466d in _start
(/home/espindola/scylla/scylla/seastar/build-dbg/tests/unit/coroutines_test+0xf3466d)
I will try to reduce the test to not need seastar.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #15 from Iain Sandoe --- (In reply to Martin Liška from comment #14) > The original problem: > > test.cc:3749:5: runtime error: member call on misaligned address > 0x41b58ab3 for type 'struct awaiter', which requires 8 byte alignment > 0x41b58ab3: note: pointer points here > > > @Iain: How do you allocate the awaiter object? The coro frame is laid out according to the types determined for the objects it contains (the awaiter types are known at the point it's laid out). It just uses the 'normal' struct building rules. Awaiters are most often initialised from return values from some promise method, but they can also be local vars or parms - perhaps I slipped up there. I have a thought that I might be failing to copy across excess alignment applied (which I will look at later). ^^^ these are things on my TO-LOOK-AT list. > > @Rafael: Can you please append output with: > export UBSAN_OPTIONS="print_stacktrace=1"
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #14 from Martin Liška --- The original problem: test.cc:3749:5: runtime error: member call on misaligned address 0x41b58ab3 for type 'struct awaiter', which requires 8 byte alignment 0x41b58ab3: note: pointer points here @Iain: How do you allocate the awaiter object? @Rafael: Can you please append output with: export UBSAN_OPTIONS="print_stacktrace=1"
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #13 from Iain Sandoe --- Created attachment 48572 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48572=edit fix for most of the UBSAN fails Most of the UBSAN fails are from a single cause; I reused the built DTOR tree on both resume and destroy edges from the await. There is a single remaining UBSAN fail (which is unrelated). I don't expect this to make any difference to the initial reported fail.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #12 from Rafael Avila de Espindola --- (In reply to Martin Liška from comment #6) > Thank you, can you please attach a pre-processed file (-E) so that one > doesn't need to clone seastar repository? The testcase that is attached doesn't require any seastar headers, just linking with libseastar.a. I will try to reduce it to a self contained test.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #11 from Iain Sandoe --- perhaps I have some invalid sharing of trees that only causes an issue for ubsan - will try build independent dtor trees for the two cases.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
--- Comment #10 from Iain Sandoe ---
It seems that the ubsan complaints look all rather similar.
At least for the following case, ubsan seems to cause a change which introduces
a bogus temporary use.
class-00-co-ret.C u=is a very simple coroutine - this is the output of
-fdump-tree-gimple for the main body of the actor function.
without ubsan.
_7 = _ptr->__p;
frame_ptr->__aw_s.is = coro1::promise_type::initial_suspend (_7);
[return slot optimization]
_8 = _ptr->__aw_s.is;
_9 = coro1::suspend_always_prt::await_ready (_8);
retval.0 = ~_9;
if (retval.0 != 0) goto ; else goto ;
:
frame_ptr->__resume_at = 2;
_10 = _ptr->__aw_s.is;
coro1::suspend_always_prt::await_suspend (_10,
frame_ptr->__self_h);
D.11053 = .CO_YIELD (2, 0, , , frame_ptr);
retval.1 = D.11053;
switch (retval.1) , case 0: , case 1:
>
:
.CO_SUSPN ();
:
goto resume.2;
:
goto destroy.2;
destroy.2:
_11 = _ptr->__aw_s.is;
coro1::suspend_always_prt::~suspend_always_prt (_11);
goto coro.delete.promise;
:
resume.2:
frame_ptr->__i_a_r_c = 1;
_12 = _ptr->__aw_s.is;
coro1::suspend_always_prt::await_resume (_12);
_13 = _ptr->__aw_s.is;
coro1::suspend_always_prt::~suspend_always_prt (_13);
{
puts ("coro1: about to return");
_14 = _ptr->__p;
coro1::promise_type::return_value (_14, 42);
goto final.suspend;
with ubsan
_7 = _ptr->__p;
frame_ptr->__aw_s.is = coro1::promise_type::initial_suspend (_7);
[return slot optimization]
D.11410 = _ptr->__aw_s.is;
.UBSAN_NULL (D.11410, 4B, 0);
_8 = coro1::suspend_always_prt::await_ready (D.11410);
retval.0 = ~_8;
if (retval.0 != 0) goto ; else goto ;
:
frame_ptr->__resume_at = 2;
D.11413 = _ptr->__aw_s.is;
.UBSAN_NULL (D.11413, 4B, 0);
coro1::suspend_always_prt::await_suspend (D.11413,
frame_ptr->__self_h);
D.11322 = .CO_YIELD (2, 0, , , frame_ptr);
retval.1 = D.11322;
switch (retval.1) , case 0: , case 1:
>
:
.CO_SUSPN ();
:
goto resume.2;
:
goto destroy.2;
destroy.2:
D.11415 = _ptr->__aw_s.is;
.UBSAN_NULL (D.11415, 4B, 0);
coro1::suspend_always_prt::~suspend_always_prt (D.11415);
goto coro.delete.promise;
:
resume.2:
frame_ptr->__i_a_r_c = 1;
D.11416 = _ptr->__aw_s.is;
.UBSAN_NULL (D.11416, 4B, 0);
coro1::suspend_always_prt::await_resume (D.11416);
.UBSAN_NULL (D.11415, 4B, 0);
^^^ this is not correct D.11415 is not valid here, (D.11416 would be).
coro1::suspend_always_prt::~suspend_always_prt (D.11415);
{
puts ("coro1: about to return");
D.11417 = _ptr->__p;
.UBSAN_NULL (D.11417, 4B, 4);
coro1::promise_type::return_value (D.11417, 42);
goto final.suspend;
}
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Iain Sandoe changed: What|Removed |Added Assignee|marxin at gcc dot gnu.org |iains at gcc dot gnu.org --- Comment #9 from Iain Sandoe --- some of the issues noted are likely addressed by patches already in my queue - so taking this for now.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Martin Liška changed: What|Removed |Added Status|WAITING |NEW Assignee|unassigned at gcc dot gnu.org |marxin at gcc dot gnu.org --- Comment #8 from Martin Liška --- Created attachment 48555 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48555=edit Failures for UBSAN in coroutine test-suite And there are quite many UBSAN errors.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #7 from Martin Liška --- In the meantime, I run all tests in gcc/testsuite/g++.dg/coroutines/torture and I see: $ g++ co-ret-17-void-ret-coro.C -fsanitize=address -fcoroutines -O2 && ./a.out = ==8226==ERROR: AddressSanitizer: heap-use-after-free on address 0x60400010 at pc 0x00401286 bp 0x7fffe130 sp 0x7fffe128 READ of size 8 at 0x60400010 thread T0 #0 0x401285 in main (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x401285) #1 0x770dfcea in __libc_start_main ../csu/libc-start.c:308 #2 0x401309 in _start (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x401309) 0x60400010 is located 0 bytes inside of 48-byte region [0x60400010,0x60400040) freed by thread T0 here: #0 0x776ae9f7 in operator delete(void*) /home/marxin/Programming/gcc/libsanitizer/asan/asan_new_delete.cpp:160 #1 0x4015b7 in _Z7my_coroRNSt7__n486116coroutine_handleIvEE.actor(my_coro(std::__n4861::coroutine_handle&)::_Z7my_coroRNSt7__n486116coroutine_handleIvEE.frame*) (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x4015b7) previously allocated by thread T0 here: #0 0x776adff7 in operator new(unsigned long) /home/marxin/Programming/gcc/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x401902 in my_coro(std::__n4861::coroutine_handle&) (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x401902) #2 0x403d8f in __local_asan_preinit (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x403d8f) SUMMARY: AddressSanitizer: heap-use-after-free (/home/marxin/Programming/gcc/gcc/testsuite/g++.dg/coroutines/torture/a.out+0x401285) in main Shadow bytes around the buggy address: 0x0c087fff7fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7fe0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c087fff7ff0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c087fff8000: fa fa[fd]fd fd fd fd fd fa fa fa fa fa fa fa fa 0x0c087fff8010: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c087fff8020: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c087fff8030: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c087fff8040: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c087fff8050: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user:f7 Container overflow: fc Array cookie:ac Intra object redzone:bb ASan internal: fe Left alloca redzone: ca Right alloca redzone:cb Shadow gap: cc ==8226==ABORTING
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #6 from Martin Liška --- Thank you, can you please attach a pre-processed file (-E) so that one doesn't need to clone seastar repository?
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137
--- Comment #5 from Rafael Avila de Espindola ---
With a seastar patched for c++ 20 (mostly dropping a few experimental/ from
includes and experimental:: from names), the following is all that is needed:
#include
#include
using namespace seastar;
int main(int argc, char** argv) {
seastar::app_template app;
app.run(argc, argv, [] () -> future<> {
future<> xyz = make_ready_future<>().then([] {});
co_await std::move(xyz);
});
return 0;
}
I have attached a partially preprocessed version that can be use with current
seastar from https://github.com/scylladb/seastar.
First build seastar in debug mode:
$ mkdir build
$ cd build
$ cmake -DCMAKE_BUILD_TYPE=Debug .. -GNinja
$ ninja
And the test can be built with
$ g++ test.cc -fcoroutines $(pkg-config --cflags --libs
/seastar/build-dbg/seastar.pc --static) -o t-asan
It crashes like this:
$ export ASAN_OPTIONS=disable_coredump=0:abort_on_error=1
$ export UBSAN_OPTIONS=halt_on_error=1:abort_on_error=1
$ gdb --args ./t-asan -c 1
...
run
...
test.cc:3749:5: runtime error: member call on misaligned address 0x41b58ab3
for type 'struct awaiter', which requires 8 byte alignment
0x41b58ab3: note: pointer points here
If seastar is build with clang (CC=clang CXX=clang++ cmake...) or if the
sanitizers are disabled (-DSeastar_SANITIZE=OFF) and valgrind is used instead,
the program doesn't crash.
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #4 from Rafael Avila de Espindola --- Created attachment 48547 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48547=edit testcase
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Martin Liška changed: What|Removed |Added Ever confirmed|0 |1 Status|UNCONFIRMED |WAITING Last reconfirmed||2020-05-15 --- Comment #3 from Martin Liška --- @Rafael: Can you please create a simplified test-case?
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 --- Comment #2 from Iain Sandoe --- Hi Rafael, nor me, the coros implementation is a set of AST transforms - so is supposed to present the following code with valid trees - which would be analysed "as normal".
[Bug sanitizer/95137] Sanitizers seem to be missing support for coroutines
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95137 Jakub Jelinek changed: What|Removed |Added CC||iains at gcc dot gnu.org --- Comment #1 from Jakub Jelinek --- Nothing I'd be aware of.
