Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

Started by Tom Lane4 months ago6 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

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

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)
Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#1)
Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

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

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Tom Lane (#3)
Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

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

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Daniel Gustafsson (#4)
Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

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

Attachments:

v1-separate-c-and-cxx-printf-archetypes.patchtext/x-diff; charset=us-ascii; name=v1-separate-c-and-cxx-printf-archetypes.patchDownload+169-12
#6Jelte Fennema-Nio
postgres@jeltef.nl
In reply to: Tom Lane (#5)
Re: Something in our JIT code is screwing up PG_PRINTF_ATTRIBUTE

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]/messages/by-id/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com.

[1]: /messages/by-id/CAGECzQR21OnnKiZO_1rLWO0-16kg1JBxnVq-wymYW0-_1cUNtg@mail.gmail.com