While thinking about the Perl circular-reference problem reported by
Aleksander Alekseev [1], I realized that plperl's plperl_sv_to_datum()
is exposed to the same issue. It looks like:
static Datum
plperl_sv_to_datum(SV *sv, Oid typid, int32 typmod,
...
{
/* we might recurse */
check_stack_depth();
...
else if (SvROK(sv))
{
...
/*
* If it's a reference to something else, such as a scalar, just
* recursively look through the reference.
*/
return plperl_sv_to_datum(SvRV(sv), typid, typmod,
fcinfo, finfo, typioparam,
isnull);
That recursive call is a tail recursion. If the compiler is smart
enough to optimize that into a loop, then the stack doesn't grow and
the check_stack_depth() call won't get us out of the infinite loop.
I tried to demonstrate this with:
create type mytype as (a int);
create or replace function hashit(int) returns mytype
language plperl as
$$
# my $h = {a => $_[0]};
my $h;
$h = \\$h;
return $h;
$$;
select hashit(42);
With gcc 14.3.1, I get a "stack depth limit exceeded" error,
indicating that that compiler doesn't spot the tail recursion
opportunity. But with clang 21.0.0 (macOS' current compiler),
this is indeed an uninterruptible infinite loop.
A narrow fix would be to insert CHECK_FOR_INTERRUPTS() into
plperl_sv_to_datum(), mimicking what we did in da82fbb8f and
c0f17b04d. However, now that I've seen this, it seems likely to me
that other recursive routines that rely on check_stack_depth()
might have a similar issue. There are a fair number that look
like their recursive calls are tail-recursion-optimizable.
It's not really a problem unless the input data structure can
contain a self-referential loop, but how much faith do we want
to place in there not being one?
So what I'm considering is putting CHECK_FOR_INTERRUPTS() into
check_stack_depth() itself, more or less as attached. That's kind
of ugly from a modularity/separation-of-concerns standpoint, but it
ensures that we don't miss any cases where this could be an issue.
A larger question is whether this is a good enough answer,
or whether callers that are at risk of this need to expend
more effort on detecting input circularity. In the PL/Perl case
that started this, I judged that we didn't need to do more, but
that conclusion might not hold for still-hypothetical other
callers. However, something like this could be a good backstop
anyway.
regards, tom lane
[1]
https://www.postgresql.org/message-id/flat/CAJ7c6TPbjkzUk4qJ5dHvDNEz0hBuFue3A-XWz_%3D897z%2BBC%2Bz8A%40mail.gmail.com
diff --git a/src/backend/utils/misc/stack_depth.c b/src/backend/utils/misc/stack_depth.c
index 914a4393bf3..986872cd291 100644
--- a/src/backend/utils/misc/stack_depth.c
+++ b/src/backend/utils/misc/stack_depth.c
@@ -91,6 +91,14 @@ restore_stack_base(pg_stack_base_t base)
*
* check_stack_depth() just throws an error summarily. stack_is_too_deep()
* can be used by code that wants to handle the error condition itself.
+ *
+ * Some recursive routines that use this may not actually consume increasing
+ * stack space, because their recursive calls can be optimized into loops
+ * via tail recursion. In such cases there's a hazard that a referential
+ * loop in the input data could result in an uninterruptible infinite loop.
+ * Rather than expend effort trying to reason about whether that's possible
+ * in specific cases, we put a CHECK_FOR_INTERRUPTS() call here. Note that
+ * direct users of stack_is_too_deep() need to consider that for themselves.
*/
void
check_stack_depth(void)
@@ -104,6 +112,8 @@ check_stack_depth(void)
"after ensuring the platform's stack depth limit is adequate.",
max_stack_depth)));
}
+
+ CHECK_FOR_INTERRUPTS();
}
bool
