BUG #17113: Assert failed on calling a function fixed after an extension reload
The following bug has been logged on the website:
Bug reference: 17113
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 13.3
Operating system: Ubuntu 20.04
Description:
The following SQL snippet:
CREATE EXTENSION cube;
CREATE FUNCTION f1(c1 text, c2 text) RETURNS float8 AS $$
SELECT cube_distance(cube(c1::text), cube(c2::text));
$$ LANGUAGE sql;
CREATE FUNCTION f2(c1 text, c2 text) RETURNS float8 AS $$
DECLARE
res float8;
BEGIN
SELECT f1(c1, c2) INTO res;
RETURN res;
END;
$$ LANGUAGE plpgsql;
DROP EXTENSION cube;
SELECT f2('(0)', '(1)');
CREATE EXTENSION cube;
SELECT f2('(0)', '(1)');
leads to a failed assertion with the following stack:
Core was generated by `postgres: law regression [local] SELECT
'.
Program terminated with signal SIGABRT, Aborted.
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
(gdb) bt
#0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1 0x00007fc7d3326859 in __GI_abort () at abort.c:79
#2 0x000055a76178b342 in ExceptionalCondition
(conditionName=conditionName@entry=0x7fc7c41286e3 "!stmt->mod_stmt",
errorType=errorType@entry=0x7fc7c4126015 "FailedAssertion",
fileName=0x7ffce51ce2a0 "#\263xa\247U",
fileName@entry=0x7fc7c4128348 "pl_exec.c",
lineNumber=lineNumber@entry=4225) at assert.c:69
#3 0x00007fc7c4118111 in exec_stmt_execsql
(estate=estate@entry=0x7ffce51ce850, stmt=stmt@entry=0x55a763a29210)
at pl_exec.c:4225
#4 0x00007fc7c41191e0 in exec_stmts (estate=estate@entry=0x7ffce51ce850,
stmts=0x55a763a29268) at pl_exec.c:2059
#5 0x00007fc7c41198a8 in exec_stmt_block
(estate=estate@entry=0x7ffce51ce850, block=block@entry=0x55a763a29340)
at pl_exec.c:1910
#6 0x00007fc7c411998e in exec_toplevel_block
(estate=estate@entry=0x7ffce51ce850, block=0x55a763a29340)
at pl_exec.c:1608
#7 0x00007fc7c4119c88 in plpgsql_exec_function
(func=func@entry=0x55a763a76da8, fcinfo=fcinfo@entry=0x55a763a9f0a8,
simple_eval_estate=simple_eval_estate@entry=0x0,
simple_eval_resowner=simple_eval_resowner@entry=0x0,
procedure_resowner=procedure_resowner@entry=0x0, atomic=<optimized out>)
at pl_exec.c:611
#8 0x00007fc7c4124802 in plpgsql_call_handler (fcinfo=0x55a763a9f0a8) at
pl_handler.c:277
#9 0x000055a761487629 in ExecInterpExpr (state=0x55a763a9efc0,
econtext=0x55a763a9ed00, isnull=0x7ffce51ceb67)
at execExprInterp.c:725
#10 0x000055a761483f07 in ExecInterpExprStillValid (state=0x55a763a9efc0,
econtext=0x55a763a9ed00,
isNull=0x7ffce51ceb67) at execExprInterp.c:1824
#11 0x000055a7614c39d4 in ExecEvalExprSwitchContext (isNull=0x7ffce51ceb67,
econtext=0x55a763a9ed00,
state=0x55a763a9efc0) at ../../../src/include/executor/executor.h:339
#12 ExecProject (projInfo=0x55a763a9efb8) at
../../../src/include/executor/executor.h:373
#13 ExecResult (pstate=<optimized out>) at nodeResult.c:136
#14 0x000055a76149416c in ExecProcNodeFirst (node=0x55a763a9ebe8) at
execProcnode.c:463
#15 0x000055a76148cdbc in ExecProcNode (node=0x55a763a9ebe8) at
../../../src/include/executor/executor.h:257
#16 ExecutePlan (estate=estate@entry=0x55a763a9e9b0,
planstate=0x55a763a9ebe8, use_parallel_mode=<optimized out>,
operation=operation@entry=CMD_SELECT, sendTuples=sendTuples@entry=true,
numberTuples=numberTuples@entry=0,
direction=ForwardScanDirection, dest=0x55a763a2d3b8, execute_once=true)
at execMain.c:1551
#17 0x000055a76148cf9c in standard_ExecutorRun (queryDesc=0x55a76395adc0,
direction=ForwardScanDirection, count=0,
execute_once=<optimized out>) at execMain.c:361
#18 0x000055a76148d068 in ExecutorRun
(queryDesc=queryDesc@entry=0x55a76395adc0,
direction=direction@entry=ForwardScanDirection, count=count@entry=0,
execute_once=<optimized out>) at execMain.c:305
#19 0x000055a76164d2f8 in PortalRunSelect
(portal=portal@entry=0x55a7639a9bb0, forward=forward@entry=true, count=0,
count@entry=9223372036854775807, dest=dest@entry=0x55a763a2d3b8) at
pquery.c:919
#20 0x000055a76164ecbb in PortalRun (portal=portal@entry=0x55a7639a9bb0,
count=count@entry=9223372036854775807,
isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true,
dest=dest@entry=0x55a763a2d3b8,
altdest=altdest@entry=0x55a763a2d3b8, qc=0x7ffce51cee70) at
pquery.c:763
#21 0x000055a76164ada7 in exec_simple_query
(query_string=query_string@entry=0x55a7639394b0 "SELECT f2('(0)',
'(1)');")
at postgres.c:1214
#22 0x000055a76164cd79 in PostgresMain (argc=argc@entry=1,
argv=argv@entry=0x7ffce51cf060, dbname=<optimized out>,
username=<optimized out>) at postgres.c:4486
#23 0x000055a7615a62f3 in BackendRun (port=port@entry=0x55a76395a0b0) at
postmaster.c:4506
#24 0x000055a7615a9508 in BackendStartup (port=port@entry=0x55a76395a0b0) at
postmaster.c:4228
#25 0x000055a7615a974f in ServerLoop () at postmaster.c:1745
#26 0x000055a7615aac9c in PostmasterMain (argc=3, argv=<optimized out>) at
postmaster.c:1417
#27 0x000055a7614eb752 in main (argc=3, argv=0x55a763933520) at main.c:209
reproduced on REL9_6_STABLE..master.
PG Bug reporting form <noreply@postgresql.org> writes:
The following SQL snippet:
...
leads to a failed assertion with the following stack:
Hmm ... it seems fine in v14 and HEAD, but I do see the crash in v13.
I have some errands to run, but will take a closer look later.
regards, tom lane
I wrote:
PG Bug reporting form <noreply@postgresql.org> writes:
The following SQL snippet:
...
leads to a failed assertion with the following stack:
Hmm ... it seems fine in v14 and HEAD, but I do see the crash in v13.
So actually this is an uninitialized-variable problem, which makes
it mostly luck whether it fails or not. (Turning on
RANDOMIZE_ALLOCATED_MEMORY would make it much more reproducible,
though, and I imagine valgrind would complain as well.)
The core of the issue is that this coding pattern in exec_stmt_execsql
is unsafe:
/*
* On the first call for this statement generate the plan, and detect
* whether the statement is INSERT/UPDATE/DELETE
*/
if (expr->plan == NULL)
{
exec_prepare_plan(estate, expr, CURSOR_OPT_PARALLEL_OK, true);
// compute stmt->mod_stmt here
}
It's possible for exec_prepare_plan to throw an error after setting
expr->plan, in which case if we come here again then expr->plan is
already set so we'll never compute stmt->mod_stmt at all.
Since the above is a fairly tempting coding pattern, I tried to make
it safe by rearranging things so that exec_prepare_plan wouldn't set
expr->plan until the end. That idea crashed and burned though; the
plan refcount manipulations we do don't work correctly if we do it
like that, and trying to change that looks like it'd create fairly
substantial risks of leaking plan refcounts on error.
So I ended up with the attached, which just adds some warning comments
saying "don't do it like that" and then fixes the two places that did
do it like that (exec_stmt_call is broken too).
This requires changing struct PLpgSQL_stmt_execsql, which is something
of an ABI hazard for pldebugger and the like. I think we can make it
safe in the back branches by putting the new mod_stmt_set bool after
the other three bools, though that's rather an unintuitive ordering.
For me, the new test case fails in v10..HEAD but not 9.6; but I think
that's just random chance. The bug is really quite old.
Barring objections, I'll push this tomorrow.
(Memo to self: the back branches have more variants of the same issue,
so their patches are going to look different.)
regards, tom lane