Subject: FullTransactionId changes are causing portability issues

From: Tom Lane <[email-masked]>
Date: Wed, 22 May 2019 09:56:07 -0400

Our Solaris packager reports that 12beta1 is failing to build for him
on some Solaris variants:

> The link failure is:

> ---
> Undefined			first referenced
>  symbol  			    in file
> ReadNextFullTransactionId           pg_checksums.o
> ld: fatal: symbol referencing errors. No output written to pg_checksums
> ---

> Now, ReadNextFullTransactionId() is implemented in
> src/backend/access/transam/varsup.c but I cannot see varsop.o being
> included in any of the libraries pg_checksum is linked against
> (libpgcommon.a and libpgport.a).

> When I check the pg_checksum.o I find that it references
> ReadNextFullTransactionId on the platforms that fail but not where it
> doesn't. The failed platforms are all sparc variants plus 64-bit x86
> on Solaris 11.

> The compiler used in Sun Studio 12u1, very old and and I can try to
> upgrade and see if that helps.
> [ it didn't ]

I'm a bit mystified why we did not see this problem in the buildfarm,
especially since we have at least one critter (damselfly) running an
OpenSolaris variant.  Nonetheless, it sure looks like a "somebody
was sloppy about frontend/backend separation" problem.

Fix ideas anyone?  I think we need to not only solve the immediate
problem (which might just take an #ifndef FRONTEND somewhere) but
also close the testing gap so we don't get blindsided like this
again.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Wed, 22 May 2019 10:48:08 -0400

>> ld: fatal: symbol referencing errors. No output written to pg_checksums

On looking closer, the fix is simple and matches what we've done
elsewhere: transam.h needs to have "#ifndef FRONTEND" to protect
its static inline function from being compiled into frontend code.

So the disturbing thing here is that we no longer have any active
buildfarm members that can build HEAD but have the won't-elide-
unused-static-functions problem.  Clearly we'd better close that
gap somehow ... anyone have an idea about how to test it better?

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Wed, 22 May 2019 15:55:50 -0400

> gap somehow ... anyone have an idea about how to test it better?

Ah-hah --- some study of the gcc manual finds that modern versions
of gcc have

`-fkeep-inline-functions'
     In C, emit `static' functions that are declared `inline' into the
     object file, even if the function has been inlined into all of its
     callers.  This switch does not affect functions using the `extern
     inline' extension in GNU C89.  In C++, emit any and all inline
     functions into the object file.

This seems to do exactly what we need to test for this problem.
I've confirmed that with it turned on, a modern platform finds
the ReadNextFullTransactionId problem with yesterday's sources,
and that everything seems green as of HEAD.

So, we'd obviously not want to turn this on for normal builds,
but could we get a buildfarm animal or two to use this switch?

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Wed, 22 May 2019 13:03:12 -0700

Hi,

> > gap somehow ... anyone have an idea about how to test it better?

I'm somewhat inclined to just declare that people using such old
compilers ought to just use something newer. Having to work around
broken compilers that are so old that we don't even have a buildfarm
animal actually exposing that behaviour, seems like wasted effort. IMO
it'd make sense to just treat this as part of the requirements for a C99
compiler.

> but could we get a buildfarm animal or two to use this switch?

I could easily add that to one of mine, if we decide to go for that.

Greetings,

Andres Freund

---

From: Tom Lane <[email-masked]>
Date: Wed, 22 May 2019 16:13:02 -0400

> it'd make sense to just treat this as part of the requirements for a C99

TBH, I too supposed the requirement for this had gone away with the C99
move.  But according to the discussion today on -packagers, there are
still supported variants of Solaris that have compilers that speak C99
but don't have this behavior.  Per Bjorn's report:

> I tried Sun Studio 12u2 and then a more drastic upgrade to Developer
> Studio 12.5 but both had the same effect.

It doesn't sound like "use a newer compiler" is going to be a helpful
answer there.

(It is annoying that nobody is running such a platform in the buildfarm,
I agree.  But I don't have the resources to spin up a real-Solaris
buildfarm member.)

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Thu, 23 May 2019 10:11:34 -0700

Hi,

On 2019-05-22 16:13:02 -0400, Tom Lane wrote:
> Per Bjorn's report:

Well, GCC is available on solaris, and IIRC not that hard to install
(isn't it just a 'pkg install gcc' or such?). Don't think we need to
invest a lot of energy fixing a compiler / OS combo that effectively
isn't developed further.

Greetings,

Andres Freund

---

From: Tom Lane <[email-masked]>
Date: Thu, 23 May 2019 13:46:15 -0400

> invest a lot of energy fixing a compiler / OS combo that effectively

I'm not really excited about adopting a position that PG will only
build on GCC and clones thereof.

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Thu, 23 May 2019 10:52:31 -0700

Hi,

> build on GCC and clones thereof.

That's not what I said though? Not supporting one compiler, on an OS
that's effectively not being developed anymore, with a pretty
indefensible behaviour, requiring not insignificant work by everyone,
isn't the same as standardizing on gcc. I mean, we obviously are going
to continue at the absolute very least gcc, llvm/clang and msvc.

Greetings,

Andres Freund

---

From: Tom Lane <[email-masked]>
Date: Thu, 23 May 2019 14:05:19 -0400

> to continue at the absolute very least gcc, llvm/clang and msvc.

I think you're vastly overstating the case for refusing support for this.
Adding "#ifndef FRONTEND" to relevant headers isn't a huge amount of work
--- it's certainly far less of a problem than the Microsoft-droppings
we've had to put in in so many places.  The only real issue in my mind
is the lack of buildfarm support for detecting that we need to do so.

Also relevant here is that you have no evidence for the assumption that
these old Solaris compilers are the only live platform with the problem.
Yeah, we wish our buildfarm covered everything of interest, but it does
not.  Maybe, if we get to beta2 without any additional reports of build
failures on beta1, that would be a bit of evidence that nobody else cares
--- but we have no such evidence right now.  We certainly can't assume
that any pre-v12 release provides evidence of that, because up till
I retired pademelon, it was forcing us to keep this case supported.

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Thu, 23 May 2019 11:49:08 -0700

Hi,

> is the lack of buildfarm support for detecting that we need to do so.

Well, doing it for every single inline function is pretty annoying, just
from a bulkiness perspective. And figuring out exactly which inline
function needs this isn't easy without something that actually shows the
problem.

> I retired pademelon, it was forcing us to keep this case supported.

I don't think I'm advocating for not fixing the issue we had for
solaris, for 12. I just don't think this a reasonable approach going
forward.

Greetings,

Andres Freund

---

From: Tom Lane <[email-masked]>
Date: Thu, 23 May 2019 18:27:53 -0400

> from a bulkiness perspective.

Oh, I certainly wasn't suggesting we do that.

> And figuring out exactly which inline

... which is why we need a buildfarm animal that shows the problem.
We had some, up until the C99 move.

If the only practical way to detect the issue were to run some ancient
platform or other, I'd tend to agree with you that it's not worth the
trouble.  But if we can spot it just by using -fkeep-inline-functions
on an animal or two, I think it's a reasonable thing to keep supporting
the case for a few years more.

			regards, tom lane