public inbox for [email protected]  

help / color / mirror / Atom feed

From: David Rowley <[email protected]>
To: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected]
Subject: Re: pgsql: pg_logicalinspect: Fix possible crash when passing a directory p
Date: Thu, 17 Jul 2025 12:52:07 +1200
Message-ID: <CAApHDvoOnd4PBNV0qyJVLmbWvWUjztzcMH5xY2AGp5Vov6XU3Q@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAApHDvqrhFfnetbcwgGkJ=z63T8HfQ_OyP=vX8BYiXyxFKt67w@mail.gmail.com>
	<[email protected]>
	<CAApHDvqf4tKwE0oP8ybL2Eu-SD=C34pdZ9vVDSJwvqDXi88nUg@mail.gmail.com>
	<[email protected]>
	<CAApHDvrFdXjbrV6KCx_GHKYSufUbNDYSsjppcJQiGOURfJE6qg@mail.gmail.com>
	<[email protected]>

On Thu, 17 Jul 2025 at 02:56, Tom Lane <[email protected]> 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...

#include <stdio.h>

#define __builtin_constant_p(x) _Generic((1 ? ((void*)((x)*(uintptr_t)0)) : &(int){1}), int*: 1, void*: 0)

#define test(e) printf("%d\n", __builtin_constant_p(e));

#define ERROR 1

int main(void)
{
	int v = 1;
	test(v);
	test(ERROR);
	test(1);
	return 0;
}


Attachments:

  [text/plain] isconst.c (279B, 2-isconst.c)
  download | inline:
#include <stdio.h>

#define __builtin_constant_p(x) _Generic((1 ? ((void*)((x)*(uintptr_t)0)) : &(int){1}), int*: 1, void*: 0)

#define test(e) printf("%d\n", __builtin_constant_p(e));

#define ERROR 1

int main(void)
{
	int v = 1;
	test(v);
	test(ERROR);
	test(1);
	return 0;
}

view thread (16+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: pgsql: pg_logicalinspect: Fix possible crash when passing a directory p
  In-Reply-To: <CAApHDvoOnd4PBNV0qyJVLmbWvWUjztzcMH5xY2AGp5Vov6XU3Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox