Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Jan 25, 2015 at 2:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> This is scary as hell. I intend to go around and manually audit
>> every single PG_TRY in the current source code, but that is obviously
>> not a long-term solution. Anybody have an idea about how we might
>> get trustworthy mechanical detection of this type of situation?
> One idea I've been thinking about for a while is to create some new,
> safer notation. Suppose we did this:
> PG_TRY_WITH_CLEANUP(cleanup_function, cleanup_argument);
> {
> /* code requiring cleanup */
> }
> PG_END_TRY_WITH_CLEANUP();
> Instead of doing anything with sigsetjmp(), this would just push a
> frame onto a cleanup stack. We would call of those callbacks from
> innermost to outermost before doing siglongjmp(). With this design,
> we don't need any volatile-ization.
Maybe not, but the notational pain of exposing everything that the
cleanup_function needs to see would be substantial. We have basically
this design already with PG_ENSURE_ERROR_CLEANUP, and that's a PITA to
use.
Also and perhaps more to the point, I'm no longer convinced that this sort
of thing doesn't require any volatile markers. The fundamental problem
we're hitting with PG_TRY is that the compiler is optimizing on the
assumption that no "unexpected" touches/changes of local variables can
happen as a result of unexpected control flow. I think it might still be
willing to optimize away superficially-dead stores even if you structure
stuff as above. We need to take a closer look at the uses of
PG_ENSURE_ERROR_CLEANUP as well ...
regards, tom lane