On Fri, Jul 28, 2017 at 7:22 PM, Tom Lane <[email protected]> wrote:
> Ashutosh Sharma <[email protected]> writes:
>> Thanks for the patch. I am seeing some compilation errors on Windows
>> with the patch. Below are the errors observed,
>> ...
>> I did spent some time to find reason for these compilation errors and
>> could eventually find that some of the Windows specific functions
>> inside plperl module are calling Perl APIs without fetching the perl
>> interpreter's context using dTHX.
>
> Ah, thanks. I just stuck that in where compiler errors were telling
> me to, so I didn't realize there were Windows-specific functions to
> worry about.
>
>> Moreover, I have also tested this patch along with the patch to import
>> switches used by perl into plperl and together it fixes the server
>> crash issue. Also, now, the interpreter size in both perl and plperl
>> module are the same thereby generating the same key on both plperl and
>> perl module. Thanks.
>
> Excellent. So this looks like the avenue to pursue.
>
> As far as the question of which symbols to import goes: on my own
> machines I'm seeing a lot of things like
>
> $ perl -MConfig -e 'print $Config{ccflags}'
> -D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector
> -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
>
> $ perl -MConfig -e 'print $Config{ccflags}'
> -Ae -D_HPUX_SOURCE -Wl,+vnocompatwarnings -DDEBUGGING -I/usr/local/include
> -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
>
> I'm really quite afraid to import symbols like _LARGEFILE_SOURCE and
> _FILE_OFFSET_BITS into plperl; those *will* break things if they
> are different from what core Postgres is built with. Moreover,
> I came across a relevant data structure in perl.h:
>
> /* These are all the compile time options that affect binary compatibility.
> Other compile time options that are binary compatible are in perl.c
> Both are combined for the output of perl -V
> However, this string will be embedded in any shared perl library, which
> will
> allow us add a comparison check in perlmain.c in the near future. */
> #ifdef DOINIT
> EXTCONST char PL_bincompat_options[] =
> # ifdef DEBUG_LEAKING_SCALARS
> " DEBUG_LEAKING_SCALARS"
> # endif
> # ifdef DEBUG_LEAKING_SCALARS_FORK_DUMP
> " DEBUG_LEAKING_SCALARS_FORK_DUMP"
> # endif
> # ifdef FAKE_THREADS
> " FAKE_THREADS"
> # endif
> # ifdef MULTIPLICITY
> " MULTIPLICITY"
> # endif
> ... lots more ...
Thanks for sharing this information. I too had a look into
'PL_bincompat_options' data structure in perl.h and i didn't see any
macro name starting with underscore. Based on this information, we can
now confidently say that we do not need any -D switches starting with
underscore for binary compatibility purpose.
>
> Assuming that the Perl crew know what they're doing and this list is
> complete, I notice that not one of the symbols they show as relevant
> starts with an underscore. So I'm thinking that my previous semi-
> joking idea of absorbing only -D switches for names that *don't*
> start with an underscore was actually a good solution.
Yes, it was a good solution indeed.
If that
> turns out to not be enough of a filter, we could consider looking
> into perl.h to extract the set of symbols tested in building
> PL_bincompat_options and then intersecting that with what we get
> from Perl's ccflags. But that would be a lot more complex, so
> I'd rather go with the simpler filter rule for now.
Okay, as per your suggestion, I have modified my earlier patches that
imports the -D switches used by perl into plperl accordingly i.e. it
now ignores the switches whose name starts with underscore. Please
find plperl_win_v3 and plperl_linux_v2 patches attached with this
email.
>
> (BTW, you never did tell us exactly what defines you're getting
> out of Perl's flags on the problem installation.)
I am really sorry about that. I just missed that in my earlier email.
Here are the defines used in the perl where i could reproduce the
issue,
C:\Users\ashu>perl -MConfig -e "print $Config{ccflags}"
-nologo -GF -W3 -O1 -MD -Zi -DNDEBUG -GL -fp:precise -DWIN32
-D_CONSOLE -DNO_STRICT -DWIN64 -DCONSERVATIVE -DUSE_SITECUSTOMIZE
-DPERL_TEXTMODE_SCRIPTS -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index e0bf607..f0ada1b 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -518,6 +518,16 @@ sub mkvcbuild
$solution->AddProject('plperl', 'dll', 'PLs', 'src/pl/plperl');
$plperl->AddIncludeDir($solution->{options}->{perl} . '/lib/CORE');
$plperl->AddDefine('PLPERL_HAVE_UID_GID');
+
+ foreach my $f (split(" ",$Config{ccflags}))
+ {
+ if ($f =~ /^-D/ && $f !~ /^-D_/)
+ {
+ $f =~ s/\-D//;
+ $plperl->AddDefine($f);
+ }
+ }
+
foreach my $xs ('SPI.xs', 'Util.xs')
{
(my $xsc = $xs) =~ s/\.xs/.c/;
diff --git a/config/perl.m4 b/config/perl.m4
index bed2eae..d4ec25f 100644
--- a/config/perl.m4
+++ b/config/perl.m4
@@ -49,6 +49,16 @@ AC_DEFUN([PGAC_CHECK_PERL_CONFIGS],
[m4_foreach([pgac_item], [$1], [PGAC_CHECK_PERL_CONFIG(pgac_item)])])
+# PGAC_CHECK_PERL_EMBED_CCFLAGS
+# -----------------------------
+AC_DEFUN([PGAC_CHECK_PERL_EMBED_CCFLAGS],
+[AC_REQUIRE([PGAC_PATH_PERL])
+AC_MSG_CHECKING([for CFLAGS to compile embedded Perl])
+perl_ccflags=`$PERL -MConfig -e 'foreach $f (split(" ",$Config{ccflags})) {if ($f =~ /^-D/ && $f !~ /^-D_/) {print $f, " "}}'`
+AC_SUBST(perl_ccflags)dnl
+AC_MSG_RESULT([$perl_ccflags])])
+
+
# PGAC_CHECK_PERL_EMBED_LDFLAGS
# -----------------------------
# We are after Embed's ldopts, but without the subset mentioned in
diff --git a/configure b/configure
index aff72eb..ddff4ef 100755
--- a/configure
+++ b/configure
@@ -667,6 +667,7 @@ python_includespec
python_version
python_majorversion
PYTHON
+perl_ccflags
perl_embed_ldflags
perl_useshrplib
perl_privlibexp
@@ -7767,6 +7768,18 @@ documentation for details. Use --without-perl to disable building
PL/Perl." "$LINENO" 5
fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for Perl ccflags" >&5
+$as_echo_n "checking for Perl ccflags... " >&6; }
+perl_ccflags=`$PERL -MConfig -e 'foreach $f (split(" ",$Config{ccflags})) {if ($f =~ /^-D/ && $f !~ /^-D_/) {print $f, " "}}'`
+
+if test -z "$perl_ccflags" ; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $perl_ccflags" >&5
+$as_echo "$perl_ccflags" >&6; }
+fi
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for flags to link embedded Perl" >&5
$as_echo_n "checking for flags to link embedded Perl... " >&6; }
if test "$PORTNAME" = "win32" ; then
diff --git a/configure.in b/configure.in
index 72e5b17..13c69d6 100644
--- a/configure.in
+++ b/configure.in
@@ -938,6 +938,7 @@ You might have to rebuild your Perl installation. Refer to the
documentation for details. Use --without-perl to disable building
PL/Perl.])
fi
+ PGAC_CHECK_PERL_EMBED_CCFLAGS
PGAC_CHECK_PERL_EMBED_LDFLAGS
fi
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index dc8a89a..ff1c185 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -304,6 +304,7 @@ else
endif
perl_archlibexp = @perl_archlibexp@
perl_privlibexp = @perl_privlibexp@
+perl_ccflags = @perl_ccflags@
perl_embed_ldflags = @perl_embed_ldflags@
# Miscellaneous
diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile
index b8e3585..dce0513 100644
--- a/src/pl/plperl/GNUmakefile
+++ b/src/pl/plperl/GNUmakefile
@@ -12,7 +12,7 @@ override CPPFLAGS += -DPLPERL_HAVE_UID_GID
override CPPFLAGS += -Wno-comment
endif
-override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS) -I$(perl_archlibexp)/CORE
+override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS) -I$(perl_archlibexp)/CORE $(perl_ccflags)
rpathdir = $(perl_archlibexp)/CORE
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers