There are at least two bugs in reorderbuffer.c's ReorderBufferCommit():
1. Although "iterstate" is modified within the PG_TRY segment and
referenced within the PG_CATCH segment, it is not marked "volatile".
This means that its value upon reaching the PG_CATCH segment is
indeterminate. In practice, what can happen is that it gets set back
to its value at the time of reaching PG_TRY, which will always be NULL,
so that the effect would be to miss out calling
ReorderBufferIterTXNFinish in the CATCH code.
2. On the other hand, if we get past the ReorderBufferIterTXNFinish
call within the PG_TRY block and then suffer a failure,
ReorderBufferIterTXNFinish will be called again in the PG_CATCH block.
This is due to failure to reset iterstate to NULL after the finish call.
(So this error could be masked if the compiler did cause iterstate
to revert to NULL during longjmp.)
I'm not sure whether #1 is harmless, but #2 most certainly isn't, as
it would result in access to already-freed memory.
The first of these was pointed out to me by Mark Wilding of Salesforce.
It's really pretty distressing that modern versions of gcc don't warn
about this (not even with -Wclobbered). The very ancient gcc on "gaur"
does warn, but my experience is that it emits a lot of false positives
too, so I'm not that eager anymore to plaster "volatile" on any variable
it whinges about. Still, it sure looks like we need a "volatile" here.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
