Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit c91963d

Browse files
committed
Set the stack_base_ptr in main(), not in random other places.
Previously we did this in PostmasterMain() and InitPostmasterChild(), which meant that stack depth checking was disabled in non-postmaster server processes, for instance in single-user mode. That seems like a fairly bad idea, since there's no a-priori restriction on the complexity of queries we will run in single-user mode. Moreover, this led to not having quite the same stack depth limit in all processes, which likely has no real-world effect but it offends my inner neatnik. Setting the depth in main() guarantees that check_stack_depth() is armed and has a consistent interpretation of stack depth in all forms of server processes. While at it, move the code associated with checking the stack depth out of tcop/postgres.c (which was never a great home for it) into a new file src/backend/utils/misc/stack_depth.c. Discussion: https://postgr.es/m/2081982.1734393311@sss.pgh.pa.us
1 parent 957ba9f commit c91963d

File tree

9 files changed

+212
-191
lines changed

9 files changed

+212
-191
lines changed

src/backend/main/main.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ main(int argc, char *argv[])
113113
MyProcPid = getpid();
114114
MemoryContextInit();
115115

116+
/*
117+
* Set reference point for stack-depth checking. (There's no point in
118+
* enabling this before error reporting works.)
119+
*/
120+
(void) set_stack_base();
121+
116122
/*
117123
* Set up locale information
118124
*/

src/backend/postmaster/postmaster.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -985,11 +985,6 @@ PostmasterMain(int argc, char *argv[])
985985
*/
986986
set_max_safe_fds();
987987

988-
/*
989-
* Set reference point for stack-depth checking.
990-
*/
991-
(void) set_stack_base();
992-
993988
/*
994989
* Initialize pipe (or process handle on Windows) that allows children to
995990
* wake up from sleep on postmaster death.

src/backend/tcop/postgres.c

Lines changed: 0 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,6 @@ bool Log_disconnections = false;
9494

9595
int log_statement = LOGSTMT_NONE;
9696

97-
/* GUC variable for maximum stack depth (measured in kilobytes) */
98-
int max_stack_depth = 100;
99-
10097
/* wait N seconds to allow attach from a debugger */
10198
int PostAuthDelay = 0;
10299

@@ -124,15 +121,6 @@ typedef struct BindParamCbData
124121
* ----------------
125122
*/
126123

127-
/* max_stack_depth converted to bytes for speed of checking */
128-
static long max_stack_depth_bytes = 100 * 1024L;
129-
130-
/*
131-
* Stack base pointer -- initialized by PostmasterMain and inherited by
132-
* subprocesses (but see also InitPostmasterChild).
133-
*/
134-
static char *stack_base_ptr = NULL;
135-
136124
/*
137125
* Flag to keep track of whether we have started a transaction.
138126
* For extended query protocol this has to be remembered across messages.
@@ -3513,133 +3501,6 @@ ProcessInterrupts(void)
35133501
HandleParallelApplyMessages();
35143502
}
35153503

3516-
/*
3517-
* set_stack_base: set up reference point for stack depth checking
3518-
*
3519-
* Returns the old reference point, if any.
3520-
*/
3521-
pg_stack_base_t
3522-
set_stack_base(void)
3523-
{
3524-
#ifndef HAVE__BUILTIN_FRAME_ADDRESS
3525-
char stack_base;
3526-
#endif
3527-
pg_stack_base_t old;
3528-
3529-
old = stack_base_ptr;
3530-
3531-
/*
3532-
* Set up reference point for stack depth checking. On recent gcc we use
3533-
* __builtin_frame_address() to avoid a warning about storing a local
3534-
* variable's address in a long-lived variable.
3535-
*/
3536-
#ifdef HAVE__BUILTIN_FRAME_ADDRESS
3537-
stack_base_ptr = __builtin_frame_address(0);
3538-
#else
3539-
stack_base_ptr = &stack_base;
3540-
#endif
3541-
3542-
return old;
3543-
}
3544-
3545-
/*
3546-
* restore_stack_base: restore reference point for stack depth checking
3547-
*
3548-
* This can be used after set_stack_base() to restore the old value. This
3549-
* is currently only used in PL/Java. When PL/Java calls a backend function
3550-
* from different thread, the thread's stack is at a different location than
3551-
* the main thread's stack, so it sets the base pointer before the call, and
3552-
* restores it afterwards.
3553-
*/
3554-
void
3555-
restore_stack_base(pg_stack_base_t base)
3556-
{
3557-
stack_base_ptr = base;
3558-
}
3559-
3560-
/*
3561-
* check_stack_depth/stack_is_too_deep: check for excessively deep recursion
3562-
*
3563-
* This should be called someplace in any recursive routine that might possibly
3564-
* recurse deep enough to overflow the stack. Most Unixen treat stack
3565-
* overflow as an unrecoverable SIGSEGV, so we want to error out ourselves
3566-
* before hitting the hardware limit.
3567-
*
3568-
* check_stack_depth() just throws an error summarily. stack_is_too_deep()
3569-
* can be used by code that wants to handle the error condition itself.
3570-
*/
3571-
void
3572-
check_stack_depth(void)
3573-
{
3574-
if (stack_is_too_deep())
3575-
{
3576-
ereport(ERROR,
3577-
(errcode(ERRCODE_STATEMENT_TOO_COMPLEX),
3578-
errmsg("stack depth limit exceeded"),
3579-
errhint("Increase the configuration parameter \"max_stack_depth\" (currently %dkB), "
3580-
"after ensuring the platform's stack depth limit is adequate.",
3581-
max_stack_depth)));
3582-
}
3583-
}
3584-
3585-
bool
3586-
stack_is_too_deep(void)
3587-
{
3588-
char stack_top_loc;
3589-
long stack_depth;
3590-
3591-
/*
3592-
* Compute distance from reference point to my local variables
3593-
*/
3594-
stack_depth = (long) (stack_base_ptr - &stack_top_loc);
3595-
3596-
/*
3597-
* Take abs value, since stacks grow up on some machines, down on others
3598-
*/
3599-
if (stack_depth < 0)
3600-
stack_depth = -stack_depth;
3601-
3602-
/*
3603-
* Trouble?
3604-
*
3605-
* The test on stack_base_ptr prevents us from erroring out if called
3606-
* during process setup or in a non-backend process. Logically it should
3607-
* be done first, but putting it here avoids wasting cycles during normal
3608-
* cases.
3609-
*/
3610-
if (stack_depth > max_stack_depth_bytes &&
3611-
stack_base_ptr != NULL)
3612-
return true;
3613-
3614-
return false;
3615-
}
3616-
3617-
/* GUC check hook for max_stack_depth */
3618-
bool
3619-
check_max_stack_depth(int *newval, void **extra, GucSource source)
3620-
{
3621-
long newval_bytes = *newval * 1024L;
3622-
long stack_rlimit = get_stack_depth_rlimit();
3623-
3624-
if (stack_rlimit > 0 && newval_bytes > stack_rlimit - STACK_DEPTH_SLOP)
3625-
{
3626-
GUC_check_errdetail("\"max_stack_depth\" must not exceed %ldkB.",
3627-
(stack_rlimit - STACK_DEPTH_SLOP) / 1024L);
3628-
GUC_check_errhint("Increase the platform's stack depth limit via \"ulimit -s\" or local equivalent.");
3629-
return false;
3630-
}
3631-
return true;
3632-
}
3633-
3634-
/* GUC assign hook for max_stack_depth */
3635-
void
3636-
assign_max_stack_depth(int newval, void *extra)
3637-
{
3638-
long newval_bytes = newval * 1024L;
3639-
3640-
max_stack_depth_bytes = newval_bytes;
3641-
}
3642-
36433504
/*
36443505
* GUC check_hook for client_connection_check_interval
36453506
*/
@@ -5099,40 +4960,6 @@ forbidden_in_wal_sender(char firstchar)
50994960
}
51004961

51014962

5102-
/*
5103-
* Obtain platform stack depth limit (in bytes)
5104-
*
5105-
* Return -1 if unknown
5106-
*/
5107-
long
5108-
get_stack_depth_rlimit(void)
5109-
{
5110-
#if defined(HAVE_GETRLIMIT)
5111-
static long val = 0;
5112-
5113-
/* This won't change after process launch, so check just once */
5114-
if (val == 0)
5115-
{
5116-
struct rlimit rlim;
5117-
5118-
if (getrlimit(RLIMIT_STACK, &rlim) < 0)
5119-
val = -1;
5120-
else if (rlim.rlim_cur == RLIM_INFINITY)
5121-
val = LONG_MAX;
5122-
/* rlim_cur is probably of an unsigned type, so check for overflow */
5123-
else if (rlim.rlim_cur >= LONG_MAX)
5124-
val = LONG_MAX;
5125-
else
5126-
val = rlim.rlim_cur;
5127-
}
5128-
return val;
5129-
#else
5130-
/* On Windows we set the backend stack size in src/backend/Makefile */
5131-
return WIN32_STACK_RLIMIT;
5132-
#endif
5133-
}
5134-
5135-
51364963
static struct rusage Save_r;
51374964
static struct timeval Save_t;
51384965

src/backend/utils/init/miscinit.c

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,13 +106,6 @@ InitPostmasterChild(void)
106106
pgwin32_signal_initialize();
107107
#endif
108108

109-
/*
110-
* Set reference point for stack-depth checking. This might seem
111-
* redundant in !EXEC_BACKEND builds, but it's better to keep the depth
112-
* logic the same with and without that build option.
113-
*/
114-
(void) set_stack_base();
115-
116109
InitProcessGlobals();
117110

118111
/*

src/backend/utils/misc/Makefile

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ OBJS = \
2929
queryenvironment.o \
3030
rls.o \
3131
sampling.o \
32+
stack_depth.o \
3233
superuser.o \
3334
timeout.o \
3435
tzparser.o

src/backend/utils/misc/meson.build

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ backend_sources += files(
1414
'queryenvironment.c',
1515
'rls.c',
1616
'sampling.c',
17+
'stack_depth.c',
1718
'superuser.c',
1819
'timeout.c',
1920
'tzparser.c',

0 commit comments

Comments
 (0)