MIME-Version: 1.0
References: <E1ts2ug-001xVQ-04@gemulon.postgresql.org>
 <CAApHDvqrhFfnetbcwgGkJ=z63T8HfQ_OyP=vX8BYiXyxFKt67w@mail.gmail.com>
 <6c00a8b2-0c40-44f0-b603-f6ae28b7694a@eisentraut.org>
 <CAApHDvqf4tKwE0oP8ybL2Eu-SD=C34pdZ9vVDSJwvqDXi88nUg@mail.gmail.com>
 <dc9665a8-987a-443d-ab18-f9865d21d034@eisentraut.org>
 <CAApHDvrFdXjbrV6KCx_GHKYSufUbNDYSsjppcJQiGOURfJE6qg@mail.gmail.com>
 <1244428.1752677780@sss.pgh.pa.us>
In-Reply-To: <1244428.1752677780@sss.pgh.pa.us>
From: David Rowley <dgrowleyml@gmail.com>
Date: Thu, 17 Jul 2025 12:52:07 +1200
Message-ID: 
 <CAApHDvoOnd4PBNV0qyJVLmbWvWUjztzcMH5xY2AGp5Vov6XU3Q@mail.gmail.com>
Subject: Re: pgsql: pg_logicalinspect: Fix possible crash when passing a
 directory p
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Peter Eisentraut <peter@eisentraut.org>,
 Masahiko Sawada <msawada@postgresql.org>,
	pgsql-committers@lists.postgresql.org
Content-Type: multipart/mixed; boundary="000000000000f20257063a156bd4"
Archived-At: 
 <https://www.postgresql.org/message-id/CAApHDvoOnd4PBNV0qyJVLmbWvWUjztzcMH5xY2AGp5Vov6XU3Q%40mail.gmail.com>
Precedence: bulk

--000000000000f20257063a156bd4
Content-Type: text/plain; charset="UTF-8"

On Thu, 17 Jul 2025 at 02:56, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Looking again at the code for ereport_domain(), I wondered if
> something like this would help MSVC see through it:
>
>  #define ereport_domain(elevel, domain, ...)    \
>     do { \
>         const int elevel_ = (elevel); \
> +       const bool is_error_ = (elevel_ >= ERROR); \
>         pg_prevent_errno_in_scope(); \
>         if (errstart(elevel_, domain)) \
>             __VA_ARGS__, errfinish(__FILE__, __LINE__, __func__); \
> -       if (elevel_ >= ERROR) \
> +       if (is_error_) \
>             pg_unreachable(); \
>     } while(0)
>
> This preserves single evaluation of the elevel parameter, and
> perhaps it'd move the needle on whether the compiler thinks
> is_error_ is a compile-time constant.  I'm just guessing
> though, don't have this compiler to test with.

I tried this and it unfortunately doesn't fix the issue. I expect that
the compiler is losing the ability to detect const-ness through the
"const" variables, and since is_error_ is being set from elevel_ it's
not seen as compile-time detectability constant either.

I spent a bit more time searching for a solution and did find
something that works well enough for this case in [1]. Unfortunately,
it only works with C11. See attached .c file and output below.

C11 test:

> cl /std:c11 isconst.c && isconst.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.44.35211 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

isconst.c
Microsoft (R) Incremental Linker Version 14.44.35211.0
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:isconst.exe
isconst.obj
0
1
1

C99 test:

> cl isconst.c && isconst.exe
Microsoft (R) C/C++ Optimizing Compiler Version 19.44.35211 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

isconst.c
isconst.c(12): error C2059: syntax error: 'type'
isconst.c(13): error C2059: syntax error: 'type'
isconst.c(14): error C2059: syntax error: 'type'

I didn't manage to find anything that works in C99.

David

[1] https://www.reddit.com/r/C_Programming/comments/o3ekqe/i_think_i_found_a_c11_compliant_way_to_detect/

--000000000000f20257063a156bd4
Content-Type: text/plain; charset="US-ASCII"; name="isconst.c"
Content-Disposition: attachment; filename="isconst.c"
Content-Transfer-Encoding: base64
Content-ID: <f_md6o9zb40>
X-Attachment-Id: f_md6o9zb40

I2luY2x1ZGUgPHN0ZGlvLmg+CgojZGVmaW5lIF9fYnVpbHRpbl9jb25zdGFudF9wKHgpIF9HZW5l
cmljKCgxID8gKCh2b2lkKikoKHgpKih1aW50cHRyX3QpMCkpIDogJihpbnQpezF9KSwgaW50Kjog
MSwgdm9pZCo6IDApCgojZGVmaW5lIHRlc3QoZSkgcHJpbnRmKCIlZFxuIiwgX19idWlsdGluX2Nv
bnN0YW50X3AoZSkpOwoKI2RlZmluZSBFUlJPUiAxCgppbnQgbWFpbih2b2lkKQp7CglpbnQgdiA9
IDE7Cgl0ZXN0KHYpOwoJdGVzdChFUlJPUik7Cgl0ZXN0KDEpOwoJcmV0dXJuIDA7Cn0K
--000000000000f20257063a156bd4--