From: | Maxim Orlov <orlovmg(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | m(dot)litsarev(at)postgrespro(dot)ru, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: Warning -Wclobbered in PG_TRY(...) |
Date: | 2025-05-29 14:45:03 |
Message-ID: | CACG=ezbo=38qYGrETTPfqbuAUqbJQCZLPBkXwvjbPvz2unb0OA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 29 May 2025 at 14:14, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> -Wclobbered would be really useful if it worked --- but sadly,
> on practically all gcc and clang versions, those warnings have
> nearly nothing to do with reality. We typically ignore them.
Yes, in the vast majority of cases. But I believe this is not one of them.
Actually, we do
not adhere to the C language standard for a setjump/logjump. Notice how the
non-volatile local variable "bool dorethrow##" utilized in the
setjump/logjump block.
This topic reminds me of [0].
In fact, we do not receive a warning for a C-lang build because the
compiler does not
appear to be "smart" enough to put this variable in a register. I ran a
synthetic test
equivalent to PG_TRY using godbolt.org, and the final code was the same
whether
the volatile qualifier was used or not. This is not the case with the C++
build. If the
volatile qualifier is not used, the compiler optimizes this line and puts
it in a register,
and we get a warning. And to get rid of this warning, you need to either
turn it off via
the pragma directive, write your own version of PG_TRY with a volatile type
qualifier,
or patch Postgres. None of these solutions sound good to me. You may be
wondering
who will write Postgres extensions in C++. Having two different runtimes is
absolute
misery, and I completely agree. However, such extensions already exist; for
example,
pg_duckdb. AFAICS, it is quite popular in our area.
So, to be safe, we should follow the standard and include a volatile
qualifier here. It
really costs us nothing; the resulting code will be identical for C, and
we'll avoid any
future issues. In the case of C++, it will also resolve the warning.
Don't consider me as something of a formalist or someone who wants to make
mountains out of molehills. I just want to make things better. PFA trivial
patch.
[0]
https://www.postgresql.org/message-id/flat/2eda015b-7dff-47fd-d5e2-f1a9899b90a6%40postgrespro.ru
--
Best regards,
Maxim Orlov.
Attachment | Content-Type | Size |
---|---|---|
v1-0001-Use-volatile-to-avoid-unspecified-behavior-in-PG_.patch | application/octet-stream | 991 bytes |
From | Date | Subject | |
---|---|---|---|
Next Message | Melanie Plageman | 2025-05-29 14:57:02 | Correcting freeze conflict horizon calculation |
Previous Message | David G. Johnston | 2025-05-29 14:43:32 | Re: pg18: Virtual generated columns are not (yet) safe when superuser selects from them |