I'm looking at the full retval cleanup patch and plan to do that next/soon, but it is much more involved, so i think it is good to land this patch first and remove the ICE and double-destroy behavior. (The other fix requires either manually putting in more cleanup code, or rearranging how the current cleanup code get emitted and where contracts are generated, which is double the work because a coroutine ramp function basically goes down a totally different code path. I've got a prototype working, and will hopefully have a patch prepared next week.)
Changes since v1: I've updated the comment with a FIXME and reference to the other bug report. On Thu, Sep 17, 2026 at 7:27 PM Jason Merrill <[email protected]> wrote: > On 9/17/26 6:58 PM, Joshua Berne wrote: > > > > > > On Thu, Sep 17, 2026 at 6:36 PM Jason Merrill <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 9/15/26 10:42 PM, Joshua Berne wrote: > > > Attached is a patch for PR127281. I added testing to an existing > > test > > > in order to explore more adjacent corners and capture a runtime > > failure > > > that was hiding behind the ICE. When there were two effective > > blocks in > > > a function the compiler ICEd because of two conflicting > > declarations for > > > the return value cleanup sentinel. With no intervening > declaration, > > > there were two destructors calls if an exception escaped the > > destruction > > > of temporaries. > > > > > > There's another followup bug (127414) that requires additional > > cleanup > > > steps to be added to handle proper exceptions escaping from > > > postcondition evaluations. Right now we leak or double destroy, > > this > > > fix makes it all just leak, and the fix to 127414 looks like it > will > > > depend on this fix as well. > > > > > > I ran the full dg test suite (check=g++ and check-gcc) before and > > > aftetr, no new tests failed and 3 new tests passed (the runtime > > testing > > > that i added for the existing test). > > > > Hmm, this seems like a bandaid until the future 127414 patch actually > > fixes the interaction between contracts and the return cleanup. I'm > > open to applying it anyway, but both the commit message and the code > > comment should make that clear. > > > > > > I think it's more of a subset of that fix (that is separable) than a > > bandaid. The 127414 fix seems like it will require adding in the > > cleanup guards in the right places around the created set of contract > > evaluation statements, (in 2 places) and that still requires supressing > > the extra adding of those guards by do_poplevel (which will just > > inevitably ice or double-destroy if left there). > > I imagined it moving the contract evaluation inside the existing guards > rather than duplicating them, but in any case it's true that you need to > avoid duplicate guards somehow, and this is a straightforward way to do > that. > > This fix also impacts anyone using contracts (due to the ICE), while > > 127414 will only impact those using a throwing violation handler. > > > > Either way, i'm happy to improve the commit message and comment, should > > i reference 127414 in both? > Well, it's already in the commit message, so just need to add it to the > comment. > > Jason > >
v2-0001-c-fix-return-object-cleanup-for-functions-with-co.patch
Description: Binary data
