Silencing upcoming warning about stack_base_ptr - Mailing list pgsql-hackers

GCC 12, coming soon to a distro near you, complains like this:

postgres.c: In function 'set_stack_base':
postgres.c:3430:24: warning: storing the address of local variable 'stack_base' in 'stack_base_ptr'
[-Wdangling-pointer=]
 3430 |         stack_base_ptr = &stack_base;
      |         ~~~~~~~~~~~~~~~^~~~~~~~~~~~~
postgres.c:3419:25: note: 'stack_base' declared here
 3419 |         char            stack_base;
      |                         ^~~~~~~~~~
postgres.c:136:13: note: 'stack_base_ptr' declared here
  136 | char       *stack_base_ptr = NULL;
      |             ^~~~~~~~~~~~~~

(that's visible now on buildfarm member caiman).  We probably
should take some thought for silencing this before it starts
to be in people's faces during routine development.

Fixing this is a bit harder than one could wish because we export
set_stack_base() for use by PL/Java, so it would be better to not
change that API.  I ended up with the attached patch, which works
to silence the warning so long as the new subroutine
set_stack_base_from() is marked pg_noinline.  I'm a little worried
that in a year or two GCC will be smart enough to complain anyway.
If that happens, we could probably silence the warning again by
moving set_stack_base() to a different source file --- but at some
point we might have to give up and change its API, I suppose.

I also took this opportunity to re-static-ify the stack_base_ptr
variable itself, as PL/Java seems to have adopted set_stack_base
since 1.5.0.  That part perhaps should not be back-patched.

            regards, tom lane

diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 735fed490b..a05d84c0f4 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -582,6 +582,7 @@ HANDLE        PostmasterHandle;
 void
 PostmasterMain(int argc, char *argv[])
 {
+    char        stack_reference = '\0';
     int            opt;
     int            status;
     char       *userDoption = NULL;
@@ -1083,7 +1084,7 @@ PostmasterMain(int argc, char *argv[])
     /*
      * Set reference point for stack-depth checking.
      */
-    set_stack_base();
+    set_stack_base_from(&stack_reference);

     /*
      * Initialize pipe (or process handle on Windows) that allows children to
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 38d8b97894..b228948045 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -129,17 +129,15 @@ static long max_stack_depth_bytes = 100 * 1024L;

 /*
  * Stack base pointer -- initialized by PostmasterMain and inherited by
- * subprocesses. This is not static because old versions of PL/Java modify
- * it directly. Newer versions use set_stack_base(), but we want to stay
- * binary-compatible for the time being.
+ * subprocesses (but see also InitPostmasterChild).
  */
-char       *stack_base_ptr = NULL;
+static char *stack_base_ptr = NULL;

 /*
  * On IA64 we also have to remember the register stack base.
  */
 #if defined(__ia64__) || defined(__ia64)
-char       *register_stack_base_ptr = NULL;
+static char *register_stack_base_ptr = NULL;
 #endif

 /*
@@ -3408,15 +3406,36 @@ ia64_get_bsp(void)
 #endif                            /* IA64 */


+/*
+ * set_stack_base_from: set up reference point for stack depth checking
+ *
+ * The passed-in value must be the address of a local variable in the caller.
+ * Ideally the caller is the process's outermost function, but it's also
+ * acceptable for it to be not too many stack levels down from there.
+ */
+pg_noinline void
+set_stack_base_from(char *stack_reference)
+{
+    /* Set up reference point for stack depth checking */
+    stack_base_ptr = stack_reference;
+#if defined(__ia64__) || defined(__ia64)
+    register_stack_base_ptr = ia64_get_bsp();
+#endif
+}
+
 /*
  * set_stack_base: set up reference point for stack depth checking
  *
+ * This is exported for use by PL/Java.  At some point we might have to
+ * require it to take a stack_reference pointer to silence compiler warnings.
+ * For now, we don't, so don't break the API unnecessarily.
+ *
  * Returns the old reference point, if any.
  */
 pg_stack_base_t
 set_stack_base(void)
 {
-    char        stack_base;
+    char        stack_reference = '\0';
     pg_stack_base_t old;

 #if defined(__ia64__) || defined(__ia64)
@@ -3426,11 +3445,7 @@ set_stack_base(void)
     old = stack_base_ptr;
 #endif

-    /* Set up reference point for stack depth checking */
-    stack_base_ptr = &stack_base;
-#if defined(__ia64__) || defined(__ia64)
-    register_stack_base_ptr = ia64_get_bsp();
-#endif
+    set_stack_base_from(&stack_reference);

     return old;
 }
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 0868e5a24f..1cb15d54d4 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -94,6 +94,8 @@ bool        IgnoreSystemIndexes = false;
 void
 InitPostmasterChild(void)
 {
+    char        stack_reference = '\0';
+
     IsUnderPostmaster = true;    /* we are a postmaster subprocess now */

     /*
@@ -106,13 +108,14 @@ InitPostmasterChild(void)
 #endif

     /*
-     * Set reference point for stack-depth checking. We re-do that even in the
-     * !EXEC_BACKEND case, because there are some edge cases where processes
-     * are started with an alternative stack (e.g. starting bgworkers when
-     * running postgres using the rr debugger, as bgworkers are launched from
-     * signal handlers).
+     * Set reference point for stack-depth checking.  This might seem
+     * redundant in !EXEC_BACKEND builds; but it's not because the postmaster
+     * launches its children from signal handlers, so we might be running on
+     * an alternative stack.  To avoid the notational cruft of having
+     * InitPostmasterChild callers pass a reference variable, we assume that
+     * one declared here is close enough to the stack base.
      */
-    set_stack_base();
+    set_stack_base_from(&stack_reference);

     InitProcessGlobals();

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 0abc3ad540..24c17d5517 100644
--- a/src/include/miscadmin.h
+++ b/src/include/miscadmin.h
@@ -287,6 +287,7 @@ typedef struct
 typedef char *pg_stack_base_t;
 #endif

+extern void set_stack_base_from(char *stack_reference);
 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);