From 335824ed7f63778e8cf954b86433e95d9d48b16e Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 17 Jul 2025 15:49:03 +1200 Subject: [PATCH v5] Detect elog(ERROR) can't return in MSVC when using C11 --- src/backend/backup/basebackup_target.c | 4 +-- src/include/c.h | 27 ++++++++++++++++++++ src/include/utils/elog.h | 35 +++++++++++++------------- 3 files changed, 47 insertions(+), 19 deletions(-) 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 f303ba0605a..0149e027f5c 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -331,6 +331,33 @@ #define pg_unreachable() abort() #endif +/* + * Define a compiler-independent macro for determining if an expression is a + * compile-time integer const. 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* an integer constant. Callers may check if this + * macro is defined by checking if HAVE_PG_BUILTIN_INTEGER_CONSTANT_P is + * defined. + */ +#if defined(HAVE__BUILTIN_CONSTANT_P) + +/* When __builtin_const_p() is available, use it. */ +#define pg_builtin_integer_constant_p(x) __builtin_constant_p(x) +#define HAVE_PG_BUILTIN_INTEGER_CONSTANT_P +#elif defined(_MSC_VER) && defined(__STDC_VERSION__) + +/* + * With MSVC we can use a trick with _Generic to make this work. This has + * been borrowed from: + * https://stackoverflow.com/questions/49480442/detecting-integer-constant-expressions-in-macros + * and only works with integer constants. Compilation will fail if given a + * constant or variable of any type other than an integer. + */ +#define pg_builtin_integer_constant_p(x) \ + _Generic((1 ? ((void *) ((x) * (uintptr_t) 0)) : &(int) {1}), int *: 1, void *: 0) +#define HAVE_PG_BUILTIN_INTEGER_CONSTANT_P +#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..b4945eb7ee0 100644 --- a/src/include/utils/elog.h +++ b/src/include/utils/elog.h @@ -119,36 +119,37 @@ 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 - * 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 - * 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. + * When pg_builtin_integer_constant_p 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 pg_builtin_integer_constant_p() 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. * * 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, - * 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. + * we're just adding code bloat. So, if pg_builtin_integer_constant_p 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_INTEGER_CONSTANT_P #define ereport_domain(elevel, domain, ...) \ do { \ pg_prevent_errno_in_scope(); \ - if (__builtin_constant_p(elevel) && (elevel) >= ERROR ? \ + if (pg_builtin_integer_constant_p(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_integer_constant_p(elevel) && (elevel) >= ERROR) \ pg_unreachable(); \ } while(0) -#else /* !HAVE__BUILTIN_CONSTANT_P */ +#else /* !HAVE_PG_BUILTIN_INTEGER_CONSTANT_P */ #define ereport_domain(elevel, domain, ...) \ do { \ const int elevel_ = (elevel); \ @@ -158,7 +159,7 @@ struct Node; if (elevel_ >= ERROR) \ pg_unreachable(); \ } while(0) -#endif /* HAVE__BUILTIN_CONSTANT_P */ +#endif /* HAVE_PG_BUILTIN_INTEGER_CONSTANT_P */ #define ereport(elevel, ...) \ ereport_domain(elevel, TEXTDOMAIN, __VA_ARGS__) -- 2.40.1.windows.1