Re: configure fails for perl check on CentOS8

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: configure fails for perl check on CentOS8
Date: 2019-10-20 17:23:10
Message-ID: 3875.1571592190@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
> On 10/18/19 9:50 AM, Tom Lane wrote:
>> Can we fix this by using something other than perl_alloc() as
>> the tested-for function? That is surely a pretty arbitrary
>> choice. Are there any standard Perl entry points that are just
>> plain functions with no weird macro expansions?

> I had a look in perl's proto.h but didn't see any obvious candidate. I
> tried a couple of others (e.g. Perl_get_context) and got the same result
> reported above.

I poked into this on a Fedora 30 installation and determined that the
stray reference is coming from this bit in Perl's inline.h:

/* saves machine code for a common noreturn idiom typically used in Newx*() */
GCC_DIAG_IGNORE_DECL(-Wunused-function);
static void
S_croak_memory_wrap(void)
{
Perl_croak_nocontext("%s",PL_memory_wrap);
}
GCC_DIAG_RESTORE_DECL;

Apparently, gcc is smart enough to optimize this away as unused ...
at any optimization level higher than -O0. I confirmed that it works
at -O0 too, if you change this function declaration to "static inline"
--- but evidently, not doing so was intentional, so we won't get much
cooperation if we propose changing it (back?) to a plain static inline.

So the failure occurs just from reading this header, independently of
which particular Perl function we try to call; I'd supposed that there
was some connection between perl_alloc and PL_memory_wrap, but there
isn't.

I still don't much like the originally proposed patch. IMO it makes too
many assumptions about what is in Perl's ccflags, and perhaps more to the
point, it is testing libperl's linkability using switches we will not use
when we actually build plperl. So I think there's a pretty high risk of
breaking other cases if we fix it that way.

The right way to fix it, likely, is to add CFLAGS_SL while performing this
particular autoconf test, as that would replicate the switches used for
plperl (and it turns out that adding -fPIC does solve this problem).
But the configure script doesn't currently know about CFLAGS_SL, so we'd
have to do some refactoring to approach it that way. Moving that logic
from the platform-specific Makefiles into configure doesn't seem
unreasonable, but it would make the patch bigger.

A less invasive idea is to forcibly add -O1 to CFLAGS for this autoconf
test. We'd have to be careful about doing so for non-gcc compilers, as
they might not understand that switch syntax ... but probably we could
get away with changing CFLAGS only when using a gcc-alike. Still, that's
a hack, and it doesn't have much to recommend it other than being more
localized.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message vignesh C 2019-10-20 17:28:03 Re: Ordering of header file inclusion
Previous Message David G. Johnston 2019-10-20 17:14:50 Re: jsonb_set() strictness considered harmful to data