Set the stack_base_ptr in main(), not in random other places.
authorTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Dec 2024 17:08:39 +0000 (12:08 -0500)
committerTom Lane <tgl@sss.pgh.pa.us>
Tue, 17 Dec 2024 17:08:42 +0000 (12:08 -0500)
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

src/backend/main/main.c
src/backend/postmaster/postmaster.c
src/backend/tcop/postgres.c
src/backend/utils/init/miscinit.c
src/backend/utils/misc/Makefile
src/backend/utils/misc/meson.build
src/backend/utils/misc/stack_depth.c [new file with mode: 0644]
src/include/miscadmin.h
src/include/tcop/tcopprot.h

index 2d98d97e8d862a1df4857f4f9c4a4f6299371947..864714107cb7eb80fb2a2073fef371b928839848 100644 (file)
@@ -113,6 +113,12 @@ main(int argc, char *argv[])
        MyProcPid = getpid();
        MemoryContextInit();
 
+       /*
+        * Set reference point for stack-depth checking.  (There's no point in
+        * enabling this before error reporting works.)
+        */
+       (void) set_stack_base();
+
        /*
         * Set up locale information
         */
index 6f37822c8875a3f0f27e84818650f3e964ebaaa9..6f849ffbcb5d871428dc7d1a9d6553ef5e2ec367 100644 (file)
@@ -985,11 +985,6 @@ PostmasterMain(int argc, char *argv[])
         */
        set_max_safe_fds();
 
-       /*
-        * Set reference point for stack-depth checking.
-        */
-       (void) set_stack_base();
-
        /*
         * Initialize pipe (or process handle on Windows) that allows children to
         * wake up from sleep on postmaster death.
index e0a603f42bbce2892ee7802efe4edc9cddaceb15..3ce088f0bad7c39bfd1de4273f1019ab4b04a454 100644 (file)
@@ -94,9 +94,6 @@ bool          Log_disconnections = false;
 
 int                    log_statement = LOGSTMT_NONE;
 
-/* GUC variable for maximum stack depth (measured in kilobytes) */
-int                    max_stack_depth = 100;
-
 /* wait N seconds to allow attach from a debugger */
 int                    PostAuthDelay = 0;
 
@@ -124,15 +121,6 @@ typedef struct BindParamCbData
  * ----------------
  */
 
-/* max_stack_depth converted to bytes for speed of checking */
-static long max_stack_depth_bytes = 100 * 1024L;
-
-/*
- * Stack base pointer -- initialized by PostmasterMain and inherited by
- * subprocesses (but see also InitPostmasterChild).
- */
-static char *stack_base_ptr = NULL;
-
 /*
  * Flag to keep track of whether we have started a transaction.
  * For extended query protocol this has to be remembered across messages.
@@ -3513,133 +3501,6 @@ ProcessInterrupts(void)
                HandleParallelApplyMessages();
 }
 
-/*
- * set_stack_base: set up reference point for stack depth checking
- *
- * Returns the old reference point, if any.
- */
-pg_stack_base_t
-set_stack_base(void)
-{
-#ifndef HAVE__BUILTIN_FRAME_ADDRESS
-       char            stack_base;
-#endif
-       pg_stack_base_t old;
-
-       old = stack_base_ptr;
-
-       /*
-        * Set up reference point for stack depth checking.  On recent gcc we use
-        * __builtin_frame_address() to avoid a warning about storing a local
-        * variable's address in a long-lived variable.
-        */
-#ifdef HAVE__BUILTIN_FRAME_ADDRESS
-       stack_base_ptr = __builtin_frame_address(0);
-#else
-       stack_base_ptr = &stack_base;
-#endif
-
-       return old;
-}
-
-/*
- * restore_stack_base: restore reference point for stack depth checking
- *
- * This can be used after set_stack_base() to restore the old value. This
- * is currently only used in PL/Java. When PL/Java calls a backend function
- * from different thread, the thread's stack is at a different location than
- * the main thread's stack, so it sets the base pointer before the call, and
- * restores it afterwards.
- */
-void
-restore_stack_base(pg_stack_base_t base)
-{
-       stack_base_ptr = base;
-}
-
-/*
- * check_stack_depth/stack_is_too_deep: check for excessively deep recursion
- *
- * This should be called someplace in any recursive routine that might possibly
- * recurse deep enough to overflow the stack.  Most Unixen treat stack
- * overflow as an unrecoverable SIGSEGV, so we want to error out ourselves
- * before hitting the hardware limit.
- *
- * 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.
- */
-void
-check_stack_depth(void)
-{
-       if (stack_is_too_deep())
-       {
-               ereport(ERROR,
-                               (errcode(ERRCODE_STATEMENT_TOO_COMPLEX),
-                                errmsg("stack depth limit exceeded"),
-                                errhint("Increase the configuration parameter \"max_stack_depth\" (currently %dkB), "
-                                                "after ensuring the platform's stack depth limit is adequate.",
-                                                max_stack_depth)));
-       }
-}
-
-bool
-stack_is_too_deep(void)
-{
-       char            stack_top_loc;
-       long            stack_depth;
-
-       /*
-        * Compute distance from reference point to my local variables
-        */
-       stack_depth = (long) (stack_base_ptr - &stack_top_loc);
-
-       /*
-        * Take abs value, since stacks grow up on some machines, down on others
-        */
-       if (stack_depth < 0)
-               stack_depth = -stack_depth;
-
-       /*
-        * Trouble?
-        *
-        * The test on stack_base_ptr prevents us from erroring out if called
-        * during process setup or in a non-backend process.  Logically it should
-        * be done first, but putting it here avoids wasting cycles during normal
-        * cases.
-        */
-       if (stack_depth > max_stack_depth_bytes &&
-               stack_base_ptr != NULL)
-               return true;
-
-       return false;
-}
-
-/* GUC check hook for max_stack_depth */
-bool
-check_max_stack_depth(int *newval, void **extra, GucSource source)
-{
-       long            newval_bytes = *newval * 1024L;
-       long            stack_rlimit = get_stack_depth_rlimit();
-
-       if (stack_rlimit > 0 && newval_bytes > stack_rlimit - STACK_DEPTH_SLOP)
-       {
-               GUC_check_errdetail("\"max_stack_depth\" must not exceed %ldkB.",
-                                                       (stack_rlimit - STACK_DEPTH_SLOP) / 1024L);
-               GUC_check_errhint("Increase the platform's stack depth limit via \"ulimit -s\" or local equivalent.");
-               return false;
-       }
-       return true;
-}
-
-/* GUC assign hook for max_stack_depth */
-void
-assign_max_stack_depth(int newval, void *extra)
-{
-       long            newval_bytes = newval * 1024L;
-
-       max_stack_depth_bytes = newval_bytes;
-}
-
 /*
  * GUC check_hook for client_connection_check_interval
  */
@@ -5099,40 +4960,6 @@ forbidden_in_wal_sender(char firstchar)
 }
 
 
-/*
- * Obtain platform stack depth limit (in bytes)
- *
- * Return -1 if unknown
- */
-long
-get_stack_depth_rlimit(void)
-{
-#if defined(HAVE_GETRLIMIT)
-       static long val = 0;
-
-       /* This won't change after process launch, so check just once */
-       if (val == 0)
-       {
-               struct rlimit rlim;
-
-               if (getrlimit(RLIMIT_STACK, &rlim) < 0)
-                       val = -1;
-               else if (rlim.rlim_cur == RLIM_INFINITY)
-                       val = LONG_MAX;
-               /* rlim_cur is probably of an unsigned type, so check for overflow */
-               else if (rlim.rlim_cur >= LONG_MAX)
-                       val = LONG_MAX;
-               else
-                       val = rlim.rlim_cur;
-       }
-       return val;
-#else
-       /* On Windows we set the backend stack size in src/backend/Makefile */
-       return WIN32_STACK_RLIMIT;
-#endif
-}
-
-
 static struct rusage Save_r;
 static struct timeval Save_t;
 
index 3b7b2ebec08d6cf9da3be7cc2713763d473f3cc2..d24ac133fb7faa73e22126fb138c78aa529c73c5 100644 (file)
@@ -106,13 +106,6 @@ InitPostmasterChild(void)
        pgwin32_signal_initialize();
 #endif
 
-       /*
-        * Set reference point for stack-depth checking.  This might seem
-        * redundant in !EXEC_BACKEND builds, but it's better to keep the depth
-        * logic the same with and without that build option.
-        */
-       (void) set_stack_base();
-
        InitProcessGlobals();
 
        /*
index d9f59785b9824064d308383fc457eed1d2fa5132..b362ae437710d1257399827e4160a9595265a6db 100644 (file)
@@ -29,6 +29,7 @@ OBJS = \
        queryenvironment.o \
        rls.o \
        sampling.o \
+       stack_depth.o \
        superuser.o \
        timeout.o \
        tzparser.o
index 66695022052249580b776ae012ca8eb1ddb84420..51dea174d2b262ea973dfe654179bd6962eaceb5 100644 (file)
@@ -14,6 +14,7 @@ backend_sources += files(
   'queryenvironment.c',
   'rls.c',
   'sampling.c',
+  'stack_depth.c',
   'superuser.c',
   'timeout.c',
   'tzparser.c',
diff --git a/src/backend/utils/misc/stack_depth.c b/src/backend/utils/misc/stack_depth.c
new file mode 100644 (file)
index 0000000..6bc02ea
--- /dev/null
@@ -0,0 +1,197 @@
+/*-------------------------------------------------------------------------
+ *
+ * stack_depth.c
+ *       Functions for monitoring and limiting process stack depth
+ *
+ * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ *
+ * IDENTIFICATION
+ *       src/backend/utils/misc/stack_depth.c
+ *
+ *-------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include <limits.h>
+#include <sys/resource.h>
+
+#include "miscadmin.h"
+#include "utils/guc_hooks.h"
+
+
+/* GUC variable for maximum stack depth (measured in kilobytes) */
+int                    max_stack_depth = 100;
+
+/* max_stack_depth converted to bytes for speed of checking */
+static long max_stack_depth_bytes = 100 * 1024L;
+
+/*
+ * Stack base pointer -- initialized by set_stack_base(), which
+ * should be called from main().
+ */
+static char *stack_base_ptr = NULL;
+
+
+/*
+ * set_stack_base: set up reference point for stack depth checking
+ *
+ * Returns the old reference point, if any.
+ */
+pg_stack_base_t
+set_stack_base(void)
+{
+#ifndef HAVE__BUILTIN_FRAME_ADDRESS
+       char            stack_base;
+#endif
+       pg_stack_base_t old;
+
+       old = stack_base_ptr;
+
+       /*
+        * Set up reference point for stack depth checking.  On recent gcc we use
+        * __builtin_frame_address() to avoid a warning about storing a local
+        * variable's address in a long-lived variable.
+        */
+#ifdef HAVE__BUILTIN_FRAME_ADDRESS
+       stack_base_ptr = __builtin_frame_address(0);
+#else
+       stack_base_ptr = &stack_base;
+#endif
+
+       return old;
+}
+
+/*
+ * restore_stack_base: restore reference point for stack depth checking
+ *
+ * This can be used after set_stack_base() to restore the old value. This
+ * is currently only used in PL/Java. When PL/Java calls a backend function
+ * from different thread, the thread's stack is at a different location than
+ * the main thread's stack, so it sets the base pointer before the call, and
+ * restores it afterwards.
+ */
+void
+restore_stack_base(pg_stack_base_t base)
+{
+       stack_base_ptr = base;
+}
+
+
+/*
+ * check_stack_depth/stack_is_too_deep: check for excessively deep recursion
+ *
+ * This should be called someplace in any recursive routine that might possibly
+ * recurse deep enough to overflow the stack.  Most Unixen treat stack
+ * overflow as an unrecoverable SIGSEGV, so we want to error out ourselves
+ * before hitting the hardware limit.
+ *
+ * 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.
+ */
+void
+check_stack_depth(void)
+{
+       if (stack_is_too_deep())
+       {
+               ereport(ERROR,
+                               (errcode(ERRCODE_STATEMENT_TOO_COMPLEX),
+                                errmsg("stack depth limit exceeded"),
+                                errhint("Increase the configuration parameter \"max_stack_depth\" (currently %dkB), "
+                                                "after ensuring the platform's stack depth limit is adequate.",
+                                                max_stack_depth)));
+       }
+}
+
+bool
+stack_is_too_deep(void)
+{
+       char            stack_top_loc;
+       long            stack_depth;
+
+       /*
+        * Compute distance from reference point to my local variables
+        */
+       stack_depth = (long) (stack_base_ptr - &stack_top_loc);
+
+       /*
+        * Take abs value, since stacks grow up on some machines, down on others
+        */
+       if (stack_depth < 0)
+               stack_depth = -stack_depth;
+
+       /*
+        * Trouble?
+        *
+        * The test on stack_base_ptr prevents us from erroring out if called
+        * before that's been set.  Logically it should be done first, but putting
+        * it last avoids wasting cycles during normal cases.
+        */
+       if (stack_depth > max_stack_depth_bytes &&
+               stack_base_ptr != NULL)
+               return true;
+
+       return false;
+}
+
+
+/* GUC check hook for max_stack_depth */
+bool
+check_max_stack_depth(int *newval, void **extra, GucSource source)
+{
+       long            newval_bytes = *newval * 1024L;
+       long            stack_rlimit = get_stack_depth_rlimit();
+
+       if (stack_rlimit > 0 && newval_bytes > stack_rlimit - STACK_DEPTH_SLOP)
+       {
+               GUC_check_errdetail("\"max_stack_depth\" must not exceed %ldkB.",
+                                                       (stack_rlimit - STACK_DEPTH_SLOP) / 1024L);
+               GUC_check_errhint("Increase the platform's stack depth limit via \"ulimit -s\" or local equivalent.");
+               return false;
+       }
+       return true;
+}
+
+/* GUC assign hook for max_stack_depth */
+void
+assign_max_stack_depth(int newval, void *extra)
+{
+       long            newval_bytes = newval * 1024L;
+
+       max_stack_depth_bytes = newval_bytes;
+}
+
+/*
+ * Obtain platform stack depth limit (in bytes)
+ *
+ * Return -1 if unknown
+ */
+long
+get_stack_depth_rlimit(void)
+{
+#if defined(HAVE_GETRLIMIT)
+       static long val = 0;
+
+       /* This won't change after process launch, so check just once */
+       if (val == 0)
+       {
+               struct rlimit rlim;
+
+               if (getrlimit(RLIMIT_STACK, &rlim) < 0)
+                       val = -1;
+               else if (rlim.rlim_cur == RLIM_INFINITY)
+                       val = LONG_MAX;
+               /* rlim_cur is probably of an unsigned type, so check for overflow */
+               else if (rlim.rlim_cur >= LONG_MAX)
+                       val = LONG_MAX;
+               else
+                       val = rlim.rlim_cur;
+       }
+       return val;
+#else
+       /* On Windows we set the backend stack size in src/backend/Makefile */
+       return WIN32_STACK_RLIMIT;
+#endif
+}
index 42a2b38cac98837d29a9463df3499ecb28a34031..3f97fcef800175b1c8e8187d7f7b3ea43afa6210 100644 (file)
@@ -288,7 +288,12 @@ extern PGDLLIMPORT int VacuumCostBalance;
 extern PGDLLIMPORT bool VacuumCostActive;
 
 
-/* in tcop/postgres.c */
+/* in utils/misc/stack_depth.c */
+
+extern PGDLLIMPORT int max_stack_depth;
+
+/* Required daylight between max_stack_depth and the kernel limit, in bytes */
+#define STACK_DEPTH_SLOP (512 * 1024L)
 
 typedef char *pg_stack_base_t;
 
@@ -296,6 +301,7 @@ extern pg_stack_base_t set_stack_base(void);
 extern void restore_stack_base(pg_stack_base_t base);
 extern void check_stack_depth(void);
 extern bool stack_is_too_deep(void);
+extern long get_stack_depth_rlimit(void);
 
 /* in tcop/utility.c */
 extern void PreventCommandIfReadOnly(const char *cmdname);
index 0c36d927425cf07287fbd31291d02d0d34bfebc0..3c6ed917e174b42ec5ed4fc5ba4ebe0efb78bd30 100644 (file)
 #include "utils/queryenvironment.h"
 
 
-/* Required daylight between max_stack_depth and the kernel limit, in bytes */
-#define STACK_DEPTH_SLOP (512 * 1024L)
-
 extern PGDLLIMPORT CommandDest whereToSendOutput;
 extern PGDLLIMPORT const char *debug_query_string;
-extern PGDLLIMPORT int max_stack_depth;
 extern PGDLLIMPORT int PostAuthDelay;
 extern PGDLLIMPORT int client_connection_check_interval;
 
@@ -86,7 +82,6 @@ extern void PostgresSingleUserMain(int argc, char *argv[],
                                                                   const char *username) pg_attribute_noreturn();
 extern void PostgresMain(const char *dbname,
                                                 const char *username) pg_attribute_noreturn();
-extern long get_stack_depth_rlimit(void);
 extern void ResetUsage(void);
 extern void ShowUsage(const char *title);
 extern int     check_log_duration(char *msec_str, bool was_logged);