Re: pl/perl extension fails on Windows

On Wed, Jul 19, 2017 at 5:01 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I am really suspicious that this means your libperl was built in an unsafe
> fashion, that is, by injecting configuration choices as random -D switches
> in the build process rather than making sure the choices were recorded in
> perl's config.h. As an example, looking at the perl 5.24.1 headers on
> a Fedora box, it looks to me like PERL_SAWAMPERSAND could only get defined
> if PERL_COPY_ON_WRITE were not defined, and the only way that that can
> happen is if PERL_NO_COW is defined, and there are no references to the
> latter anyplace except in this particular #if defined test in perl.h.

Hmm, it might not be so random as all that. Have a look at this
commit log entry:

commit 1a904fc88069e249a4bd0ef196a3f1a7f549e0fe
Author: Father Chrysostomos <sprout(at)cpan(dot)org>
Date: Sun Nov 25 12:57:04 2012 -0800

Disable PL_sawampersand

PL_sawampersand actually causes bugs (e.g., perl #4289), because the
behaviour changes. eval '$&' after a match will produce different
results depending on whether $& was seen before the match.

Using copy-on-write for the pre-match copy (preceding patches do that)
alleviates the slowdown caused by mentioning $&. The copy doesn’t
happen unless the string is modified after the match. It’s now a
post- match copy. So we no longer need to do things differently
depending on whether $& has been seen.

PL_sawampersand is now #defined to be equal to what it would be if
every program began with $',$&,$`.

I left the PL_sawampersand code in place, in case this commit proves
immature. Running Configure with -Accflags=PERL_SAWAMPERSAND will
reënable the PL_sawampersand mechanism.

Based on a bit of experimentation, that last bit contains a typo: it
should say -Accflags=-DPERL_SAWAMPERSAND; as written, the -D is
missing.[1] Anyway, the point is that at least in this case, there
seems to have been some idea that somebody might want to reenable this
in their own build even after it was disabled by default.

Perl also has a mechanism for flags added to Configure to be passed
along when building loadable modules; if it didn't, not just plperl
but every Perl module written in C would have this issue if any such
flags where used. Normally, you compile perl modules by running "perl
Makefile.PL" to generate a makefile, and then building from the
makefile. If you do that, then the Makefile ends up with a section in
it that looks like this:

# --- MakeMaker cflags section:

CCFLAGS = -fno-strict-aliasing -pipe -fstack-protector-strong
-I/opt/local/include -DPERL_USE_SAFE_PUTENV -DPERL_SAWAMPERSAND -Wall
-Werror=declaration-after-statement -Wextra -Wc++-compat
-Wwrite-strings
OPTIMIZE = -O3
PERLTYPE =
MPOLLUTE =

...and lo-and-behold, the -DPERL_SAWAMPERSAND flag which I passed to
Configure is there. After a bit of time deciphering how MakeMaker
actually works, I figured out that it gets the value for CFLAGS by
doing "use Config;" and then referencing $Config::Config{'ccflags'};
an alternative way to get it, from the shell, is to run perl
-V:ccflags.

While I'm not sure of the details, I suspect that we need to use one
of those methods to get the CCFLAGS used to build perl, and include
those when SPI.o, Util.o, and plperl.o in src/pl/plperl. Or at least
the -D switches from those CCFLAGS. Here's about the simplest thing
that seems like it might work on Linux; Windows would need something
equivalent:

override CPPFLAGS += $(shell $(PERL) -MConfig -e 'print
$$Config::Config{"ccflags"};')

On my MacBook Pro, with the built-in switches, that produces:

-fno-common -DPERL_DARWIN -mmacosx-version-min=10.12 -pipe -Os
-fno-strict-aliasing -fstack-protector-strong -I/opt/local/include
-DPERL_USE_SAFE_PUTENV

Or we could try to extract just the -D switches:

override CPPFLAGS += $(shell $(PERL) -MConfig -e 'print join " ", grep
{ /^-D/ } split /\s+/, $$Config::Config{"ccflags"};')

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] Arguably, the umlaut over "reenable" is also a typo, but that's a
sort of in a different category.