From 23fc6298e9176b38f6d86390b8af4784b889d3a6 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 17 Jul 2025 15:49:03 +1200 Subject: [PATCH v3] Detect elog(ERROR) can't return in MSVC when using C11 --- meson.build | 28 +++++++++++++++++++++++++- src/backend/backup/basebackup_target.c | 4 ++-- src/include/c.h | 17 ++++++++++++++++ src/include/utils/elog.h | 16 +++++++-------- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/meson.build b/meson.build index 5365aaf95e6..13ad0faf91c 100644 --- a/meson.build +++ b/meson.build @@ -1704,6 +1704,18 @@ endif # Compiler tests ############################################################### +# Test to see if the compiler supports C11 +c11_test = ''' +#if !defined(__STDC_VERSION__) || __STDC_VERSION__ < 201112L +#error "C11 not supported" +#endif + +int main(int argc, char **argv) +{ + return 0; +} +''' + # Do we need -std=c99 to compile C99 code? We don't want to add -std=c99 # unnecessarily, because we optionally rely on newer features. c99_test = ''' @@ -1737,7 +1749,21 @@ int main(int argc, char **argv) } ''' -if not cc.compiles(c99_test, name: 'c99', args: test_c_args) +# The current minimum version of MSVC we currently support supports at least +# C11, so lets try to compile with C11. Check if we need to actually pass the +# /std:c11 flag for that to work. If this test ends up adding the C11 flag, +# then the C99 test below should always pass and never add the C99 flag. +if cc.get_id() == 'msvc' and not cc.compiles(c11_test, name: 'c11', args: test_c_args) + if cc.compiles(c11_test, name: 'c11 with /std:c11', + args: test_c_args + ['/std:c11']) + test_c_args += '/std:c11' + cflags += '/std:c11' + else + error('C compiler does not support C11') + endif +endif + +if cc.get_id() != 'msvc' and not cc.compiles(c99_test, name: 'c99', args: test_c_args) if cc.compiles(c99_test, name: 'c99 with -std=c99', args: test_c_args + ['-std=c99']) test_c_args += '-std=c99' diff --git a/src/backend/backup/basebackup_target.c b/src/backend/backup/basebackup_target.c index 84b1309d3bd..01705368950 100644 --- a/src/backend/backup/basebackup_target.c +++ b/src/backend/backup/basebackup_target.c @@ -145,8 +145,8 @@ BaseBackupGetTargetHandle(char *target, char *target_detail) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("unrecognized target: \"%s\"", target))); - /* keep compiler quiet */ - return NULL; + /* comment out for testing. Not for commit. */ + /* return NULL; */ } /* diff --git a/src/include/c.h b/src/include/c.h index 6d4495bdd9f..6603d22c4df 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -332,6 +332,23 @@ #define pg_unreachable() abort() #endif +/* + * Define a compiler-independent macro for determining if an expression is a + * compile-time const. With MSVC using C11 and above, we can use a trick with + * _Generic to make this work. We don't define this macro to return 0 when + * unsupported due to the risk of users of the macro misbehaving if we return + * 0 when the expression *is* a built-in constant. Callers may check if this + * macro is defined by checking if HAVE_PG_BUILTIN_CONSTANT is defined. + */ +#if defined(HAVE__BUILTIN_CONSTANT_P) +#define pg_builtin_constant(x) __builtin_constant_p(x) +#define HAVE_PG_BUILTIN_CONSTANT +#elif defined(_MSC_VER) && defined(__STDC_VERSION__) && __STDC_VERSION__ >= 201112L +#define pg_builtin_constant(x) \ + _Generic((1 ? ((void *) ((x) * (uintptr_t) 0)) : &(int) {1}), int *: 1, void *: 0) +#define HAVE_PG_BUILTIN_CONSTANT +#endif + /* * pg_assume(expr) states that we assume `expr` to evaluate to true. In assert * enabled builds pg_assume() is turned into an assertion, in optimized builds diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h index 675f4f5f469..16f2a4f39b1 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -119,11 +119,11 @@ struct Node; * ereport_domain() directly, or preferably they can override the TEXTDOMAIN * macro. * - * When __builtin_constant_p is available and elevel >= ERROR we make a call + * When pg_builtin_constant is available and elevel >= ERROR we make a call * to errstart_cold() instead of errstart(). This version of the function is * marked with pg_attribute_cold which will coax supporting compilers into * generating code which is more optimized towards non-ERROR cases. Because - * we use __builtin_constant_p() in the condition, when elevel is not a + * we use pg_builtin_constant() in the condition, when elevel is not a * compile-time constant, or if it is, but it's < ERROR, the compiler has no * need to generate any code for this branch. It can simply call errstart() * unconditionally. @@ -131,24 +131,24 @@ struct Node; * If elevel >= ERROR, the call will not return; we try to inform the compiler * of that via pg_unreachable(). However, no useful optimization effect is * obtained unless the compiler sees elevel as a compile-time constant, else - * we're just adding code bloat. So, if __builtin_constant_p is available, + * we're just adding code bloat. So, if pg_builtin_constant is available, * use that to cause the second if() to vanish completely for non-constant * cases. We avoid using a local variable because it's not necessary and * prevents gcc from making the unreachability deduction at optlevel -O0. *---------- */ -#ifdef HAVE__BUILTIN_CONSTANT_P +#ifdef HAVE_PG_BUILTIN_CONSTANT #define ereport_domain(elevel, domain, ...) \ do { \ pg_prevent_errno_in_scope(); \ - if (__builtin_constant_p(elevel) && (elevel) >= ERROR ? \ + if (pg_builtin_constant(elevel) && (elevel) >= ERROR ? \ errstart_cold(elevel, domain) : \ errstart(elevel, domain)) \ __VA_ARGS__, errfinish(__FILE__, __LINE__, __func__); \ - if (__builtin_constant_p(elevel) && (elevel) >= ERROR) \ + if (pg_builtin_constant(elevel) && (elevel) >= ERROR) \ pg_unreachable(); \ } while(0) -#else /* !HAVE__BUILTIN_CONSTANT_P */ +#else /* !HAVE_PG_BUILTIN_CONSTANT */ #define ereport_domain(elevel, domain, ...) \ do { \ const int elevel_ = (elevel); \ @@ -158,7 +158,7 @@ struct Node; if (elevel_ >= ERROR) \ pg_unreachable(); \ } while(0) -#endif /* HAVE__BUILTIN_CONSTANT_P */ +#endif /* HAVE_PG_BUILTIN_CONSTANT */ #define ereport(elevel, ...) \ ereport_domain(elevel, TEXTDOMAIN, __VA_ARGS__) -- 2.40.1.windows.1