Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE - Mailing list pgsql-hackers

Daniel Gustafsson <daniel@yesql.se> writes:
> On 4 Dec 2025, at 19:01, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> On the other hand, aligning the C++ compiler with the C compiler
>> is likely to avoid other problems, so maybe it's better to focus
>> on making that happen.  I'm not sure how we'd do that automatically
>> though.

> It sounds pretty complicated to, with confidence, automatically detect this.

Yeah.  However, selecting different PG_PRINTF_ATTRIBUTE values for
C and C++ mode seems to get the job done.  I've confirmed the attached
silences these warnings for me when mixing-and-matching gcc and clang.

I don't have a lot of faith in the meson.build addition being correct,
though it worked in a simple test.  Anyone want to review that?

            regards, tom lane

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 3a0ed11fa8e..0c86d481ac8 100755
--- a/configure
+++ b/configure
@@ -14649,8 +14649,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
@@ -14710,7 +14710,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 c2413720a18..e79ea31f618 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1674,6 +1674,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 6e7ddd74683..38592faba49 100644
--- a/meson.build
+++ b/meson.build
@@ -1894,6 +1894,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']
@@ -1908,11 +1910,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: test_c_args + attrib_error_args, name: 'format ' + 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 ccd2b654d45..10060f02588 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 b0b0cfdaf79..076dab5f6d3 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