Hi, hackers!

While analyzing -Wclobbered warnings from gcc we found a true one in PostgresMain():

postgres.c: In function ‘PostgresMain’:
postgres.c:4118:25: warning: variable ‘idle_in_transaction_timeout_enabled’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered] 4118 | bool idle_in_transaction_timeout_enabled = false;
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
postgres.c:4119:25: warning: variable ‘idle_session_timeout_enabled’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
 4119 |         bool            idle_session_timeout_enabled = false;
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~

These variables must be declared volatile, because they are read after longjmp(). send_ready_for_query declared there is volatile.

Without volatile, these variables are kept in registers and restored by longjump(). I think, this is harmless because the error handling code calls disable_all_timeouts() anyway. But strictly speaking the code is invoking undefined behavior by reading those variables after longjmp(), so it's worth fixing. And for consistency with send_ready_for_query too. I believe, making them volatile doesn't affect performance in any way.

I also moved firstchar's declaration inside the loop where it's used, to make it clear that this variable needn't be volatile and is not preserved after longjmp().

Best regards,

--
Sergey Shinderuk                https://postgrespro.com/
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 01b6cc1f7d3..56d8b0814b5 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4111,12 +4111,11 @@ PostgresSingleUserMain(int argc, char *argv[],
 void
 PostgresMain(const char *dbname, const char *username)
 {
-	int			firstchar;
 	StringInfoData input_message;
 	sigjmp_buf	local_sigjmp_buf;
 	volatile bool send_ready_for_query = true;
-	bool		idle_in_transaction_timeout_enabled = false;
-	bool		idle_session_timeout_enabled = false;
+	volatile bool idle_in_transaction_timeout_enabled = false;
+	volatile bool idle_session_timeout_enabled = false;
 
 	Assert(dbname != NULL);
 	Assert(username != NULL);
@@ -4418,6 +4417,8 @@ PostgresMain(const char *dbname, const char *username)
 
 	for (;;)
 	{
+		int			firstchar;
+
 		/*
 		 * At top of loop, reset extended-query-message flag, so that any
 		 * errors encountered in "idle" state don't provoke skip.

Reply via email to