Nicely exiting PG_TRY and PG_CATCH
Mikhail Gribkov <youzhick@gmail.com>
From: Mikhail Gribkov <youzhick@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Date: 2022-10-07T07:27:26Z
Lists: pgsql-hackers
Hi hackers,
I've found some odd lines in plpython-related code. These look to me like a
potential source of problems, but maybe I'm not fully aware of some nuances.
Usually it's not a good idea to exit PG_TRY() block via return statement.
Otherwise it would leave PG_exception_stack global variable in a wrong
state and next ereport() will jump to some junk address. But here it is a
straight return in plpy_exec.c:
PG_TRY();
{
args = PyList_New(proc->nargs);
if (!args)
return NULL;
...
(
https://github.com/postgres/postgres/blob/0fe954c28584169938e5c0738cfaa9930ce77577/src/pl/plpython/plpy_exec.c#L421
)
Two more cases could be found further in the same file:
https://github.com/postgres/postgres/blob/0fe954c28584169938e5c0738cfaa9930ce77577/src/pl/plpython/plpy_exec.c#L697
https://github.com/postgres/postgres/blob/0fe954c28584169938e5c0738cfaa9930ce77577/src/pl/plpython/plpy_exec.c#L842
Isn't it a problem?
Another suspicious case is PG_CATCH block in jsonb_plpython.c:
PG_CATCH();
{
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
errmsg("could not convert value \"%s\" to jsonb", str)));
}
(
https://github.com/postgres/postgres/blob/0fe954c28584169938e5c0738cfaa9930ce77577/contrib/jsonb_plpython/jsonb_plpython.c#L372
)
The problem is that leaving PG_CATCH() without PG_RE_THROW(),
ReThrowError() or FlushErrorState() will consume an errordata stack slot,
while the stack size is only 5. Do this five times and we'll have a PANIC
on the next ereport().
As it's stated in elog.c (comment about the error stack depth at the
beginning of FlushErrorState()), "The only case where it would be more than
one deep is if we serviced an error that interrupted construction of
another message."
Looks to me that FlushErrorState() should be inserted before this ereport.
Shouldn't it?
--
best regards,
Mikhail A. Gribkov
Commits
-
Move return statements out of PG_TRY blocks.
- 0af386b0f4b8 11.20 landed
- 24964394a972 12.15 landed
- 800531846ef7 13.11 landed
- 52c9cf3239b8 14.8 landed
- 825ebc9848fd 15.3 landed
- 57d0051706b8 16.0 landed
-
Add -Wshadow=compatible-local to the standard compilation flags
- 0fe954c28584 16.0 cited