Re: segfault in HEAD when too many nested functions call

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>
Subject: Re: segfault in HEAD when too many nested functions call
Date: 2017-07-15 15:22:37
Message-ID: 12302.1500132157@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Julien Rouhaud <julien(dot)rouhaud(at)dalibo(dot)com> writes:
> Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you
> write queries which result in infinite recursion (or just too many
> nested function calls), execution ends with segfault instead of intended
> exhausted max_stack_depth:

Yes. We discussed this before the patch went in [1]. I wanted to put
a stack depth check in ExecProcNode(), and still do. Andres demurred,
claiming that that was too much overhead, but didn't really provide a
credible alternative. The thread drifted off without any resolution,
but clearly we need to do something before 10.0 final.

> Please find attached a trivial patch to fix this. I'm not sure
> ExecMakeTableFunctionResult() is the best or only place that needs to
> check the stack depth.

I don't think that that's adequate at all.

I experimented with a variant that doesn't go through
ExecMakeTableFunctionResult:

CREATE OR REPLACE FUNCTION so()
RETURNS int
LANGUAGE plpgsql
AS $$
DECLARE
rec RECORD;
BEGIN
FOR rec IN SELECT so() as x
LOOP
RETURN rec.x;
END LOOP;
RETURN NULL;
END;
$$;

SELECT so();

This manages not to crash, but I think the reason it does not is purely
accidental: "SELECT so()" has a nontrivial targetlist so we end up running
ExecBuildProjectionInfo on that, meaning that a fresh expression
compilation happens at every nesting depth, and there are
check_stack_depth calls in expression compilation. Surely that's
something we'd try to improve someday. Or it could accidentally get
broken by unrelated changes in the way plpgsql sets up queries to be
executed.

I still think that we really need to add a check in ExecProcNode().
Even if there's an argument to be made that every recursion would
somewhere go through ExecMakeTableFunctionResult, very large/complex
queries could result in substantial stack getting chewed up before
we get to that --- and we don't have an infinite amount of stack slop.

regards, tom lane

[1] https://www.postgresql.org/message-id/22833.1490390175@sss.pgh.pa.us

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-07-15 15:57:24 Re: segfault in HEAD when too many nested functions call
Previous Message Michael Paquier 2017-07-15 09:15:12 Re: Adding -E switch to pg_dumpall