From 0909380e4c9aa22b75dfe5a7e33393b48078db81 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 10 Dec 2025 17:09:10 -0500 Subject: [PATCH] Allow PG_PRINTF_ATTRIBUTE to be different in C and C++ code. Although clang claims to be compatible with gcc's printf format archetypes, this appears to be a falsehood: it likes __syslog__ (which gcc does not, on most platforms) and doesn't accept gnu_printf. This means that if you try to use gcc with clang++ or clang with g++, you get compiler warnings when compiling printf-like calls in our C++ code. This has been true for quite awhile, but it's gotten more annoying with the recent appearance of several buildfarm members that are configured like this. To fix, run separate probes for the format archetype to use with the C and C++ compilers, and conditionally define PG_PRINTF_ATTRIBUTE depending on __cplusplus. (We could alternatively insist that you not mix-and-match C and C++ compilers; but if the case works otherwise, this is a poor reason to insist on that.) No back-patch for now, but we may want to do that if this patch survives buildfarm testing. Discussion: https://postgr.es/m/986485.1764825548@sss.pgh.pa.us --- config/c-compiler.m4 | 46 ++++++++++++++++-- configure | 96 ++++++++++++++++++++++++++++++++++++-- configure.ac | 1 + meson.build | 17 ++++++- src/include/c.h | 10 ++++ src/include/pg_config.h.in | 11 +++-- 6 files changed, 169 insertions(+), 12 deletions(-) diff --git a/config/c-compiler.m4 b/config/c-compiler.m4 index 236a59e8536..e309a5e3e40 100644 --- a/config/c-compiler.m4 +++ b/config/c-compiler.m4 @@ -7,10 +7,10 @@ # Select the format archetype to be used by gcc to check printf-type functions. # We prefer "gnu_printf", as that most closely matches the features supported # by src/port/snprintf.c (particularly the %m conversion spec). However, -# on some NetBSD versions, that doesn't work while "__syslog__" does. -# If all else fails, use "printf". +# on clang and on some NetBSD versions, that doesn't work while "__syslog__" +# does. If all else fails, use "printf". AC_DEFUN([PGAC_PRINTF_ARCHETYPE], -[AC_CACHE_CHECK([for printf format archetype], pgac_cv_printf_archetype, +[AC_CACHE_CHECK([for C printf format archetype], pgac_cv_printf_archetype, [pgac_cv_printf_archetype=gnu_printf PGAC_TEST_PRINTF_ARCHETYPE if [[ "$ac_archetype_ok" = no ]]; then @@ -20,8 +20,8 @@ if [[ "$ac_archetype_ok" = no ]]; then pgac_cv_printf_archetype=printf fi fi]) -AC_DEFINE_UNQUOTED([PG_PRINTF_ATTRIBUTE], [$pgac_cv_printf_archetype], -[Define to best printf format archetype, usually gnu_printf if available.]) +AC_DEFINE_UNQUOTED([PG_C_PRINTF_ATTRIBUTE], [$pgac_cv_printf_archetype], +[Define to best C printf format archetype, usually gnu_printf if available.]) ])# PGAC_PRINTF_ARCHETYPE # Subroutine: test $pgac_cv_printf_archetype, set $ac_archetype_ok to yes or no @@ -38,6 +38,42 @@ ac_c_werror_flag=$ac_save_c_werror_flag ])# PGAC_TEST_PRINTF_ARCHETYPE +# PGAC_CXX_PRINTF_ARCHETYPE +# ------------------------- +# Because we support using gcc as C compiler with clang as C++ compiler, +# we have to be prepared to use different printf archetypes in C++ code. +# So, do the above test all over in C++. +AC_DEFUN([PGAC_CXX_PRINTF_ARCHETYPE], +[AC_CACHE_CHECK([for C++ printf format archetype], pgac_cv_cxx_printf_archetype, +[pgac_cv_cxx_printf_archetype=gnu_printf +PGAC_TEST_CXX_PRINTF_ARCHETYPE +if [[ "$ac_archetype_ok" = no ]]; then + pgac_cv_cxx_printf_archetype=__syslog__ + PGAC_TEST_CXX_PRINTF_ARCHETYPE + if [[ "$ac_archetype_ok" = no ]]; then + pgac_cv_cxx_printf_archetype=printf + fi +fi]) +AC_DEFINE_UNQUOTED([PG_CXX_PRINTF_ATTRIBUTE], [$pgac_cv_cxx_printf_archetype], +[Define to best C++ printf format archetype, usually gnu_printf if available.]) +])# PGAC_CXX_PRINTF_ARCHETYPE + +# Subroutine: test $pgac_cv_cxx_printf_archetype, set $ac_archetype_ok to yes or no +AC_DEFUN([PGAC_TEST_CXX_PRINTF_ARCHETYPE], +[ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +AC_LANG_PUSH(C++) +AC_COMPILE_IFELSE([AC_LANG_PROGRAM( +[extern void pgac_write(int ignore, const char *fmt,...) +__attribute__((format($pgac_cv_cxx_printf_archetype, 2, 3)));], +[pgac_write(0, "error %s: %m", "foo");])], + [ac_archetype_ok=yes], + [ac_archetype_ok=no]) +AC_LANG_POP([]) +ac_cxx_werror_flag=$ac_save_cxx_werror_flag +])# PGAC_TEST_CXX_PRINTF_ARCHETYPE + + # PGAC_TYPE_128BIT_INT # -------------------- # Check if __int128 is a working 128 bit integer type, and if so diff --git a/configure b/configure index 3f91117f245..5ee3c265b1c 100755 --- a/configure +++ b/configure @@ -14691,8 +14691,8 @@ _ACEOF ;; esac -{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for printf format archetype" >&5 -$as_echo_n "checking for printf format archetype... " >&6; } +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for C printf format archetype" >&5 +$as_echo_n "checking for C printf format archetype... " >&6; } if ${pgac_cv_printf_archetype+:} false; then : $as_echo_n "(cached) " >&6 else @@ -14752,7 +14752,97 @@ fi $as_echo "$pgac_cv_printf_archetype" >&6; } cat >>confdefs.h <<_ACEOF -#define PG_PRINTF_ATTRIBUTE $pgac_cv_printf_archetype +#define PG_C_PRINTF_ATTRIBUTE $pgac_cv_printf_archetype +_ACEOF + + + +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for C++ printf format archetype" >&5 +$as_echo_n "checking for C++ printf format archetype... " >&6; } +if ${pgac_cv_cxx_printf_archetype+:} false; then : + $as_echo_n "(cached) " >&6 +else + pgac_cv_cxx_printf_archetype=gnu_printf +ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +extern void pgac_write(int ignore, const char *fmt,...) +__attribute__((format($pgac_cv_cxx_printf_archetype, 2, 3))); +int +main () +{ +pgac_write(0, "error %s: %m", "foo"); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + ac_archetype_ok=yes +else + ac_archetype_ok=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag + +if [ "$ac_archetype_ok" = no ]; then + pgac_cv_cxx_printf_archetype=__syslog__ + ac_save_cxx_werror_flag=$ac_cxx_werror_flag +ac_cxx_werror_flag=yes +ac_ext=cpp +ac_cpp='$CXXCPP $CPPFLAGS' +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu + +cat confdefs.h - <<_ACEOF >conftest.$ac_ext +/* end confdefs.h. */ +extern void pgac_write(int ignore, const char *fmt,...) +__attribute__((format($pgac_cv_cxx_printf_archetype, 2, 3))); +int +main () +{ +pgac_write(0, "error %s: %m", "foo"); + ; + return 0; +} +_ACEOF +if ac_fn_cxx_try_compile "$LINENO"; then : + ac_archetype_ok=yes +else + ac_archetype_ok=no +fi +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext +ac_ext=c +ac_cpp='$CPP $CPPFLAGS' +ac_compile='$CC -c $CFLAGS $CPPFLAGS conftest.$ac_ext >&5' +ac_link='$CC -o conftest$ac_exeext $CFLAGS $CPPFLAGS $LDFLAGS conftest.$ac_ext $LIBS >&5' +ac_compiler_gnu=$ac_cv_c_compiler_gnu + +ac_cxx_werror_flag=$ac_save_cxx_werror_flag + + if [ "$ac_archetype_ok" = no ]; then + pgac_cv_cxx_printf_archetype=printf + fi +fi +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $pgac_cv_cxx_printf_archetype" >&5 +$as_echo "$pgac_cv_cxx_printf_archetype" >&6; } + +cat >>confdefs.h <<_ACEOF +#define PG_CXX_PRINTF_ATTRIBUTE $pgac_cv_cxx_printf_archetype _ACEOF diff --git a/configure.ac b/configure.ac index 8f390027403..4ba3967db0f 100644 --- a/configure.ac +++ b/configure.ac @@ -1676,6 +1676,7 @@ m4_defun([AC_PROG_CC_STDC], []) dnl We don't want that. AC_C_BIGENDIAN AC_C_INLINE PGAC_PRINTF_ARCHETYPE +PGAC_CXX_PRINTF_ARCHETYPE PGAC_C_STATIC_ASSERT PGAC_C_TYPEOF PGAC_C_TYPES_COMPATIBLE diff --git a/meson.build b/meson.build index 718150e3ac0..1256094fa57 100644 --- a/meson.build +++ b/meson.build @@ -1897,6 +1897,8 @@ if cc.compiles(''' endif +# Select the format archetype to be used to check printf-type functions. +# # Need to check a call with %m because netbsd supports gnu_printf but emits a # warning for each use of %m. printf_attributes = ['gnu_printf', '__syslog__', 'printf'] @@ -1911,11 +1913,24 @@ attrib_error_args = cc.get_supported_arguments('-Werror=format', '-Werror=ignore foreach a : printf_attributes if cc.compiles(testsrc.format(a), args: test_c_args + attrib_error_args, name: 'format ' + a) - cdata.set('PG_PRINTF_ATTRIBUTE', a) + cdata.set('PG_C_PRINTF_ATTRIBUTE', a) break endif endforeach +# We need to repeat the test for C++ because gcc and clang prefer different +# format archetypes. +if llvm.found() + attrib_error_args = cpp.get_supported_arguments('-Werror=format', '-Werror=ignored-attributes') + foreach a : printf_attributes + if cpp.compiles(testsrc.format(a), + args: attrib_error_args, name: 'cppformat ' + a) + cdata.set('PG_CXX_PRINTF_ATTRIBUTE', a) + break + endif + endforeach +endif + if cc.has_function_attribute('visibility:default') and \ cc.has_function_attribute('visibility:hidden') diff --git a/src/include/c.h b/src/include/c.h index 62cbf7a2eec..55effdda0fd 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -228,6 +228,16 @@ #define PG_USED_FOR_ASSERTS_ONLY pg_attribute_unused() #endif +/* + * Our C and C++ compilers may have different ideas about which printf + * archetype best represents what src/port/snprintf.c can do. + */ +#ifndef __cplusplus +#define PG_PRINTF_ATTRIBUTE PG_C_PRINTF_ATTRIBUTE +#else +#define PG_PRINTF_ATTRIBUTE PG_CXX_PRINTF_ATTRIBUTE +#endif + /* GCC supports format attributes */ #if defined(__GNUC__) #define pg_attribute_format_arg(a) __attribute__((format_arg(a))) diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 72434ce957e..3a7edb1f0a0 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -596,6 +596,14 @@ /* Define to the version of this package. */ #undef PACKAGE_VERSION +/* Define to best C++ printf format archetype, usually gnu_printf if + available. */ +#undef PG_CXX_PRINTF_ATTRIBUTE + +/* Define to best C printf format archetype, usually gnu_printf if available. + */ +#undef PG_C_PRINTF_ATTRIBUTE + /* Define to the name of a signed 128-bit integer type. */ #undef PG_INT128_TYPE @@ -612,9 +620,6 @@ /* PostgreSQL minor version number */ #undef PG_MINORVERSION_NUM -/* Define to best printf format archetype, usually gnu_printf if available. */ -#undef PG_PRINTF_ATTRIBUTE - /* PostgreSQL version as a string */ #undef PG_VERSION -- 2.39.5