Обсуждение: Preventing stack-overflow crashes (improving on max_expr_depth)
We've had a couple of complaints in the past about recursive functions
crashing the server by overflowing the C execution stack. There is a
GUC variable max_expr_depth that is intended to prevent this sort of
problem for the particular case of overly complex arithmetic
expressions, but it's difficult to apply a similar solution to function
calls. A function nesting depth limit would necessarily be pretty
arbitrary, since different functions might use very different amounts of
stack space.
It occurred to me today that it would not be difficult to implement a
direct check on the physical size of the execution stack. We could have
PostgresMain do this:
/* global variables: */char *stack_base_ptr;int max_stack_size; /* settable through GUC */
PostgresMain(...){ char stack_base_loc;
stack_base_ptr = &stack_base_loc;
... server main loop here ...}
Then, in key recursive routines such as ExecQual, add a stack depth test
that looks like
void check_stack_depth(void){ char stack_top_loc;
if (abs(stack_base_ptr - &stack_top_loc) > max_stack_size) elog(ERROR, "Stack depth limit exceeded");}
Essentially we're measuring the distance between the local variables of
PostgresMain and those of the current recursive routine. (The abs()
operation is needed since the stack grows up on some machines and down
on others.)
Now, instead of a max_expr_depth variable that no one really knows how
to set intelligently, we have a max_stack_size variable that we can tell
people exactly how to set: a megabyte or so less than your "ulimit -s"
value should work fine for most cases. And it works for everything;
recursive function calls, whatever.
I imagine that somewhere in the fine print of the ANSI C standard, it
says that this maneuver doesn't give well-defined results --- but I
cannot think of any platform where it wouldn't work.
Comments?
regards, tom lane
Sounds like a great approach to me. If it doesn't work, we will find
out during beta testing.
---------------------------------------------------------------------------
Tom Lane wrote:
> We've had a couple of complaints in the past about recursive functions
> crashing the server by overflowing the C execution stack. There is a
> GUC variable max_expr_depth that is intended to prevent this sort of
> problem for the particular case of overly complex arithmetic
> expressions, but it's difficult to apply a similar solution to function
> calls. A function nesting depth limit would necessarily be pretty
> arbitrary, since different functions might use very different amounts of
> stack space.
>
> It occurred to me today that it would not be difficult to implement a
> direct check on the physical size of the execution stack. We could have
> PostgresMain do this:
>
> /* global variables: */
> char *stack_base_ptr;
> int max_stack_size; /* settable through GUC */
>
> PostgresMain(...)
> {
> char stack_base_loc;
>
> stack_base_ptr = &stack_base_loc;
>
> ... server main loop here ...
> }
>
> Then, in key recursive routines such as ExecQual, add a stack depth test
> that looks like
>
> void check_stack_depth(void)
> {
> char stack_top_loc;
>
> if (abs(stack_base_ptr - &stack_top_loc) > max_stack_size)
> elog(ERROR, "Stack depth limit exceeded");
> }
>
> Essentially we're measuring the distance between the local variables of
> PostgresMain and those of the current recursive routine. (The abs()
> operation is needed since the stack grows up on some machines and down
> on others.)
>
> Now, instead of a max_expr_depth variable that no one really knows how
> to set intelligently, we have a max_stack_size variable that we can tell
> people exactly how to set: a megabyte or so less than your "ulimit -s"
> value should work fine for most cases. And it works for everything;
> recursive function calls, whatever.
>
> I imagine that somewhere in the fine print of the ANSI C standard, it
> says that this maneuver doesn't give well-defined results --- but I
> cannot think of any platform where it wouldn't work.
>
> Comments?
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
>
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
359-1001+ If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square,
Pennsylvania19073
Bruce Momjian wrote: >Sounds like a great approach to me. If it doesn't work, we will find >out during beta testing. > > Would it make sense to also have a nice little global function and/or macro available for the author of C-language recursive functions to perform a depth test before recursing? Mike Mascari mascarm@mascari.com >Tom Lane wrote: > > >>It occurred to me today that it would not be difficult to implement a >>direct check on the physical size of the execution stack. >>
Mike Mascari <mascarm@mascari.com> writes:
> Would it make sense to also have a nice little global function and/or
> macro available for the author of C-language recursive functions to
> perform a depth test before recursing?
Yeah, I envision presenting this as a nice little macro along the lines
of CHECK_STACK_OVERFLOW(). The hard part will really be deciding where
it needs to be called.
regards, tom lane
Tom Lane wrote: > Comments? Really ugly but effective. Is ABS enough on a 64-bit architecture ? Or is better use labs ? Regards Gaetano Mendola
Gaetano Mendola <mendola@bigfoot.com> writes:
> Is ABS enough on a 64-bit architecture ?
That was pseudocode, I wasn't actually planning to rely on a function.
Something more like
long diff;
diff = stack_base_ptr - &stack_top_loc;if (diff < 0) diff = -diff;if (diff > max) elog ...
regards, tom lane
--On Wednesday, December 31, 2003 11:20:49 -0500 Tom Lane <tgl@sss.pgh.pa.us> wrote: > Gaetano Mendola <mendola@bigfoot.com> writes: >> Is ABS enough on a 64-bit architecture ? > > That was pseudocode, I wasn't actually planning to rely on a function. > Something more like > > long diff; > > diff = stack_base_ptr - &stack_top_loc; > if (diff < 0) > diff = -diff; > if (diff > max) > elog ... > One archetecture that MIGHT be an issue (It's been 5+ years) is IBM 390 class under Z/OS. I'm not sure how they do stacks.... LER > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > -- Larry Rosenman http://www.lerctr.org/~ler Phone: +1 972-414-9812 E-Mail: ler@lerctr.org US Mail: 1905 Steamboat Springs Drive, Garland, TX 75044-6749
On Wed, 31 Dec 2003, Tom Lane wrote: > > Is ABS enough on a 64-bit architecture ? > > That was pseudocode, I wasn't actually planning to rely on a function. > Something more like > > long diff; FWIW, ISO has a ptrdiff_t, which may be useful here. Matthew. > diff = stack_base_ptr - &stack_top_loc; > if (diff < 0) > diff = -diff; > if (diff > max) > elog ... > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org > >