From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Silencing upcoming warning about stack_base_ptr
Date: Thu, 17 Feb 2022 18:44:27 -0500
Message-ID: <3773792.1645141467@sss.pgh.pa.us> (raw)
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
Attachments:fix-stack-base-warning.patch (text/x-diff, 4.6K) downloaddiff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.cindex 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.cindex 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.cindex 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.hindex 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);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
Subject: Re: Silencing upcoming warning about stack_base_ptr
In-Reply-To: <3773792.1645141467@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox