Re: configure fails for perl check on CentOS8

From: Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
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 19:31:06
Message-ID: 892bf456-a7c5-93f4-0e14-3eb4601bfd22@2ndQuadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On 10/20/19 1:23 PM, Tom Lane wrote:
> 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.

Yeah, I came to the same conclusion.

>
> 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.

Agreed.

>
> 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.

Sounds like a plan. I agree it's annoying to have to do something large
for something so trivial.

>
> 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.
>
>

right. I think your other plan is better.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2019-10-20 19:48:05 Re: jsonb_set() strictness considered harmful to data
Previous Message Tom Lane 2019-10-20 18:47:57 Re: documentation inconsistent re: alignment