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

Tue, 30 Dec 2003 19:15:20 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Preventing stack-overflow crashes (improving on max_expr_depth)

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, Pennsylvania 19073

Tue, 30 Dec 2003 21:42:38 -0500 (EST)
Bruce Momjian <pgman@candle.pha.pa.us>
Re: Preventing stack-overflow crashes (improving on max_expr_depth)
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.  
>>



Tue, 30 Dec 2003 22:12:20 -0500
Mike Mascari <mascarm@mascari.com>
Re: Preventing stack-overflow crashes (improving on max_expr_depth)
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

Tue, 30 Dec 2003 23:19:47 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Preventing stack-overflow crashes (improving on max_expr_depth) 
Tom Lane wrote:

> Comments?

Really ugly but effective.

Is ABS enough on a 64-bit architecture ?
Or is better use labs ?


Regards
Gaetano Mendola


Wed, 31 Dec 2003 13:54:00 +0100
Gaetano Mendola <mendola@bigfoot.com>
Re: Preventing stack-overflow crashes (improving on max_expr_depth)
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

Wed, 31 Dec 2003 11:20:49 -0500
Tom Lane <tgl@sss.pgh.pa.us>
Re: Preventing stack-overflow crashes (improving on max_expr_depth) 
--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
Wed, 31 Dec 2003 10:30:02 -0600
Larry Rosenman <ler@lerctr.org>
Re: Preventing stack-overflow crashes (improving on
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
>
>

Wed, 31 Dec 2003 17:49:49 +0000 (GMT)
Matthew Kirkwood <matthew@hairy.beasts.org>
Re: Preventing stack-overflow crashes (improving on