On 10/20/19 1:23 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.duns...@2ndquadrant.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
