gcc -Wclobbered in PostgresMain - Mailing list pgsql-hackers

From Sergey Shinderuk
Subject gcc -Wclobbered in PostgresMain
Date
Msg-id 2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru
Whole thread Raw
Responses Re: gcc -Wclobbered in PostgresMain
List pgsql-hackers
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/
Attachment

pgsql-hackers by date:

Previous
From: Daniel Gustafsson
Date:
Subject: Re: DROP DATABASE is interruptible
Next
From: Amit Langote
Date:
Subject: Re: remaining sql/json patches