From: | Sergey Shinderuk <s(dot)shinderuk(at)postgrespro(dot)ru> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | gcc -Wclobbered in PostgresMain |
Date: | 2023-07-07 12:13:14 |
Message-ID: | 2eda015b-7dff-47fd-d5e2-f1a9899b90a6@postgrespro.ru |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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 | Content-Type | Size |
---|---|---|
missing-volatile-longjmp.diff | text/x-patch | 962 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Langote | 2023-07-07 12:19:40 | Re: remaining sql/json patches |
Previous Message | Daniel Gustafsson | 2023-07-07 12:09:08 | Re: DROP DATABASE is interruptible |