Обсуждение: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

Поиск
Список
Период
Сортировка

Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Tom Lane
Дата:
Currently, configure tries "gnu_printf" then "__syslog__" then
"printf" to set PG_PRINTF_ATTRIBUTE.  Of late, buildfarm member
fritillary has been issuing warnings like

../../../../src/include/c.h:234:83: warning: 'syslog' is an unrecognized format function type [-Wformat=]
  234 | #define pg_attribute_printf(f,a) __attribute__((format(PG_PRINTF_ATTRIBUTE, f, a)))
      |                                                                                   ^
../../../../src/include/port.h:233:86: note: in expansion of macro 'pg_attribute_printf'
  233 | extern int      pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args) pg_attribute_printf(3, 0);
      |                                                                                      ^~~~~~~~~~~~~~~~~~~

I hadn't gotten around to looking at that closely, but in the past
week three new animals (borer, skeletonizer, whitefly) have started
doing that too.  On inspection, I realize that that's not happening
across our entire tree, but only within src/backend/jit/llvm.

There are at least two things that seem wrong here:

1. The affected platforms are CentOS Stream 9, CentOS Stream 10, and
AlmaLinux 10.0 (so it's not exactly old obsolete stuff).  Since those
are all Linux variants, what we *should* be choosing is "gnu_printf",
yet the configure log shows we chose "__syslog__".  Why?

2. Something in the LLVM headers on those platforms has evidently
decided that it'd be a brilliant idea to #define "__syslog__" as
"syslog", and that's breaking this.  That seems jaw-droppingly
stupid; why'd they do that, and what can we do to work around it?

Presumably, this is breaking the compiler's ability to check format
strings, so it's not cool to just ignore it.

Adding to my confusion, I do not see these warnings on a local
RHEL9 machine, which ought to be pretty equivalent to CentOS 9.

Anybody want to look into this more closely?

            regards, tom lane



Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Tom Lane
Дата:
I wrote:
> Adding to my confusion, I do not see these warnings on a local
> RHEL9 machine, which ought to be pretty equivalent to CentOS 9.

Oh!  I see part of the story: I was testing that with gcc.
If I switch to clang (version 20.1.8 here), then I see that
we pick "__syslog__" and then these complaints appear.

So why isn't clang being bug-compatible with gcc on the
names of printf archetypes?  They're certainly pretty
slavish about that on most other topics.

            regards, tom lane



Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Tom Lane
Дата:
I wrote:
> Currently, configure tries "gnu_printf" then "__syslog__" then
> "printf" to set PG_PRINTF_ATTRIBUTE.  Of late, buildfarm member
> fritillary has been issuing warnings like

> ../../../../src/include/c.h:234:83: warning: 'syslog' is an unrecognized format function type [-Wformat=]

> I hadn't gotten around to looking at that closely, but in the past
> week three new animals (borer, skeletonizer, whitefly) have started
> doing that too.  On inspection, I realize that that's not happening
> across our entire tree, but only within src/backend/jit/llvm.

Okay, I've figured out what's going on here, and it's not really
a new problem.  The proximate cause is that these four animals are
set up to explicitly pick clang:

                   'config_env' => {
                                     'CC' => 'clang'
                                   },

but they are not forcing the choice of C++ compiler, and configure
just defaults that to "g++".

Although clang claims to support the same format attributes as
gcc (in fact, its documentation merely refers you to gcc's docs),
that is a flat-out lie.  It has never supported "gnu_printf".
It does support "__syslog__" ... which gcc doesn't, at least
on Linux, and never has.

Thus, what's happening is that configure chooses PG_PRINTF_ATTRIBUTE
based on what the CC compiler likes, and then when we build C++ code
with the CXX compiler, it complains.

So if we don't want to see these warnings, either we need to try
to use the C++ compiler matching the CC choice, or we need to make
PG_PRINTF_ATTRIBUTE language-sensitive.  I think it should work
to do two configure probes and then make c.h do something like

#ifdef __cplusplus
#define PG_PRINTF_ATTRIBUTE PG_CPP_PRINTF_ATTRIBUTE
#else
#define PG_PRINTF_ATTRIBUTE PG_C_PRINTF_ATTRIBUTE
#endif

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.

For the moment we could silence the buildfarm noise by pressing
Mark W. to fix the configurations on these BF animals, which
is surely a lot less work than finding an automatic solution.

Thoughts?

            regards, tom lane



Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Daniel Gustafsson
Дата:
> 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.
Maybe documenting that it's highly recommended to use matching C and C++
compilers is enough rather than spend cycles for every configure on a check
which may be susceptible to false negatives?

--
Daniel Gustafsson




Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Tom Lane
Дата:
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


Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

От
Jelte Fennema-Nio
Дата:
On Sat, 6 Dec 2025 at 21:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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've definitely run into this myself a bunch of times. I'm wondering
if we should do this for more of these compiler features, e.g.
HAVE_TYPEOF is strictly wrong for C++ builds afaict[1].

[1]: https://www.postgresql.org/message-id/flat/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com