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);