Обсуждение: [HACKERS] pl/perl extension fails on Windows

Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> writes:
> I compiled PG 10 beta1/beta2 with "--with-perl" option on Windows and the
> extension crashes the database.
> *src/pl/plperl/Util.c: loadable library and perl binaries are mismatched
> (got handshake key 0A900080, needed 0AC80080)*

> This is seen with Perl 5.24 but not with 5.20, 5.16. What I found is that
> the handshake function is added in Perl 5.21.x and probably that is why we
> don't see this issue in earlier versions.

Well, we have various buildfarm machines running perls newer than that,
eg, crake, with 5.24.1.  So I'd say there is something busted about your
perl installation.  Perhaps leftover bits of an older version somewhere?
        regards, tom lane

On 07/12/2017 11:49 AM, Dave Page wrote:
>
>
> On Wed, Jul 12, 2017 at 4:35 PM, Tom Lane <tgl@sss.pgh.pa.us
> <mailto:tgl@sss.pgh.pa.us>> wrote:
>
>     Sandeep Thakkar <sandeep.thakkar@enterprisedb.com
>     <mailto:sandeep.thakkar@enterprisedb.com>> writes:
>     > I compiled PG 10 beta1/beta2 with "--with-perl" option on
>     Windows and the
>     > extension crashes the database.
>     > *src/pl/plperl/Util.c: loadable library and perl binaries are
>     mismatched
>     > (got handshake key 0A900080, needed 0AC80080)*
>
>     > This is seen with Perl 5.24 but not with 5.20, 5.16. What I
>     found is that
>     > the handshake function is added in Perl 5.21.x and probably that
>     is why we
>     > don't see this issue in earlier versions.
>
>     Well, we have various buildfarm machines running perls newer than
>     that,
>     eg, crake, with 5.24.1.  So I'd say there is something busted
>     about your
>     perl installation.  Perhaps leftover bits of an older version
>     somewhere?
>
>
> Well crake is a Fedora box - and we have no problems on Linux, only on
> Windows. 
>
>


Yeah, I have this on one of my Windows boxes, and haven't had time to
get to the bottom of it yet ;-(

Latest versions of ActivePerl don't ship with library descriptor files,
either, which is unpleasant.

cheers

andrew

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

On Thu, Jul 13, 2017 at 12:01 AM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
> On 07/12/2017 11:49 AM, Dave Page wrote:
>>
>>
>> Well crake is a Fedora box - and we have no problems on Linux, only on
>> Windows.
>>
>>
>
>
> Yeah, I have this on one of my Windows boxes, and haven't had time to
> get to the bottom of it yet ;-(
>

I could also see this problem in my Windows machine and I have spent
some time to analyse it.  Here are my findings:

The stacktrace where the crash happens is:
 perl524.dll!Perl_xs_handshake(const unsigned long key, void
*v_my_perl, const char * file, ...)  Line 5569 C plperl.dll!boot_PostgreSQL__InServer__Util(interpreter * my_perl, cv
* cv)  Line 490 + 0x22 bytes C perl524.dll!Perl_pp_entersub(interpreter * my_perl)  Line 3987 + 0xc bytes C
perl524.dll!Perl_runops_standard(interpreter* my_perl)  Line 41 + 0x6 bytes C perl524.dll!S_run_body(interpreter *
my_perl,long oldscope)  Line 2485 C perl524.dll!perl_run(interpreter * my_perl)  Line 2406 + 0xc bytes C
plperl.dll!plperl_init_interp() Line 829 + 0xb bytes C plperl.dll!_PG_init()  Line 470 + 0x5 bytes C
 

I couldn't get much out of above bt, but, one thing i could notice is
that the input key passed to 'Perl_xs_handshake()' function is not
matching with the key being generated inside 'Perl_xs_handshake()',
hence, the handshaking is failing.

Please have a look into the following code snippet from 'perl 5.24'
and 'Util.c' file in plperl respectively,

Perl-5.24:
=======   got = INT2PTR(void*, (UV)(key & HSm_KEY_MATCH));   need = (void *)(HS_KEY(FALSE, FALSE, "", "") &
HSm_KEY_MATCH);  if (UNLIKELY(got != need))       goto bad_handshake;
 

Util.c in plperl:
==========
485 XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
486 {
487 #if PERL_VERSION_LE(5, 21, 5)
488    dVAR; dXSARGS;
489 #else
490    dVAR; dXSBOOTARGSAPIVERCHK;

Actually the macro 'dXSBOOTARGSAPIVERCHK' in line #490 gets expanded to,

#define XS_APIVERSION_SETXSUBFN_POPMARK_BOOTCHECK        \   Perl_xs_handshake(HS_KEY(TRUE, TRUE, "v"
PERL_API_VERSION_STRING,
""),      \       HS_CXT, __FILE__, "v" PERL_API_VERSION_STRING)

And the point to be noted is, the way, the key is being generated in
above macro and in Perl_xs_handshake(). As shown above,
'XS_APIVERSION_SETXSUBFN_POPMARK_BOOTCHECK' macro passes
'PERL_API_VERSION_STRING' as input to HS_KEY() to generate the key and
this key is passed to Perl_xs_handshake function whereas inside
Perl_xs_handshake(), there is no such version string being used to
generate the key. Hence, the key mismatch is seen thereby resulting in
a bad_handshake error.

After doing some study, I could understand that Util.c is generated
from Util.xs by xsubpp compiler at build time. This is being done in
Mkvcbuild.pm file in postgres. If I manually replace
'dXSBOOTARGSAPIVERCHK' macro with 'dXSBOOTARGSNOVERCHK' macro in
src/pl/plperl/Util.c, the things work fine. The diff is as follows,

XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
{
#if PERL_VERSION_LE(5, 21, 5)   dVAR; dXSARGS;
#else
-    dVAR; dXSBOOTARGSAPIVERCHK;
+    dVAR; dXSBOOTARGSNOVERCHK;

I need to further investigate, but let me know if you have any ideas.

>
> Latest versions of ActivePerl don't ship with library descriptor files,
> either, which is unpleasant.
>

I too had similar observation and surprisingly, I am seeing
'libperl*.a' file instead of 'perl*.lib' file in most of  the
ActiverPerl versions for Windows.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>
> After doing some study, I could understand that Util.c is generated
> from Util.xs by xsubpp compiler at build time. This is being done in
> Mkvcbuild.pm file in postgres. If I manually replace
> 'dXSBOOTARGSAPIVERCHK' macro with 'dXSBOOTARGSNOVERCHK' macro in
> src/pl/plperl/Util.c, the things work fine. The diff is as follows,
>
> XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
> {
> #if PERL_VERSION_LE(5, 21, 5)
>     dVAR; dXSARGS;
> #else
> -    dVAR; dXSBOOTARGSAPIVERCHK;
> +    dVAR; dXSBOOTARGSNOVERCHK;
>
> I need to further investigate, but let me know if you have any ideas.




Good job hunting this down!


One suggestion I saw in a little googling was that we add this to the XS
file after the inclusion of XSUB.h:
   #undef dXSBOOTARGSAPIVERCHK   #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK

cheers

andrew

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

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>> -    dVAR; dXSBOOTARGSAPIVERCHK;
>> +    dVAR; dXSBOOTARGSNOVERCHK;

> Good job hunting this down!
> One suggestion I saw in a little googling was that we add this to the XS
> file after the inclusion of XSUB.h:
>     #undef dXSBOOTARGSAPIVERCHK
>     #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK

I don't see anything even vaguely like that in the Util.c file generated
by Perl 5.10.1, which is what I've got on my RHEL machine.

What I do notice is this in Util.xs:

VERSIONCHECK: DISABLE

which leads immediately to two questions:

1. Why is your version of xsubpp apparently ignoring this directive
and generating a version check anyway?

2. Why do we have this directive in the first place?  It does not seem
to me like a terribly great idea to ignore low-level version mismatches.

In the same vein, I'm suspicious of proposals to "fix" this problem
by removing the version check, which seems to be where Ashutosh
is headed.  In the long run that seems certain to cause huge headaches.
        regards, tom lane

On 07/13/2017 10:36 AM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>>> -    dVAR; dXSBOOTARGSAPIVERCHK;
>>> +    dVAR; dXSBOOTARGSNOVERCHK;
>> Good job hunting this down!
>> One suggestion I saw in a little googling was that we add this to the XS
>> file after the inclusion of XSUB.h:
>>     #undef dXSBOOTARGSAPIVERCHK
>>     #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK
> I don't see anything even vaguely like that in the Util.c file generated
> by Perl 5.10.1, which is what I've got on my RHEL machine.




This is all fairly modern, so it's hardly surprising that it doesn't
happen with the ancient perl 5.10.

here's a snippet from the generated Util.c on crake (Fedora 25, perl 5.24):
   XS_EXTERNAL(boot_PostgreSQL__InServer__Util); /* prototype to pass   -Wmissing-prototypes */
XS_EXTERNAL(boot_PostgreSQL__InServer__Util)  {   #if PERL_VERSION_LE(5, 21, 5)       dVAR; dXSARGS;   #else
dVAR;dXSBOOTARGSAPIVERCHK;   #endif
 



>
> What I do notice is this in Util.xs:
>
> VERSIONCHECK: DISABLE
>
> which leads immediately to two questions:
>
> 1. Why is your version of xsubpp apparently ignoring this directive
> and generating a version check anyway?
>
> 2. Why do we have this directive in the first place?  It does not seem
> to me like a terribly great idea to ignore low-level version mismatches.
>
> In the same vein, I'm suspicious of proposals to "fix" this problem
> by removing the version check, which seems to be where Ashutosh
> is headed.  In the long run that seems certain to cause huge headaches.



That is a different version check. It's the equivalent of xsubpp's
--noversioncheck flag. The versions it would check are the object file
and the corresponding pm file.

In fact the usage I suggested seems to be blessed in XSUB.h in this comment:
   /* dXSBOOTARGSNOVERCHK has no API in xsubpp to choose it so do   #undef dXSBOOTARGSXSAPIVERCHK   #define
dXSBOOTARGSXSAPIVERCHKdXSBOOTARGSNOVERCHK */
 





It would be nice to get to the bottom of why we're getting a version
mismatch on Windows, since we're clearly not getting one on Linux. But
since we've got on happily all these years without the API version check
we might well survive a few more going on as we are.

cheers

andrew

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

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> It would be nice to get to the bottom of why we're getting a version
> mismatch on Windows, since we're clearly not getting one on Linux.

Yeah, that's what's bothering me: as long as that remains unexplained,
I don't have any confidence that we're fixing the right thing.
        regards, tom lane

Re: Dave Page 2017-07-12 <CA+OCxox=h5xftcnyk1AfA_QKSQSruD=Er0qweZakiCEg3U4rcg@mail.gmail.com>
> > Well, we have various buildfarm machines running perls newer than that,
> > eg, crake, with 5.24.1.  So I'd say there is something busted about your
> > perl installation.  Perhaps leftover bits of an older version somewhere?
> >
> 
> Well crake is a Fedora box - and we have no problems on Linux, only on
> Windows.

The plperl segfault on Debian's kfreebsd port I reported back in 2013
is also still present:

https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de

https://buildd.debian.org/status/fetch.php?pkg=postgresql-10&arch=kfreebsd-amd64&ver=10~beta2-1&stamp=1499947011&raw=0

(Arguably, this is a toy architecture, so we can just leave it unfixed
there without any harm...)

Christoph

On Thu, Jul 13, 2017 at 6:04 PM, Andrew Dunstan
<andrew.dunstan@2ndquadrant.com> wrote:
>
>
> On 07/13/2017 08:08 AM, Ashutosh Sharma wrote:
>>
>> After doing some study, I could understand that Util.c is generated
>> from Util.xs by xsubpp compiler at build time. This is being done in
>> Mkvcbuild.pm file in postgres. If I manually replace
>> 'dXSBOOTARGSAPIVERCHK' macro with 'dXSBOOTARGSNOVERCHK' macro in
>> src/pl/plperl/Util.c, the things work fine. The diff is as follows,
>>
>> XS_EXTERNAL(boot_PostgreSQL__InServer__Util)
>> {
>> #if PERL_VERSION_LE(5, 21, 5)
>>     dVAR; dXSARGS;
>> #else
>> -    dVAR; dXSBOOTARGSAPIVERCHK;
>> +    dVAR; dXSBOOTARGSNOVERCHK;
>>
>> I need to further investigate, but let me know if you have any ideas.
>
>
>
>
> Good job hunting this down!
>
>
> One suggestion I saw in a little googling was that we add this to the XS
> file after the inclusion of XSUB.h:
>
>     #undef dXSBOOTARGSAPIVERCHK
>     #define dXSBOOTARGSAPIVERCHK dXSBOOTARGSNOVERCHK
>

Thanks for that suggestion. It was really helpful. I think, the point
is, in XSUB.h, the macro 'dXSBOOTARGSXSAPIVERCHK' has been redefined
for novercheck but surprisingly it hasn't been done for
'dXSBOOTARGSAPIVERCHK' macro and that could be the reason why we want
to redefine it in the XS file after including XSUB.h file. Attached is
the patch that redefines 'dXSBOOTARGSAPIVERCHK' in Util.xs and SPI.xs
files. Please have a look into the attached patch and let me know your
comments. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> Actually the function used for generation of handshake Key i.e HS_KEYp()
> considers 'sizeof(PerlInterpreter)' to generate the key and somehow the
> sizeof PerlInterpreter is not uniform in plperl and perl modules incase of
> Windows but on Linux it remains same in both the modules.

Yipes.  So actually, this is catching a live ABI problem, which presumably
we've escaped seeing bad effects from only through sheer good luck.
I suppose that the discrepancies in the struct contents only occur after
the last field that plperl happens to touch directly --- but that is
unlikely to be true forever.

> *typedef struct interpreter PerlInterpreter;struct interpreter {#  include
> "intrpvar.h"};*
> where intrpvar.h has different variables defined inside it and most of the
> variables definition are protected with various macros. And there are some
> macros that are just defined in perl but not in plperl module which means
> the sizeof(PerlInterpreter) on the two modules are going to be different
> and thereby resulting in a  different key.

I imagine the route to a solution is to fix things so that the relevant
macros are all defined correctly in both cases.  But why they aren't
already is certainly an interesting question.  Have you identified just
which fields are added or missing relative to what libperl thinks?
        regards, tom lane

On Wed, Jul 19, 2017 at 12:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
>> Actually the function used for generation of handshake Key i.e HS_KEYp()
>> considers 'sizeof(PerlInterpreter)' to generate the key and somehow the
>> sizeof PerlInterpreter is not uniform in plperl and perl modules incase of
>> Windows but on Linux it remains same in both the modules.
>
> Yipes.

+1 for "yipes".  It sounds like we should really try to fix the
underlying problem, rather than just working around the check.

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

On Wed, Jul 19, 2017 at 9:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> > Actually the function used for generation of handshake Key i.e HS_KEYp()
> > considers 'sizeof(PerlInterpreter)' to generate the key and somehow the
> > sizeof PerlInterpreter is not uniform in plperl and perl modules incase of
> > Windows but on Linux it remains same in both the modules.
>
> Yipes.  So actually, this is catching a live ABI problem, which presumably
> we've escaped seeing bad effects from only through sheer good luck.
> I suppose that the discrepancies in the struct contents only occur after
> the last field that plperl happens to touch directly --- but that is
> unlikely to be true forever.
>
> > *typedef struct interpreter PerlInterpreter;struct interpreter {#  include
> > "intrpvar.h"};*
> > where intrpvar.h has different variables defined inside it and most of the
> > variables definition are protected with various macros. And there are some
> > macros that are just defined in perl but not in plperl module which means
> > the sizeof(PerlInterpreter) on the two modules are going to be different
> > and thereby resulting in a  different key.
>
> I imagine the route to a solution is to fix things so that the relevant
> macros are all defined correctly in both cases.  But why they aren't
> already is certainly an interesting question.  Have you identified just
> which fields are added or missing relative to what libperl thinks?

Here are the list of macros and variables from 'intrpvar.h' file that
are just defined in perl module but not in plperl on Windows,

#ifdef PERL_USES_PL_PIDSTATUS
PERLVAR(I, pidstatus,   HV *)       /* pid-to-status mappings for waitpid */
#endif

#ifdef PERL_SAWAMPERSAND
PERLVAR(I, sawampersand, U8)        /* must save all match strings */
#endif

#ifdef FCRYPT
PERLVARI(I, cryptseen,  bool,   FALSE)  /* has fast crypt() been initialized? */
#else
/* One byte hole in the interpreter structure.  */
#endif

#ifdef USE_REENTRANT_API
PERLVAR(I, reentrant_buffer, REENTR *)  /* here we store the _r buffers */
#endif

#ifdef PERL_GLOBAL_STRUCT_PRIVATE
PERLVARI(I, my_cxt_keys, const char **, NULL) /* per-module array of
pointers to MY_CXT_KEY constants */
# endif

#ifdef DEBUG_LEAKING_SCALARS_FORK_DUMP
/* File descriptor to talk to the child which dumps scalars.  */
PERLVARI(I, dumper_fd,  int,    -1)
#endif

#ifdef DEBUG_LEAKING_SCALARS
PERLVARI(I, sv_serial,  U32,    0)  /* SV serial number, used in sv.c */
#endif

#ifdef PERL_TRACE_OPS
PERLVARA(I, op_exec_cnt, OP_max+2, UV)
#endif

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> On Wed, Jul 19, 2017 at 9:42 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I imagine the route to a solution is to fix things so that the relevant
>> macros are all defined correctly in both cases.  But why they aren't
>> already is certainly an interesting question.  Have you identified just
>> which fields are added or missing relative to what libperl thinks?

> Here are the list of macros and variables from 'intrpvar.h' file that
> are just defined in perl module but not in plperl on Windows,

> #ifdef PERL_USES_PL_PIDSTATUS
> PERLVAR(I, pidstatus,   HV *)       /* pid-to-status mappings for waitpid */
> #endif

> #ifdef PERL_SAWAMPERSAND
> PERLVAR(I, sawampersand, U8)        /* must save all match strings */
> #endif

> #ifdef FCRYPT
> PERLVARI(I, cryptseen,  bool,   FALSE)  /* has fast crypt() been initialized? */
> #else
> /* One byte hole in the interpreter structure.  */
> #endif

> #ifdef USE_REENTRANT_API
> PERLVAR(I, reentrant_buffer, REENTR *)  /* here we store the _r buffers */
> #endif

> #ifdef PERL_GLOBAL_STRUCT_PRIVATE
> PERLVARI(I, my_cxt_keys, const char **, NULL) /* per-module array of
> pointers to MY_CXT_KEY constants */
> # endif

> #ifdef DEBUG_LEAKING_SCALARS_FORK_DUMP
> /* File descriptor to talk to the child which dumps scalars.  */
> PERLVARI(I, dumper_fd,  int,    -1)
> #endif

> #ifdef DEBUG_LEAKING_SCALARS
> PERLVARI(I, sv_serial,  U32,    0)  /* SV serial number, used in sv.c */
> #endif

> #ifdef PERL_TRACE_OPS
> PERLVARA(I, op_exec_cnt, OP_max+2, UV)
> #endif

Huh.  So those seem like symbols that ought to be exposed somewhere in
Perl's headers.  Perhaps we're failing to #include some "perl_config.h" or
equivalent file that records these ABI options?
        regards, tom lane

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> Here are the list of macros and variables from 'intrpvar.h' file that
> are just defined in perl module but not in plperl on Windows,

> #ifdef PERL_USES_PL_PIDSTATUS
> PERLVAR(I, pidstatus,   HV *)       /* pid-to-status mappings for waitpid */
> #endif

> #ifdef PERL_SAWAMPERSAND
> PERLVAR(I, sawampersand, U8)        /* must save all match strings */
> #endif

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.

Where did your perl installation come from, anyway?  Are you sure the .h
files match up with the executables?
        regards, tom lane

On Wed, Jul 19, 2017 at 05:01:31PM -0400, Tom Lane wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> > Here are the list of macros and variables from 'intrpvar.h' file that
> > are just defined in perl module but not in plperl on Windows,
> 
> > #ifdef PERL_USES_PL_PIDSTATUS
> > PERLVAR(I, pidstatus,   HV *)       /* pid-to-status mappings for waitpid */
> > #endif
> 
> > #ifdef PERL_SAWAMPERSAND
> > PERLVAR(I, sawampersand, U8)        /* must save all match strings */
> > #endif
> 
> 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.
> 
> Where did your perl installation come from, anyway?  Are you sure the .h
> files match up with the executables?

I see corresponding symptoms with the following Perl distributions:

strawberry-perl-5.26.0.1-64bit.msi: src/pl/plperl/Util.c: loadable library and perl binaries are mismatched (got
handshakekey 0000000011800080, needed 0000000011c00080)
 
ActivePerl-5.24.1.2402-MSWin32-x64-401627.exe: src/pl/plperl/Util.c: loadable library and perl binaries are mismatched
(gothandshake key 0000000011500080, needed 0000000011900080)
 

So, this affects each of the two prominent families of Perl Windows binaries.
Notes for anyone trying to reproduce:

- Both of those Perl distributions require the hacks described here:
https://www.postgresql.org/message-id/flat/CABcP5fjEjgOsh097cWnQrsK9yCswo4DZxp-V47DKCH-MxY9Gig%40mail.gmail.com
- Add PERL_USE_UNSAFE_INC=1 to the environment until we update things to cope with this Perl 5.26 change:
http://search.cpan.org/~xsawyerx/perl-5.26.0/pod/perldelta.pod#Removal_of_the_current_directory_(%22.%22)_from_@INC

On Mon, Jul 24, 2017 at 11:58 AM, Noah Misch <noah@leadboat.com> wrote:
> On Wed, Jul 19, 2017 at 05:01:31PM -0400, Tom Lane wrote:
>> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
>> > Here are the list of macros and variables from 'intrpvar.h' file that
>> > are just defined in perl module but not in plperl on Windows,
>>
>> > #ifdef PERL_USES_PL_PIDSTATUS
>> > PERLVAR(I, pidstatus,   HV *)       /* pid-to-status mappings for waitpid */
>> > #endif
>>
>> > #ifdef PERL_SAWAMPERSAND
>> > PERLVAR(I, sawampersand, U8)        /* must save all match strings */
>> > #endif
>>
>> 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.
>>
>> Where did your perl installation come from, anyway?  Are you sure the .h
>> files match up with the executables?
>
> I see corresponding symptoms with the following Perl distributions:
>
> strawberry-perl-5.26.0.1-64bit.msi:
>   src/pl/plperl/Util.c: loadable library and perl binaries are mismatched (got handshake key 0000000011800080, needed
0000000011c00080)
> ActivePerl-5.24.1.2402-MSWin32-x64-401627.exe:
>   src/pl/plperl/Util.c: loadable library and perl binaries are mismatched (got handshake key 0000000011500080, needed
0000000011900080)
>
> So, this affects each of the two prominent families of Perl Windows binaries.
>

I think the real question is where do we go from here.  Ashutosh has
proposed a patch up-thread based on a suggestion from Andrew, but it
is not clear if we want to go there as that seems to be bypassing
handshake mechanism.  The other tests and analysis seem to indicate
that the new version Perl binaries on Windows are getting built with
flags that are incompatible with what we use for plperl and it is not
clear why perl is using those flags.  Do you think we can do something
at our end to make it work or someone should check with Perl community
about it?

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

On Wed, Jul 19, 2017 at 5:01 PM, Tom Lane <tgl@sss.pgh.pa.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@cpan.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
willproduce 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_SAWAMPERSANDwill   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.

On 07/25/2017 08:58 AM, Amit Kapila wrote:
>
> I think the real question is where do we go from here.  Ashutosh has
> proposed a patch up-thread based on a suggestion from Andrew, but it
> is not clear if we want to go there as that seems to be bypassing
> handshake mechanism.  The other tests and analysis seem to indicate
> that the new version Perl binaries on Windows are getting built with
> flags that are incompatible with what we use for plperl and it is not
> clear why perl is using those flags.  Do you think we can do something
> at our end to make it work or someone should check with Perl community
> about it?
>


No amount of checking with the Perl community is likely to resolve this
quickly w.r.t. existing releases of Perl.

We seem to have a choice either to abandon, at least temporarily, perl
support on Windows for versions of perl >= 5.20 (I think) or adopt the
suggestion I made. It's not a complete hack - it's something at least
somewhat blessed in perl's XSUB.h:
   /* dXSBOOTARGSNOVERCHK has no API in xsubpp to choose it so do   #undef dXSBOOTARGSXSAPIVERCHK   #define
dXSBOOTARGSXSAPIVERCHKdXSBOOTARGSNOVERCHK */
 


Note that on earlier versions of perl we got by without this check for
years without any issue or complaint I recall hearing of. If we don't
adopt this I would not be at all surprised to see Windows packagers
adopt it anyway.

cheers

andrew



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

Amit Kapila <amit.kapila16@gmail.com> writes:
> I think the real question is where do we go from here.  Ashutosh has
> proposed a patch up-thread based on a suggestion from Andrew, but it
> is not clear if we want to go there as that seems to be bypassing
> handshake mechanism.

That definitely seems like the wrong route to me.  If the resulting
code works, it would at best be accidental.

> The other tests and analysis seem to indicate
> that the new version Perl binaries on Windows are getting built with
> flags that are incompatible with what we use for plperl and it is not
> clear why perl is using those flags.  Do you think we can do something
> at our end to make it work or someone should check with Perl community
> about it?

It would be a good idea to find somebody who knows more about how these
Perl distros are built.
        regards, tom lane

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> No amount of checking with the Perl community is likely to resolve this
> quickly w.r.t. existing releases of Perl.

Yes, but if they are shipping broken perl builds that cannot support
building of extension modules, they need to be made aware of that.
If that *isn't* the explanation, then we need to find out what is.
        regards, tom lane

Robert Haas <robertmhaas@gmail.com> writes:
> 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.
> ...
> 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.

Hm, I had the idea that we were already asking ExtUtils::Embed for that,
but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
like a promising avenue to pursue.

It would be useful to see the results of

perl -MExtUtils::Embed -e ccopts

on one of the affected installations, and compare that to the problematic
field(s).
        regards, tom lane

On 07/25/2017 11:00 AM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> 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.
>> ...
>> 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.
> Hm, I had the idea that we were already asking ExtUtils::Embed for that,
> but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
> like a promising avenue to pursue.
>
> It would be useful to see the results of
>
> perl -MExtUtils::Embed -e ccopts
>
> on one of the affected installations, and compare that to the problematic
> field(s).
 -s -O2 -DWIN32 -DWIN64 -DCONSERVATIVE -DPERL_TEXTMODE_SCRIPTS
-DUSE_SITECUSTOMIZE -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -fwrapv
-fno-strict-aliasing -mms-bitfields  -I"C:\Perl64\lib\CORE"

cheers

andrew

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

On Tue, Jul 25, 2017 at 11:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm, I had the idea that we were already asking ExtUtils::Embed for that,
> but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
> like a promising avenue to pursue.
>
> It would be useful to see the results of
>
> perl -MExtUtils::Embed -e ccopts
>
> on one of the affected installations, and compare that to the problematic
> field(s).

Why ccopts rather than ccflags?

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 25, 2017 at 11:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Hm, I had the idea that we were already asking ExtUtils::Embed for that,
>> but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
>> like a promising avenue to pursue.
>> 
>> It would be useful to see the results of
>> perl -MExtUtils::Embed -e ccopts
>> on one of the affected installations, and compare that to the problematic
>> field(s).

> Why ccopts rather than ccflags?

I was looking at the current code which fetches ldopts, and analogizing.
Don't know the difference between ccflags and ccopts.
        regards, tom lane

On Tue, Jul 25, 2017 at 11:32 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Jul 25, 2017 at 11:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Hm, I had the idea that we were already asking ExtUtils::Embed for that,
>>> but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
>>> like a promising avenue to pursue.
>>>
>>> It would be useful to see the results of
>>> perl -MExtUtils::Embed -e ccopts
>>> on one of the affected installations, and compare that to the problematic
>>> field(s).
>
>> Why ccopts rather than ccflags?
>
> I was looking at the current code which fetches ldopts, and analogizing.
> Don't know the difference between ccflags and ccopts.

Oh, here I was thinking you were 3 steps ahead of me.  :-)

Per "perldoc ExtUtils::Embed", ccopts() = perl_inc() plus ccflags()
plus ccdlflags().

On my system:

[rhaas pgsql]$ perl -MExtUtils::Embed -e 'for (qw(ccopts ccflags
ccdlflags perl_inc)) { print "==$_==\n"; eval "$_()"; print "\n\n";
};'
==ccopts==-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
-I/opt/local/lib/perl5/5.24/darwin-thread-multi-2level/CORE

==ccflags==-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

==ccdlflags==


==perl_inc==-I/opt/local/lib/perl5/5.24/darwin-thread-multi-2level/CORE

I don't have a clear sense of whether ccopts() or ccflags() is what we
want here, but FWIW ccopts() is more inclusive.

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

On 07/25/2017 11:32 AM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Tue, Jul 25, 2017 at 11:00 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Hm, I had the idea that we were already asking ExtUtils::Embed for that,
>>> but now I see we only inquire about LDFLAGS not CCFLAGS.  Yes, this sounds
>>> like a promising avenue to pursue.
>>>
>>> It would be useful to see the results of
>>> perl -MExtUtils::Embed -e ccopts
>>> on one of the affected installations, and compare that to the problematic
>>> field(s).
>> Why ccopts rather than ccflags?
> I was looking at the current code which fetches ldopts, and analogizing.
> Don't know the difference between ccflags and ccopts.
>
>             


per docs:
   ccopts()           This function combines "perl_inc()", "ccflags()" and   "ccdlflags()"           into one.

cheers

andrew

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

On Tue, Jul 25, 2017 at 10:23 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> 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"};')

Based on discussion downthread, it seems like what we actually need to
do is update perl.m4 to extract CCFLAGS.  Turns out somebody proposed
a patch for that back in 2002:

https://www.postgresql.org/message-id/Pine.LNX.4.44.0211051045070.16317-200000%40wotan.suse.de

It seems to need a rebase.  :-)

And maybe some other changes, too.  I haven't carefully reviewed that thread.

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

Robert Haas <robertmhaas@gmail.com> writes:
> Based on discussion downthread, it seems like what we actually need to
> do is update perl.m4 to extract CCFLAGS.  Turns out somebody proposed
> a patch for that back in 2002:
> https://www.postgresql.org/message-id/Pine.LNX.4.44.0211051045070.16317-200000%40wotan.suse.de
> It seems to need a rebase.  :-)

Ah-hah, I *thought* we had considered the question once upon a time.
There were some pretty substantial compatibility concerns raised in that
thread, which is doubtless why it's still like that.

My beef about inter-compiler compatibility (if building PG with a
different compiler from that used for Perl) could probably be addressed by
absorbing only -D switches from the Perl flags.  But Peter seemed to feel
that even that could break things, and I worry that he's right for cases
like -D_FILE_OFFSET_BITS which affect libc APIs.  Usually we'd have made
the same decisions as Perl for that sort of thing, but if we didn't, it's
a mess.

I wonder whether we could adopt some rule like "absorb -D switches
for macros whose names do not begin with an underscore".  That's
surely a hack and three-quarters, but it seems safer than just
absorbing everything willy-nilly.
        regards, tom lane

On Wed, Jul 26, 2017 at 8:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> Based on discussion downthread, it seems like what we actually need to
>> do is update perl.m4 to extract CCFLAGS.  Turns out somebody proposed
>> a patch for that back in 2002:
>> https://www.postgresql.org/message-id/Pine.LNX.4.44.0211051045070.16317-200000%40wotan.suse.de
>> It seems to need a rebase.  :-)
>
> Ah-hah, I *thought* we had considered the question once upon a time.
> There were some pretty substantial compatibility concerns raised in that
> thread, which is doubtless why it's still like that.
>
> My beef about inter-compiler compatibility (if building PG with a
> different compiler from that used for Perl) could probably be addressed by
> absorbing only -D switches from the Perl flags.  But Peter seemed to feel
> that even that could break things, and I worry that he's right for cases
> like -D_FILE_OFFSET_BITS which affect libc APIs.  Usually we'd have made
> the same decisions as Perl for that sort of thing, but if we didn't, it's
> a mess.
>
> I wonder whether we could adopt some rule like "absorb -D switches
> for macros whose names do not begin with an underscore".  That's
> surely a hack and three-quarters, but it seems safer than just
> absorbing everything willy-nilly.
>

Thanks Robert, Tom, Andrew and Amit for all your inputs. I have tried
to work on the patch shared by Reinhard  long time back for Linux. I
had to rebase the patch and also had to do add some more lines of code
to make it work on Linux. For Windows, I had to prepare a separate
patch to replicate similar behaviour.  I can see that both these
patches are working as expected i.e they are able import the switches
used by Perl into plperl module during build time.  However, on
windows i am still seeing the crash and i am still working to find the
reason for this.  Here, I attach the patches that i have prepared for
linux and Windows platforms. Thanks.

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi All,

On Wed, Jul 26, 2017 at 7:56 PM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> On Wed, Jul 26, 2017 at 8:51 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> Based on discussion downthread, it seems like what we actually need to
>>> do is update perl.m4 to extract CCFLAGS.  Turns out somebody proposed
>>> a patch for that back in 2002:
>>> https://www.postgresql.org/message-id/Pine.LNX.4.44.0211051045070.16317-200000%40wotan.suse.de
>>> It seems to need a rebase.  :-)
>>
>> Ah-hah, I *thought* we had considered the question once upon a time.
>> There were some pretty substantial compatibility concerns raised in that
>> thread, which is doubtless why it's still like that.
>>
>> My beef about inter-compiler compatibility (if building PG with a
>> different compiler from that used for Perl) could probably be addressed by
>> absorbing only -D switches from the Perl flags.  But Peter seemed to feel
>> that even that could break things, and I worry that he's right for cases
>> like -D_FILE_OFFSET_BITS which affect libc APIs.  Usually we'd have made
>> the same decisions as Perl for that sort of thing, but if we didn't, it's
>> a mess.
>>
>> I wonder whether we could adopt some rule like "absorb -D switches
>> for macros whose names do not begin with an underscore".  That's
>> surely a hack and three-quarters, but it seems safer than just
>> absorbing everything willy-nilly.
>>
>
> Thanks Robert, Tom, Andrew and Amit for all your inputs. I have tried
> to work on the patch shared by Reinhard  long time back for Linux. I
> had to rebase the patch and also had to do add some more lines of code
> to make it work on Linux. For Windows, I had to prepare a separate
> patch to replicate similar behaviour.  I can see that both these
> patches are working as expected i.e they are able import the switches
> used by Perl into plperl module during build time.  However, on
> windows i am still seeing the crash and i am still working to find the
> reason for this.  Here, I attach the patches that i have prepared for
> linux and Windows platforms. Thanks.

There was a small problem with my patch for windows that i had
submitted yesterday. It was reading the switches in '-D*' form and
including those as it is in the plperl build. Infact, it should be
removing '-D' option (from say -DPERL_CORE) and just add the macro
name (PERL_CORE) to the define list using AddDefine('PERL_CORE').
Anyways, attached is the patch that corrects this issue. The patch now
imports all the switches used by perl into plperl module but, after
doing so, i am seeing some compilation errors on Windows. Following is
the error observed,

SPI.obj : error LNK2019: unresolved external symbol PerlProc_setjmp
referenced in function do_plperl_return_next

I did some analysis in order to find the reason for this error and
could see that for Windows, sigsetjmp is defined as setjmp in PG. But,
setjmp is also defined by Perl. Hence, after including perl header
files, setjmp gets redefined as 'PerlProc_setjmp'.

In short, we have 'src/pl/plperl/SPI.c' file including 'perl.h'
followed 'XSUB.h'. perl.h defines PerlProc_setjmp() as,

#define PerlProc_setjmp(b, n) Sigsetjmp((b), (n))

and Sigsetjmp is defined as:

#define Sigsetjmp(buf,save_mask) setjmp((buf))

Then XSUB.h redefines setjmp as:

# define setjmp PerlProc_setjmp

which basically creates a loop in the preprocessor definitions.

Another problem with setjmp() redefinition is that setjmp takes one
argument whereas PerlProc_setjmp takes two arguments.

To fix this compilation error i have removed the setjmp() redefinition
from XSUB.h in perl and after doing that the build succeeds and also
'create language plperl' command is working fine i.e. there is no more
server crash.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> Anyways, attached is the patch that corrects this issue. The patch now
> imports all the switches used by perl into plperl module but, after
> doing so, i am seeing some compilation errors on Windows. Following is
> the error observed,

> SPI.obj : error LNK2019: unresolved external symbol PerlProc_setjmp
> referenced in function do_plperl_return_next

That's certainly a mess, but how come that wasn't happening before?
        regards, tom lane

On 07/27/2017 12:34 PM, Ashutosh Sharma wrote:
> On Thu, Jul 27, 2017 at 7:51 PM, Tom Lane <tgl@sss.pgh.pa.us
> <mailto:tgl@sss.pgh.pa.us>> wrote:
> > Ashutosh Sharma <ashu.coek88@gmail.com
> <mailto:ashu.coek88@gmail.com>> writes:
> >> Anyways, attached is the patch that corrects this issue. The patch now
> >> imports all the switches used by perl into plperl module but, after
> >> doing so, i am seeing some compilation errors on Windows. Following is
> >> the error observed,
> >
> >> SPI.obj : error LNK2019: unresolved external symbol PerlProc_setjmp
> >> referenced in function do_plperl_return_next
> >
> > That's certainly a mess, but how come that wasn't happening before?
>
> Earlier we were using Perl-5.20 version which i think didn't have
> handshaking mechanism. From perl-5.22 onwards, the functions like
> Perl_xs_handshake() or HS_KEY were introduced for handshaking purpose
> and to ensure that the handshaking between plperl and perl doesn't
> fail, we are now trying to import the switches used by  perl into
> plperl. As a result of this, macros like PERL_IMPLICIT_SYS is getting
> defined in plperl which eventually opens the following definitions
> from XSUB.h resulting in the compilation error.
>
>    499 #if defined(PERL_IMPLICIT_SYS) && !defined(PERL_CORE)
>     518 #    undef ioctl
>     519 #    undef getlogin
>     520*#    undef setjmp*
>     ...........
>     ...........
>
>     651 #    define times               PerlProc_times
>     652 #    define wait                PerlProc_wait
>     653 *#    define setjmp              PerlProc_setjmp
>
> *



What is the minimal set of extra defines required to sort out the
handshake fingerprint issue?

cheers

andrew

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

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> What is the minimal set of extra defines required to sort out the
> handshake fingerprint issue?

Also, exactly what defines do you end up importing in your test build?
        regards, tom lane

On Thu, Jul 27, 2017 at 10:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
>> Anyways, attached is the patch that corrects this issue. The patch now
>> imports all the switches used by perl into plperl module but, after
>> doing so, i am seeing some compilation errors on Windows. Following is
>> the error observed,
>
>> SPI.obj : error LNK2019: unresolved external symbol PerlProc_setjmp
>> referenced in function do_plperl_return_next
>
> That's certainly a mess, but how come that wasn't happening before?

How about we fix it like this?

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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> How about we fix it like this?

That seems pretty invasive; I'm not excited about breaking a lot of
unrelated code (particularly third-party extensions) for plperl's benefit.
Even if we wanted to do that in HEAD, it seems like a nonstarter for
released branches.

An even bigger issue is that if Perl feels free to redefine sigsetjmp,
what other libc calls might they decide to horn in on?

So I was trying to figure a way to not include XSUB.h except in a very
limited part of plperl, like ideally just the .xs files.  It's looking
like that would take nontrivial refactoring though :-(.  Another problem
is that this critical bit of the library API is in XSUB.h:

#if defined(PERL_IMPLICIT_CONTEXT) && !defined(PERL_NO_GET_CONTEXT) && !defined(PERL_CORE)
#  undef aTHX
#  undef aTHX_
#  define aTHX        PERL_GET_THX
#  define aTHX_        aTHX,
#endif

As best I can tell, that's absolute brain death on the part of the Perl
crew; it means you can't write working calling code at all without
including XSUB.h, or at least copying-and-pasting this bit out of it.
        regards, tom lane

On 07/27/2017 04:33 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> How about we fix it like this?
> That seems pretty invasive; I'm not excited about breaking a lot of
> unrelated code (particularly third-party extensions) for plperl's benefit.
> Even if we wanted to do that in HEAD, it seems like a nonstarter for
> released branches.
>
> An even bigger issue is that if Perl feels free to redefine sigsetjmp,
> what other libc calls might they decide to horn in on?
>
> So I was trying to figure a way to not include XSUB.h except in a very
> limited part of plperl, like ideally just the .xs files.  It's looking
> like that would take nontrivial refactoring though :-(.  Another problem
> is that this critical bit of the library API is in XSUB.h:
>
> #if defined(PERL_IMPLICIT_CONTEXT) && !defined(PERL_NO_GET_CONTEXT) && !defined(PERL_CORE)
> #  undef aTHX
> #  undef aTHX_
> #  define aTHX        PERL_GET_THX
> #  define aTHX_        aTHX,
> #endif
>
> As best I can tell, that's absolute brain death on the part of the Perl
> crew; it means you can't write working calling code at all without
> including XSUB.h, or at least copying-and-pasting this bit out of it.
>
>             

That's the sort of thing that prompted me to ask what was the minimal
set of defines required to fix the original problem (assuming such a
thing exists)

We haven't used PERL_IMPLICIT_CONTEXT to date, and without ill effect.
For example. it's in the ExtUtils::Embed::ccopts for the perl that
jacana and bowerbird happily build and test against.

cheers

andrew


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

Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 07/27/2017 04:33 PM, Tom Lane wrote:
>> So I was trying to figure a way to not include XSUB.h except in a very
>> limited part of plperl, like ideally just the .xs files.  It's looking
>> like that would take nontrivial refactoring though :-(.  Another problem
>> is that this critical bit of the library API is in XSUB.h:

> That's the sort of thing that prompted me to ask what was the minimal
> set of defines required to fix the original problem (assuming such a
> thing exists)
> We haven't used PERL_IMPLICIT_CONTEXT to date, and without ill effect.
> For example. it's in the ExtUtils::Embed::ccopts for the perl that
> jacana and bowerbird happily build and test against.

Well, actually, PERL_IMPLICIT_CONTEXT is turned on automatically in any
MULTIPLICITY build, and since it changes all the Perl ABIs, we *are*
relying on it.  However, after further study of the Perl docs I noticed
that we could dispense with XSUB.h if we defined PERL_NO_GET_CONTEXT
(which turns the quoted stanza into a no-op).  That results in needing to
sprinkle plperl.c with "dTHX" declarations, as explained in perlguts.pod.
They're slightly tricky to place correctly, because they load up a pointer
to the current Perl interpreter, so you have to be wary of where to put
them in functions that change interpreters.  But I seem to have it working
in the attached patch.  (One benefit of doing this extra work is that it
should be a bit more efficient, in that we load up a Perl interpreter
pointer only once per function called, not once per usage therein.  We
could remove many of those fetches too, if we wanted to sprinkle the
plperl code with yet more THX droppings; but I left that for another day.)

Armed with that, I got rid of XSUB.h in plperl.c and moved the
PG_TRY-using functions in the .xs files to plperl.c.  I think this would
fix Ashutosh's problem, though I am not in a position to try it with a
PERL_IMPLICIT_SYS build here.

            regards, tom lane

diff --git a/src/pl/plperl/SPI.xs b/src/pl/plperl/SPI.xs
index 0447c50..7651b62 100644
*** a/src/pl/plperl/SPI.xs
--- b/src/pl/plperl/SPI.xs
***************
*** 15,52 ****
  #undef _

  /* perl stuff */
  #include "plperl.h"
  #include "plperl_helpers.h"


- /*
-  * Interface routine to catch ereports and punt them to Perl
-  */
- static void
- do_plperl_return_next(SV *sv)
- {
-     MemoryContext oldcontext = CurrentMemoryContext;
-
-     PG_TRY();
-     {
-         plperl_return_next(sv);
-     }
-     PG_CATCH();
-     {
-         ErrorData  *edata;
-
-         /* Must reset elog.c's state */
-         MemoryContextSwitchTo(oldcontext);
-         edata = CopyErrorData();
-         FlushErrorState();
-
-         /* Punt the error to Perl */
-         croak_cstr(edata->message);
-     }
-     PG_END_TRY();
- }
-
-
  MODULE = PostgreSQL::InServer::SPI PREFIX = spi_

  PROTOTYPES: ENABLE
--- 15,25 ----
  #undef _

  /* perl stuff */
+ #define PG_NEED_PERL_XSUB_H
  #include "plperl.h"
  #include "plperl_helpers.h"


  MODULE = PostgreSQL::InServer::SPI PREFIX = spi_

  PROTOTYPES: ENABLE
*************** void
*** 76,82 ****
  spi_return_next(rv)
      SV *rv;
      CODE:
!         do_plperl_return_next(rv);

  SV *
  spi_spi_query(sv)
--- 49,55 ----
  spi_return_next(rv)
      SV *rv;
      CODE:
!         plperl_return_next(rv);

  SV *
  spi_spi_query(sv)
diff --git a/src/pl/plperl/Util.xs b/src/pl/plperl/Util.xs
index dbba0d7..862fec4 100644
*** a/src/pl/plperl/Util.xs
--- b/src/pl/plperl/Util.xs
***************
*** 20,67 ****
  #undef _

  /* perl stuff */
  #include "plperl.h"
  #include "plperl_helpers.h"

- /*
-  * Implementation of plperl's elog() function
-  *
-  * If the error level is less than ERROR, we'll just emit the message and
-  * return.  When it is ERROR, elog() will longjmp, which we catch and
-  * turn into a Perl croak().  Note we are assuming that elog() can't have
-  * any internal failures that are so bad as to require a transaction abort.
-  *
-  * This is out-of-line to suppress "might be clobbered by longjmp" warnings.
-  */
- static void
- do_util_elog(int level, SV *msg)
- {
-     MemoryContext oldcontext = CurrentMemoryContext;
-     char       * volatile cmsg = NULL;
-
-     PG_TRY();
-     {
-         cmsg = sv2cstr(msg);
-         elog(level, "%s", cmsg);
-         pfree(cmsg);
-     }
-     PG_CATCH();
-     {
-         ErrorData  *edata;
-
-         /* Must reset elog.c's state */
-         MemoryContextSwitchTo(oldcontext);
-         edata = CopyErrorData();
-         FlushErrorState();
-
-         if (cmsg)
-             pfree(cmsg);
-
-         /* Punt the error to Perl */
-         croak_cstr(edata->message);
-     }
-     PG_END_TRY();
- }

  static text *
  sv2text(SV *sv)
--- 20,29 ----
  #undef _

  /* perl stuff */
+ #define PG_NEED_PERL_XSUB_H
  #include "plperl.h"
  #include "plperl_helpers.h"


  static text *
  sv2text(SV *sv)
*************** util_elog(level, msg)
*** 105,111 ****
              level = ERROR;
          if (level < DEBUG5)
              level = DEBUG5;
!         do_util_elog(level, msg);

  SV *
  util_quote_literal(sv)
--- 67,73 ----
              level = ERROR;
          if (level < DEBUG5)
              level = DEBUG5;
!         plperl_util_elog(level, msg);

  SV *
  util_quote_literal(sv)
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 5a45e3e..9a26470 100644
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
*************** static void plperl_init_shared_libs(pTHX
*** 285,290 ****
--- 285,291 ----
  static void plperl_trusted_init(void);
  static void plperl_untrusted_init(void);
  static HV  *plperl_spi_execute_fetch_result(SPITupleTable *, uint64, int);
+ static void plperl_return_next_internal(SV *sv);
  static char *hek2cstr(HE *he);
  static SV **hv_store_string(HV *hv, const char *key, SV *val);
  static SV **hv_fetch_string(HV *hv, const char *key);
*************** static char *setlocale_perl(int category
*** 303,313 ****
--- 304,326 ----
  #endif

  /*
+  * Decrement the refcount of the given SV within the active Perl interpreter
+  */
+ static inline void
+ SvREFCNT_dec_current(SV *sv)
+ {
+     dTHX;
+
+     SvREFCNT_dec(sv);
+ }
+
+ /*
   * convert a HE (hash entry) key to a cstr in the current database encoding
   */
  static char *
  hek2cstr(HE *he)
  {
+     dTHX;
      char       *ret;
      SV           *sv;

*************** select_perl_context(bool trusted)
*** 641,655 ****
       * to the database AFTER on_*_init code has run. See
       * http://archives.postgresql.org/pgsql-hackers/2010-01/msg02669.php
       */
!     newXS("PostgreSQL::InServer::SPI::bootstrap",
!           boot_PostgreSQL__InServer__SPI, __FILE__);

!     eval_pv("PostgreSQL::InServer::SPI::bootstrap()", FALSE);
!     if (SvTRUE(ERRSV))
!         ereport(ERROR,
!                 (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                  errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                  errcontext("while executing PostgreSQL::InServer::SPI::bootstrap")));

      /* Fully initialized, so mark the hashtable entry valid */
      interp_desc->interp = interp;
--- 654,672 ----
       * to the database AFTER on_*_init code has run. See
       * http://archives.postgresql.org/pgsql-hackers/2010-01/msg02669.php
       */
!     {
!         dTHX;

!         newXS("PostgreSQL::InServer::SPI::bootstrap",
!               boot_PostgreSQL__InServer__SPI, __FILE__);
!
!         eval_pv("PostgreSQL::InServer::SPI::bootstrap()", FALSE);
!         if (SvTRUE(ERRSV))
!             ereport(ERROR,
!                     (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                      errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                      errcontext("while executing PostgreSQL::InServer::SPI::bootstrap")));
!     }

      /* Fully initialized, so mark the hashtable entry valid */
      interp_desc->interp = interp;
*************** plperl_init_interp(void)
*** 792,844 ****
      PERL_SET_CONTEXT(plperl);
      perl_construct(plperl);

-     /* run END blocks in perl_destruct instead of perl_run */
-     PL_exit_flags |= PERL_EXIT_DESTRUCT_END;
-
      /*
!      * Record the original function for the 'require' and 'dofile' opcodes.
!      * (They share the same implementation.) Ensure it's used for new
!      * interpreters.
       */
-     if (!pp_require_orig)
-         pp_require_orig = PL_ppaddr[OP_REQUIRE];
-     else
      {
!         PL_ppaddr[OP_REQUIRE] = pp_require_orig;
!         PL_ppaddr[OP_DOFILE] = pp_require_orig;
!     }

  #ifdef PLPERL_ENABLE_OPMASK_EARLY

!     /*
!      * For regression testing to prove that the PLC_PERLBOOT and PLC_TRUSTED
!      * code doesn't even compile any unsafe ops. In future there may be a
!      * valid need for them to do so, in which case this could be softened
!      * (perhaps moved to plperl_trusted_init()) or removed.
!      */
!     PL_op_mask = plperl_opmask;
  #endif

!     if (perl_parse(plperl, plperl_init_shared_libs,
!                    nargs, embedding, NULL) != 0)
!         ereport(ERROR,
!                 (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                  errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                  errcontext("while parsing Perl initialization")));

!     if (perl_run(plperl) != 0)
!         ereport(ERROR,
!                 (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                  errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                  errcontext("while running Perl initialization")));

  #ifdef PLPERL_RESTORE_LOCALE
!     PLPERL_RESTORE_LOCALE(LC_COLLATE, save_collate);
!     PLPERL_RESTORE_LOCALE(LC_CTYPE, save_ctype);
!     PLPERL_RESTORE_LOCALE(LC_MONETARY, save_monetary);
!     PLPERL_RESTORE_LOCALE(LC_NUMERIC, save_numeric);
!     PLPERL_RESTORE_LOCALE(LC_TIME, save_time);
  #endif

      return plperl;
  }
--- 809,870 ----
      PERL_SET_CONTEXT(plperl);
      perl_construct(plperl);

      /*
!      * Run END blocks in perl_destruct instead of perl_run.  Note that dTHX
!      * loads up a pointer to the current interpreter, so we have to postpone
!      * it to here rather than put it at the function head.
       */
      {
!         dTHX;
!
!         PL_exit_flags |= PERL_EXIT_DESTRUCT_END;
!
!         /*
!          * Record the original function for the 'require' and 'dofile'
!          * opcodes.  (They share the same implementation.)  Ensure it's used
!          * for new interpreters.
!          */
!         if (!pp_require_orig)
!             pp_require_orig = PL_ppaddr[OP_REQUIRE];
!         else
!         {
!             PL_ppaddr[OP_REQUIRE] = pp_require_orig;
!             PL_ppaddr[OP_DOFILE] = pp_require_orig;
!         }

  #ifdef PLPERL_ENABLE_OPMASK_EARLY

!         /*
!          * For regression testing to prove that the PLC_PERLBOOT and
!          * PLC_TRUSTED code doesn't even compile any unsafe ops.  In future
!          * there may be a valid need for them to do so, in which case this
!          * could be softened (perhaps moved to plperl_trusted_init()) or
!          * removed.
!          */
!         PL_op_mask = plperl_opmask;
  #endif

!         if (perl_parse(plperl, plperl_init_shared_libs,
!                        nargs, embedding, NULL) != 0)
!             ereport(ERROR,
!                     (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                      errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                      errcontext("while parsing Perl initialization")));

!         if (perl_run(plperl) != 0)
!             ereport(ERROR,
!                     (errcode(ERRCODE_EXTERNAL_ROUTINE_EXCEPTION),
!                      errmsg("%s", strip_trailing_ws(sv2cstr(ERRSV))),
!                      errcontext("while running Perl initialization")));

  #ifdef PLPERL_RESTORE_LOCALE
!         PLPERL_RESTORE_LOCALE(LC_COLLATE, save_collate);
!         PLPERL_RESTORE_LOCALE(LC_CTYPE, save_ctype);
!         PLPERL_RESTORE_LOCALE(LC_MONETARY, save_monetary);
!         PLPERL_RESTORE_LOCALE(LC_NUMERIC, save_numeric);
!         PLPERL_RESTORE_LOCALE(LC_TIME, save_time);
  #endif
+     }

      return plperl;
  }
*************** plperl_destroy_interp(PerlInterpreter **
*** 904,909 ****
--- 930,936 ----
           * public API so isn't portably available.) Meanwhile END blocks can
           * be used to perform manual cleanup.
           */
+         dTHX;

          /* Run END blocks - based on perl's perl_destruct() */
          if (PL_exit_flags & PERL_EXIT_DESTRUCT_END)
*************** plperl_destroy_interp(PerlInterpreter **
*** 930,935 ****
--- 957,963 ----
  static void
  plperl_trusted_init(void)
  {
+     dTHX;
      HV           *stash;
      SV           *sv;
      char       *key;
*************** plperl_trusted_init(void)
*** 1010,1015 ****
--- 1038,1045 ----
  static void
  plperl_untrusted_init(void)
  {
+     dTHX;
+
      /*
       * Nothing to do except execute plperl.on_plperlu_init
       */
*************** strip_trailing_ws(const char *msg)
*** 1045,1050 ****
--- 1075,1081 ----
  static HeapTuple
  plperl_build_tuple_result(HV *perlhash, TupleDesc td)
  {
+     dTHX;
      Datum       *values;
      bool       *nulls;
      HE           *he;
*************** plperl_hash_to_datum(SV *src, TupleDesc
*** 1106,1111 ****
--- 1137,1144 ----
  static SV  *
  get_perl_array_ref(SV *sv)
  {
+     dTHX;
+
      if (SvOK(sv) && SvROK(sv))
      {
          if (SvTYPE(SvRV(sv)) == SVt_PVAV)
*************** array_to_datum_internal(AV *av, ArrayBui
*** 1134,1139 ****
--- 1167,1173 ----
                          Oid arraytypid, Oid elemtypid, int32 typmod,
                          FmgrInfo *finfo, Oid typioparam)
  {
+     dTHX;
      int            i;
      int            len = av_len(av) + 1;

*************** array_to_datum_internal(AV *av, ArrayBui
*** 1205,1210 ****
--- 1239,1245 ----
  static Datum
  plperl_array_to_datum(SV *src, Oid typid, int32 typmod)
  {
+     dTHX;
      ArrayBuildState *astate;
      Oid            elemtypid;
      FmgrInfo    finfo;
*************** plperl_sv_to_literal(SV *sv, char *fqtyp
*** 1407,1412 ****
--- 1442,1448 ----
  static SV  *
  plperl_ref_from_pg_array(Datum arg, Oid typid)
  {
+     dTHX;
      ArrayType  *ar = DatumGetArrayTypeP(arg);
      Oid            elementtype = ARR_ELEMTYPE(ar);
      int16        typlen;
*************** plperl_ref_from_pg_array(Datum arg, Oid
*** 1485,1490 ****
--- 1521,1527 ----
  static SV  *
  split_array(plperl_array_info *info, int first, int last, int nest)
  {
+     dTHX;
      int            i;
      AV           *result;

*************** split_array(plperl_array_info *info, int
*** 1518,1523 ****
--- 1555,1561 ----
  static SV  *
  make_array_ref(plperl_array_info *info, int first, int last)
  {
+     dTHX;
      int            i;
      AV           *result = newAV();

*************** make_array_ref(plperl_array_info *info,
*** 1555,1560 ****
--- 1593,1599 ----
  static SV  *
  plperl_trigger_build_args(FunctionCallInfo fcinfo)
  {
+     dTHX;
      TriggerData *tdata;
      TupleDesc    tupdesc;
      int            i;
*************** plperl_trigger_build_args(FunctionCallIn
*** 1661,1666 ****
--- 1700,1706 ----
  static SV  *
  plperl_event_trigger_build_args(FunctionCallInfo fcinfo)
  {
+     dTHX;
      EventTriggerData *tdata;
      HV           *hv;

*************** plperl_event_trigger_build_args(Function
*** 1678,1683 ****
--- 1718,1724 ----
  static HeapTuple
  plperl_modify_tuple(HV *hvTD, TriggerData *tdata, HeapTuple otup)
  {
+     dTHX;
      SV          **svp;
      HV           *hvNew;
      HE           *he;
*************** plperl_inline_handler(PG_FUNCTION_ARGS)
*** 1874,1880 ****

          perlret = plperl_call_perl_func(&desc, &fake_fcinfo);

!         SvREFCNT_dec(perlret);

          if (SPI_finish() != SPI_OK_FINISH)
              elog(ERROR, "SPI_finish() failed");
--- 1915,1921 ----

          perlret = plperl_call_perl_func(&desc, &fake_fcinfo);

!         SvREFCNT_dec_current(perlret);

          if (SPI_finish() != SPI_OK_FINISH)
              elog(ERROR, "SPI_finish() failed");
*************** plperl_inline_handler(PG_FUNCTION_ARGS)
*** 1882,1888 ****
      PG_CATCH();
      {
          if (desc.reference)
!             SvREFCNT_dec(desc.reference);
          current_call_data = save_call_data;
          activate_interpreter(oldinterp);
          PG_RE_THROW();
--- 1923,1929 ----
      PG_CATCH();
      {
          if (desc.reference)
!             SvREFCNT_dec_current(desc.reference);
          current_call_data = save_call_data;
          activate_interpreter(oldinterp);
          PG_RE_THROW();
*************** plperl_inline_handler(PG_FUNCTION_ARGS)
*** 1890,1896 ****
      PG_END_TRY();

      if (desc.reference)
!         SvREFCNT_dec(desc.reference);

      current_call_data = save_call_data;
      activate_interpreter(oldinterp);
--- 1931,1937 ----
      PG_END_TRY();

      if (desc.reference)
!         SvREFCNT_dec_current(desc.reference);

      current_call_data = save_call_data;
      activate_interpreter(oldinterp);
*************** plperlu_validator(PG_FUNCTION_ARGS)
*** 2018,2023 ****
--- 2059,2065 ----
  static void
  plperl_create_sub(plperl_proc_desc *prodesc, char *s, Oid fn_oid)
  {
+     dTHX;
      dSP;
      char        subname[NAMEDATALEN + 40];
      HV           *pragma_hv = newHV();
*************** plperl_init_shared_libs(pTHX)
*** 2104,2109 ****
--- 2146,2152 ----
  static SV  *
  plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
  {
+     dTHX;
      dSP;
      SV           *retval;
      int            i;
*************** static SV  *
*** 2197,2202 ****
--- 2240,2246 ----
  plperl_call_perl_trigger_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo,
                                SV *td)
  {
+     dTHX;
      dSP;
      SV           *retval,
                 *TDsv;
*************** plperl_call_perl_event_trigger_func(plpe
*** 2265,2270 ****
--- 2309,2315 ----
                                      FunctionCallInfo fcinfo,
                                      SV *td)
  {
+     dTHX;
      dSP;
      SV           *retval,
                 *TDsv;
*************** plperl_func_handler(PG_FUNCTION_ARGS)
*** 2384,2396 ****
          sav = get_perl_array_ref(perlret);
          if (sav)
          {
              int            i = 0;
              SV          **svp = 0;
              AV           *rav = (AV *) SvRV(sav);

              while ((svp = av_fetch(rav, i, FALSE)) != NULL)
              {
!                 plperl_return_next(*svp);
                  i++;
              }
          }
--- 2429,2442 ----
          sav = get_perl_array_ref(perlret);
          if (sav)
          {
+             dTHX;
              int            i = 0;
              SV          **svp = 0;
              AV           *rav = (AV *) SvRV(sav);

              while ((svp = av_fetch(rav, i, FALSE)) != NULL)
              {
!                 plperl_return_next_internal(*svp);
                  i++;
              }
          }
*************** plperl_func_handler(PG_FUNCTION_ARGS)
*** 2427,2433 ****
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec(perlret);

      return retval;
  }
--- 2473,2479 ----
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec_current(perlret);

      return retval;
  }
*************** plperl_trigger_handler(PG_FUNCTION_ARGS)
*** 2538,2546 ****
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec(svTD);
      if (perlret)
!         SvREFCNT_dec(perlret);

      return retval;
  }
--- 2584,2592 ----
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec_current(svTD);
      if (perlret)
!         SvREFCNT_dec_current(perlret);

      return retval;
  }
*************** plperl_event_trigger_handler(PG_FUNCTION
*** 2579,2587 ****
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec(svTD);
!
!     return;
  }


--- 2625,2631 ----
      /* Restore the previous error callback */
      error_context_stack = pl_error_context.previous;

!     SvREFCNT_dec_current(svTD);
  }


*************** free_plperl_function(plperl_proc_desc *p
*** 2624,2630 ****
          plperl_interp_desc *oldinterp = plperl_active_interp;

          activate_interpreter(prodesc->interp);
!         SvREFCNT_dec(prodesc->reference);
          activate_interpreter(oldinterp);
      }
      /* Release all PG-owned data for this proc */
--- 2668,2674 ----
          plperl_interp_desc *oldinterp = plperl_active_interp;

          activate_interpreter(prodesc->interp);
!         SvREFCNT_dec_current(prodesc->reference);
          activate_interpreter(oldinterp);
      }
      /* Release all PG-owned data for this proc */
*************** plperl_hash_from_datum(Datum attr)
*** 2949,2954 ****
--- 2993,2999 ----
  static SV  *
  plperl_hash_from_tuple(HeapTuple tuple, TupleDesc tupdesc)
  {
+     dTHX;
      HV           *hv;
      int            i;

*************** static HV  *
*** 3094,3099 ****
--- 3139,3145 ----
  plperl_spi_execute_fetch_result(SPITupleTable *tuptable, uint64 processed,
                                  int status)
  {
+     dTHX;
      HV           *result;

      check_spi_usage_allowed();
*************** plperl_spi_execute_fetch_result(SPITuple
*** 3137,3152 ****


  /*
!  * Note: plperl_return_next is called both in Postgres and Perl contexts.
!  * We report any errors in Postgres fashion (via ereport).  If called in
!  * Perl context, it is SPI.xs's responsibility to catch the error and
!  * convert to a Perl error.  We assume (perhaps without adequate justification)
!  * that we need not abort the current transaction if the Perl code traps the
!  * error.
   */
  void
  plperl_return_next(SV *sv)
  {
      plperl_proc_desc *prodesc;
      FunctionCallInfo fcinfo;
      ReturnSetInfo *rsi;
--- 3183,3223 ----


  /*
!  * plperl_return_next catches any error and converts it to a Perl error.
!  * We assume (perhaps without adequate justification) that we need not abort
!  * the current transaction if the Perl code traps the error.
   */
  void
  plperl_return_next(SV *sv)
  {
+     MemoryContext oldcontext = CurrentMemoryContext;
+
+     PG_TRY();
+     {
+         plperl_return_next_internal(sv);
+     }
+     PG_CATCH();
+     {
+         ErrorData  *edata;
+
+         /* Must reset elog.c's state */
+         MemoryContextSwitchTo(oldcontext);
+         edata = CopyErrorData();
+         FlushErrorState();
+
+         /* Punt the error to Perl */
+         croak_cstr(edata->message);
+     }
+     PG_END_TRY();
+ }
+
+ /*
+  * plperl_return_next_internal reports any errors in Postgres fashion
+  * (via ereport).
+  */
+ static void
+ plperl_return_next_internal(SV *sv)
+ {
      plperl_proc_desc *prodesc;
      FunctionCallInfo fcinfo;
      ReturnSetInfo *rsi;
*************** plperl_spi_fetchrow(char *cursor)
*** 3336,3341 ****
--- 3407,3413 ----

      PG_TRY();
      {
+         dTHX;
          Portal        p = SPI_cursor_find(cursor);

          if (!p)
*************** plperl_spi_exec_prepared(char *query, HV
*** 3577,3582 ****
--- 3649,3656 ----

      PG_TRY();
      {
+         dTHX;
+
          /************************************************************
           * Fetch the saved plan descriptor, see if it's o.k.
           ************************************************************/
*************** plperl_spi_freeplan(char *query)
*** 3822,3833 ****
--- 3896,3949 ----
  }

  /*
+  * Implementation of plperl's elog() function
+  *
+  * If the error level is less than ERROR, we'll just emit the message and
+  * return.  When it is ERROR, elog() will longjmp, which we catch and
+  * turn into a Perl croak().  Note we are assuming that elog() can't have
+  * any internal failures that are so bad as to require a transaction abort.
+  *
+  * The main reason this is out-of-line is to avoid conflicts between XSUB.h
+  * and the PG_TRY macros.
+  */
+ void
+ plperl_util_elog(int level, SV *msg)
+ {
+     MemoryContext oldcontext = CurrentMemoryContext;
+     char       *volatile cmsg = NULL;
+
+     PG_TRY();
+     {
+         cmsg = sv2cstr(msg);
+         elog(level, "%s", cmsg);
+         pfree(cmsg);
+     }
+     PG_CATCH();
+     {
+         ErrorData  *edata;
+
+         /* Must reset elog.c's state */
+         MemoryContextSwitchTo(oldcontext);
+         edata = CopyErrorData();
+         FlushErrorState();
+
+         if (cmsg)
+             pfree(cmsg);
+
+         /* Punt the error to Perl */
+         croak_cstr(edata->message);
+     }
+     PG_END_TRY();
+ }
+
+ /*
   * Store an SV into a hash table under a key that is a string assumed to be
   * in the current database's encoding.
   */
  static SV **
  hv_store_string(HV *hv, const char *key, SV *val)
  {
+     dTHX;
      int32        hlen;
      char       *hkey;
      SV          **ret;
*************** hv_store_string(HV *hv, const char *key,
*** 3854,3859 ****
--- 3970,3976 ----
  static SV **
  hv_fetch_string(HV *hv, const char *key)
  {
+     dTHX;
      int32        hlen;
      char       *hkey;
      SV          **ret;
diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h
index eecd192..3f3cce9 100644
*** a/src/pl/plperl/plperl.h
--- b/src/pl/plperl/plperl.h
***************
*** 44,52 ****
--- 44,60 ----


  /* required for perl API */
+ #define PERL_NO_GET_CONTEXT
  #include "EXTERN.h"
  #include "perl.h"
+
+ /*
+  * We want to include this only within .xs files, but it must come before
+  * ppport.h, so use a #define flag to control inclusion.
+  */
+ #ifdef PG_NEED_PERL_XSUB_H
  #include "XSUB.h"
+ #endif

  /* put back our snprintf and vsnprintf */
  #ifdef USE_REPL_SNPRINTF
*************** SV           *plperl_spi_query_prepared(char *
*** 106,110 ****
--- 114,119 ----
  void        plperl_spi_freeplan(char *);
  void        plperl_spi_cursor_close(char *);
  char       *plperl_sv_to_literal(SV *, char *);
+ void        plperl_util_elog(int level, SV *msg);

  #endif                            /* PL_PERL_H */
diff --git a/src/pl/plperl/plperl_helpers.h b/src/pl/plperl/plperl_helpers.h
index 76124ed..5acac60 100644
*** a/src/pl/plperl/plperl_helpers.h
--- b/src/pl/plperl/plperl_helpers.h
*************** sv2cstr(SV *sv)
*** 54,59 ****
--- 54,61 ----
                 *res;
      STRLEN        len;

+     dTHX;
+
      /*
       * get a utf8 encoded char * out of perl. *note* it may not be valid utf8!
       */
*************** cstr2sv(const char *str)
*** 110,115 ****
--- 112,119 ----
      SV           *sv;
      char       *utf8_str;

+     dTHX;
+
      /* no conversion when SQL_ASCII */
      if (GetDatabaseEncoding() == PG_SQL_ASCII)
          return newSVpv(str, 0);
*************** cstr2sv(const char *str)
*** 134,139 ****
--- 138,145 ----
  static inline void
  croak_cstr(const char *str)
  {
+     dTHX;
+
  #ifdef croak_sv
      /* Use sv_2mortal() to be sure the transient SV gets freed */
      croak_sv(sv_2mortal(cstr2sv(str)));

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

On Fri, Jul 28, 2017 at 4:20 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> > On 07/27/2017 04:33 PM, Tom Lane wrote:
> >> So I was trying to figure a way to not include XSUB.h except in a very
> >> limited part of plperl, like ideally just the .xs files.  It's looking
> >> like that would take nontrivial refactoring though :-(.  Another problem
> >> is that this critical bit of the library API is in XSUB.h:
>
> > That's the sort of thing that prompted me to ask what was the minimal
> > set of defines required to fix the original problem (assuming such a
> > thing exists)
> > We haven't used PERL_IMPLICIT_CONTEXT to date, and without ill effect.
> > For example. it's in the ExtUtils::Embed::ccopts for the perl that
> > jacana and bowerbird happily build and test against.
>
> Well, actually, PERL_IMPLICIT_CONTEXT is turned on automatically in any
> MULTIPLICITY build, and since it changes all the Perl ABIs, we *are*
> relying on it.  However, after further study of the Perl docs I noticed
> that we could dispense with XSUB.h if we defined PERL_NO_GET_CONTEXT
> (which turns the quoted stanza into a no-op).  That results in needing to
> sprinkle plperl.c with "dTHX" declarations, as explained in perlguts.pod.
> They're slightly tricky to place correctly, because they load up a pointer
> to the current Perl interpreter, so you have to be wary of where to put
> them in functions that change interpreters.  But I seem to have it working
> in the attached patch.  (One benefit of doing this extra work is that it
> should be a bit more efficient, in that we load up a Perl interpreter
> pointer only once per function called, not once per usage therein.  We
> could remove many of those fetches too, if we wanted to sprinkle the
> plperl code with yet more THX droppings; but I left that for another day.)
>
> Armed with that, I got rid of XSUB.h in plperl.c and moved the
> PG_TRY-using functions in the .xs files to plperl.c.  I think this would
> fix Ashutosh's problem, though I am not in a position to try it with a
> PERL_IMPLICIT_SYS build here.

Thanks for the patch. I am seeing some compilation errors on Windows
with the patch. Below are the errors observed,

1>ClCompile:
1>  plperl.c
1>src/pl/plperl/plperl.c(4051): error C2065: 'my_perl' : undeclared identifier

2>ClCompile:
2>  hstore_plperl.c
2>contrib/hstore_plperl/hstore_plperl.c(77): error C2065: 'my_perl' :
undeclared identifier

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. Please note that, we have now
defined PERL_NO_GET_CONTEXT macro before including the Perl headers in
plperl.h file which means any plperl function calling Perl APIs should
try to fetch the perl interpreter context variable 'my_perl' at the
start of the function using dTHX but, we were not doing that for
'setlocale_perl', 'hstore_to_plperl' and 'plperl_to_hstore' functions.
I have corrected this and attached is the new version of patch.

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.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Sharma <ashu.coek88@gmail.com> 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
compatibleare in perl.c  Both are combined for the output of perl -V  However, this string will be embedded in any
sharedperl 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 ...

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

(BTW, you never did tell us exactly what defines you're getting
out of Perl's flags on the problem installation.)
        regards, tom lane

On Fri, Jul 28, 2017 at 7:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> 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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> On Fri, Jul 28, 2017 at 7:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.

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

OK, thanks.  I've pushed the XSUB/dTHX patch after another round of
code-reading and some minor comment improvements; we'll soon see
what the buildfarm thinks of it.  In the meantime I'll work on these
two patches.

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

Uh-huh.  So the issue is indeed that they're injecting PERL_IMPLICIT_SYS
via a command-line #define rather than putting it into perl's config.h,
and that results in a change in the apparent size of the PerlInterp
struct (because of IMem and friends).  We'd expected as much, but it's
good to have clear confirmation.
        regards, tom lane

On Fri, Jul 28, 2017 at 10:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
>> On Fri, Jul 28, 2017 at 7:22 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> 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.
>
>> 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.
>
> OK, thanks.  I've pushed the XSUB/dTHX patch after another round of
> code-reading and some minor comment improvements; we'll soon see
> what the buildfarm thinks of it.  In the meantime I'll work on these
> two patches.

Sure, Thanks a lot.

>
>>> (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
>
> Uh-huh.  So the issue is indeed that they're injecting PERL_IMPLICIT_SYS
> via a command-line #define rather than putting it into perl's config.h,
> and that results in a change in the apparent size of the PerlInterp
> struct (because of IMem and friends).

Yes, That's right. We would have seen different result if the
PERL_IMPLICIT_SYS would not have been defined globally.

We'd expected as much, but it's
> good to have clear confirmation.

That's right. It is always good to have a clear confirmation. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> On Fri, Jul 28, 2017 at 10:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Uh-huh.  So the issue is indeed that they're injecting PERL_IMPLICIT_SYS
>> via a command-line #define rather than putting it into perl's config.h,
>> and that results in a change in the apparent size of the PerlInterp
>> struct (because of IMem and friends).

> Yes, That's right. We would have seen different result if the
> PERL_IMPLICIT_SYS would not have been defined globally.

I've pushed the changes to the build scripts now.  I had to revise the
Mkvcbuild.pm part a bit, because you'd forgotten to propagate the extra
defines into hstore_plperl.  So that code is untested; please confirm
that HEAD works in your problem environment now.

I notice that Mkvcbuild.pm is artificially injecting a define for
PLPERL_HAVE_UID_GID.  I strongly suspect that that was a hack meant
to work around the lack of this mechanism, and that we could now
get rid of it or clean it up.  But there's no evidence in the commit
log about the reason for it --- it seems to go back to the original
addition of MSVC support.  Anybody remember?
        regards, tom lane

On Sat, Jul 29, 2017 at 12:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
>> On Fri, Jul 28, 2017 at 10:05 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Uh-huh.  So the issue is indeed that they're injecting PERL_IMPLICIT_SYS
>>> via a command-line #define rather than putting it into perl's config.h,
>>> and that results in a change in the apparent size of the PerlInterp
>>> struct (because of IMem and friends).
>
>> Yes, That's right. We would have seen different result if the
>> PERL_IMPLICIT_SYS would not have been defined globally.
>
> I've pushed the changes to the build scripts now.  I had to revise the
> Mkvcbuild.pm part a bit, because you'd forgotten to propagate the extra
> defines into hstore_plperl.

Thanks for that.

So that code is untested; please confirm
> that HEAD works in your problem environment now.
>

I quickly ran the some basic test-cases on my Windows machine (machine
where i have been regenerating the issue) and they are all passing.
Also, all the automated test-cases for contrib module hstore_plperl
are passing.

> I notice that Mkvcbuild.pm is artificially injecting a define for
> PLPERL_HAVE_UID_GID.  I strongly suspect that that was a hack meant
> to work around the lack of this mechanism, and that we could now
> get rid of it or clean it up.  But there's no evidence in the commit
> log about the reason for it --- it seems to go back to the original
> addition of MSVC support.  Anybody remember?
>
>                         regards, tom lane

Christoph Berg <myon@debian.org> writes:
> The plperl segfault on Debian's kfreebsd port I reported back in 2013
> is also still present:
> https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de
>
https://buildd.debian.org/status/fetch.php?pkg=postgresql-10&arch=kfreebsd-amd64&ver=10~beta2-1&stamp=1499947011&raw=0

So it'd be interesting to know if it's any better with HEAD ...
        regards, tom lane

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> I quickly ran the some basic test-cases on my Windows machine (machine
> where i have been regenerating the issue) and they are all passing.
> Also, all the automated test-cases for contrib module hstore_plperl
> are passing.

Cool, thanks.  I hope people will set up some buildfarm machines with
the formerly-broken configurations.
        regards, tom lane

Re: Tom Lane 2017-07-28 <3254.1501276475@sss.pgh.pa.us>
> Christoph Berg <myon@debian.org> writes:
> > The plperl segfault on Debian's kfreebsd port I reported back in 2013
> > is also still present:
> > https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de
> >
https://buildd.debian.org/status/fetch.php?pkg=postgresql-10&arch=kfreebsd-amd64&ver=10~beta2-1&stamp=1499947011&raw=0
> 
> So it'd be interesting to know if it's any better with HEAD ...

Unfortunately not:

============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
============== installing plperl                      ==============
server closed the connection unexpectedly       This probably means the server terminated abnormally       before or
whileprocessing the request.
 
connection to server was lost

The only interesting line in log/postmaster.log is a log_line_prefix-less
Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
... which is unchanged from the beta2 output.

Christoph

Christoph Berg <myon@debian.org> writes:
> Re: Tom Lane 2017-07-28 <3254.1501276475@sss.pgh.pa.us>
>> Christoph Berg <myon@debian.org> writes:
>>> The plperl segfault on Debian's kfreebsd port I reported back in 2013
>>> is also still present:

>> So it'd be interesting to know if it's any better with HEAD ...

> Unfortunately not:
> The only interesting line in log/postmaster.log is a log_line_prefix-less
> Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
> ... which is unchanged from the beta2 output.

Well, that's quite interesting, because it implies that this is indeed
the same type of problem.  I wonder why the patch didn't fix it?
        regards, tom lane

Hi Christoph,

On Mon, Jul 31, 2017 at 2:44 AM, Christoph Berg <myon@debian.org> wrote:
> Re: Tom Lane 2017-07-28 <3254.1501276475@sss.pgh.pa.us>
>> Christoph Berg <myon@debian.org> writes:
>> > The plperl segfault on Debian's kfreebsd port I reported back in 2013
>> > is also still present:
>> > https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de
>> >
https://buildd.debian.org/status/fetch.php?pkg=postgresql-10&arch=kfreebsd-amd64&ver=10~beta2-1&stamp=1499947011&raw=0
>>
>> So it'd be interesting to know if it's any better with HEAD ...
>
> Unfortunately not:
>
> ============== creating database "pl_regression"      ==============
> CREATE DATABASE
> ALTER DATABASE
> ============== installing plperl                      ==============
> server closed the connection unexpectedly
>         This probably means the server terminated abnormally
>         before or while processing the request.
> connection to server was lost
>
> The only interesting line in log/postmaster.log is a log_line_prefix-less
> Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
> ... which is unchanged from the beta2 output.


I am not able to reproduce this issue on my Windows or Linux box. Have
you deleted Util.c and SPI.c files before starting with the build? The
point is, these files are generated during build time and if they
already exist then i think. they are not re generated. I would suggest
to delete both the .c files and rebuild your source and then give a
try.

Here are the results i got on Windows and Linux respectively on HEAD,

On Windows:

C:\Users\ashu\git-clone-postgresql\postgresql\src\tools\msvc>vcregress.bat
PLCHECK
============================================================
Checking plperl
(using postmaster on localhost, default port)
============== dropping database "pl_regression"      ==============
NOTICE:  database "pl_regression" does not exist, skipping
DROP DATABASE
============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
============== installing plperl                      ==============
CREATE EXTENSION
============== installing plperlu                     ==============
CREATE EXTENSION
============== running regression test queries        ==============
test plperl                   ... ok
test plperl_lc                ... ok
test plperl_trigger           ... ok
test plperl_shared            ... ok
test plperl_elog              ... ok
test plperl_util              ... ok
test plperl_init              ... ok
test plperlu                  ... ok
test plperl_array             ... ok
test plperl_plperlu           ... ok

======================All 10 tests passed.
======================

On Linux:


LD_LIBRARY_PATH="/home/ashu/git-clone-postgresql/postgresql/tmp_install/home/ashu/git-clone-postgresql/postgresql/TMP/postgres/lib"
../../../src/test/regress/pg_regress --temp-instance=./tmp_check
--inputdir=. --bindir=     --dbname=pl_regression
--load-extension=plperl  --load-extension=plperlu plperl plperl_lc
plperl_trigger plperl_shared plperl_elog plperl_util plperl_init
plperlu plperl_array
============== creating temporary instance            ==============
============== initializing database system           ==============
============== starting postmaster                    ==============
running on port 50848 with PID 18140
============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
============== installing plperl                      ==============
CREATE EXTENSION
============== installing plperlu                     ==============
CREATE EXTENSION
============== running regression test queries        ==============
test plperl                   ... ok
test plperl_lc                ... ok
test plperl_trigger           ... ok
test plperl_shared            ... ok
test plperl_elog              ... ok
test plperl_util              ... ok
test plperl_init              ... ok
test plperlu                  ... ok
test plperl_array             ... ok
============== shutting down postmaster               ==============
============== removing temporary instance            ==============

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

Hi Christoph,

On Mon, Jul 31, 2017 at 9:18 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> Hi Christoph,
>
> On Mon, Jul 31, 2017 at 2:44 AM, Christoph Berg <myon@debian.org> wrote:
>> Re: Tom Lane 2017-07-28 <3254.1501276475@sss.pgh.pa.us>
>>> Christoph Berg <myon@debian.org> writes:
>>> > The plperl segfault on Debian's kfreebsd port I reported back in 2013
>>> > is also still present:
>>> > https://www.postgresql.org/message-id/20130515064201.GC704%40msgid.df7cb.de
>>> >
https://buildd.debian.org/status/fetch.php?pkg=postgresql-10&arch=kfreebsd-amd64&ver=10~beta2-1&stamp=1499947011&raw=0
>>>
>>> So it'd be interesting to know if it's any better with HEAD ...
>>
>> Unfortunately not:
>>
>> ============== creating database "pl_regression"      ==============
>> CREATE DATABASE
>> ALTER DATABASE
>> ============== installing plperl                      ==============
>> server closed the connection unexpectedly
>>         This probably means the server terminated abnormally
>>         before or while processing the request.
>> connection to server was lost
>>
>> The only interesting line in log/postmaster.log is a log_line_prefix-less
>> Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
>> ... which is unchanged from the beta2 output.
>

It seems like you are observing this crash on kFreeBSD OS. Well, me or
any of my colleague is not having this OS hence, i can't comment on
that. But, we do have Ubuntu OS (another Debian based OS) and I am not
seeing any server crash here as well,

edb@ubuntu:~/PGsources/postgresql/src/pl/plperl$ make check
make -C ../../../src/test/regress pg_regress
.....
/bin/mkdir -p '/home/edb/PGsources/postgresql'/tmp_install/log
make -C '../../..'
DESTDIR='/home/edb/PGsources/postgresql'/tmp_install install
>'/home/edb/PGsources/postgresql'/tmp_install/log/install.log 2>&1
PATH="/home/edb/PGsources/postgresql/tmp_install/home/edb/PGsources/postgresql/inst/bin:$PATH"
LD_LIBRARY_PATH="/home/edb/PGsources/postgresql/tmp_install/home/edb/PGsources/postgresql/inst/lib:$LD_LIBRARY_PATH"
../../../src/test/regress/pg_regress --temp-instance=./tmp_check
--inputdir=. --bindir=     --dbname=pl_regression
--load-extension=plperl  --load-extension=plperlu plperl plperl_lc
plperl_trigger plperl_shared plperl_elog plperl_util plperl_init
plperlu plperl_array plperl_plperlu
============== creating temporary instance            ==============
============== initializing database system           ==============
============== starting postmaster                    ==============
running on port 50848 with PID 43441
============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
============== installing plperl                      ==============
CREATE EXTENSION
============== installing plperlu                     ==============
CREATE EXTENSION
============== running regression test queries        ==============
test plperl                   ... ok
test plperl_lc                ... ok
test plperl_trigger           ... ok
test plperl_shared            ... ok
test plperl_elog              ... ok
test plperl_util              ... ok
test plperl_init              ... ok
test plperlu                  ... ok
test plperl_array             ... ok
test plperl_plperlu           ... ok
============== shutting down postmaster               ==============
============== removing temporary instance            ==============


As i mentioned in my earlier email, could you please delete the Utilc.
and SPI.c files from src/pl/plperl directory, rebuild the source code
and then let us know the results. Thanks.

>
> I am not able to reproduce this issue on my Windows or Linux box. Have
> you deleted Util.c and SPI.c files before starting with the build? The
> point is, these files are generated during build time and if they
> already exist then i think. they are not re generated. I would suggest
> to delete both the .c files and rebuild your source and then give a
> try.
>
> Here are the results i got on Windows and Linux respectively on HEAD,
>
> On Windows:
>
> C:\Users\ashu\git-clone-postgresql\postgresql\src\tools\msvc>vcregress.bat
> PLCHECK
> ============================================================
> Checking plperl
> (using postmaster on localhost, default port)
> ============== dropping database "pl_regression"      ==============
> NOTICE:  database "pl_regression" does not exist, skipping
> DROP DATABASE
> ============== creating database "pl_regression"      ==============
> CREATE DATABASE
> ALTER DATABASE
> ============== installing plperl                      ==============
> CREATE EXTENSION
> ============== installing plperlu                     ==============
> CREATE EXTENSION
> ============== running regression test queries        ==============
> test plperl                   ... ok
> test plperl_lc                ... ok
> test plperl_trigger           ... ok
> test plperl_shared            ... ok
> test plperl_elog              ... ok
> test plperl_util              ... ok
> test plperl_init              ... ok
> test plperlu                  ... ok
> test plperl_array             ... ok
> test plperl_plperlu           ... ok
>
> ======================
>  All 10 tests passed.
> ======================
>
> On Linux:
>
>
LD_LIBRARY_PATH="/home/ashu/git-clone-postgresql/postgresql/tmp_install/home/ashu/git-clone-postgresql/postgresql/TMP/postgres/lib"
> ../../../src/test/regress/pg_regress --temp-instance=./tmp_check
> --inputdir=. --bindir=     --dbname=pl_regression
> --load-extension=plperl  --load-extension=plperlu plperl plperl_lc
> plperl_trigger plperl_shared plperl_elog plperl_util plperl_init
> plperlu plperl_array
> ============== creating temporary instance            ==============
> ============== initializing database system           ==============
> ============== starting postmaster                    ==============
> running on port 50848 with PID 18140
> ============== creating database "pl_regression"      ==============
> CREATE DATABASE
> ALTER DATABASE
> ============== installing plperl                      ==============
> CREATE EXTENSION
> ============== installing plperlu                     ==============
> CREATE EXTENSION
> ============== running regression test queries        ==============
> test plperl                   ... ok
> test plperl_lc                ... ok
> test plperl_trigger           ... ok
> test plperl_shared            ... ok
> test plperl_elog              ... ok
> test plperl_util              ... ok
> test plperl_init              ... ok
> test plperlu                  ... ok
> test plperl_array             ... ok
> ============== shutting down postmaster               ==============
> ============== removing temporary instance            ==============
>
> --
> With Regards,
> Ashutosh Sharma
> EnterpriseDB:http://www.enterprisedb.com

Re: Ashutosh Sharma 2017-07-31 <CAE9k0PnzYxyKHuwJonUEDt2xunPUc8VUVPWsd0BScRd3u+j8_A@mail.gmail.com>
> > The only interesting line in log/postmaster.log is a log_line_prefix-less
> > Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
> > ... which is unchanged from the beta2 output.
> 
> 
> I am not able to reproduce this issue on my Windows or Linux box. Have
> you deleted Util.c and SPI.c files before starting with the build?

That was from a git checkout which didn't contain the files.
Retrying anyway:

[127] 10:28 myon@experimental_k-a-dchroot.falla:~/po/po/src/pl/plperl $ make clean
rm -f plperl.so   libplperl.a  libplperl.pc
rm -f SPI.c Util.c plperl.o SPI.o Util.o  perlchunks.h plperl_opmask.h
rm -rf results/ regression.diffs regression.out tmp_check/ tmp_check_iso/ log/ output_iso/
[0] 10:29 myon@experimental_k-a-dchroot.falla:~/po/po/src/pl/plperl $ make
'/usr/bin/perl' ./text2macro.pl --strip='^(\#.*|\s*)$' plc_perlboot.pl plc_trusted.pl > perlchunks.h
'/usr/bin/perl' plperl_opmask.pl plperl_opmask.h
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security-fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -fPIC -I. -I. -I../../../src/include
-D_GNU_SOURCE  -I/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE  -c -o plperl.o plperl.c
 
'/usr/bin/perl' /usr/share/perl/5.26/ExtUtils/xsubpp -typemap /usr/share/perl/5.26/ExtUtils/typemap SPI.xs >SPI.c
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security-fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -fPIC -I. -I. -I../../../src/include
-D_GNU_SOURCE  -I/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE  -c -o SPI.o SPI.c
 
'/usr/bin/perl' /usr/share/perl/5.26/ExtUtils/xsubpp -typemap /usr/share/perl/5.26/ExtUtils/typemap Util.xs >Util.c
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security-fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -fPIC -I. -I. -I../../../src/include
-D_GNU_SOURCE  -I/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE  -c -o Util.o Util.c
 
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute
-Wformat-security-fno-strict-aliasing -fwrapv -fexcess-precision=standard -O2 -fPIC -shared -o plperl.so plperl.o SPI.o
Util.o -L../../../src/port -L../../../src/common -Wl,--as-needed
-Wl,-rpath,'/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE',--enable-new-dtags -fstack-protector-strong -L/usr/local/lib
-L/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE-lperl -ldl -lm -lpthread -lc -lcrypt 
 
[0] 10:29 myon@experimental_k-a-dchroot.falla:~/po/po/src/pl/plperl $ make check
make -C ../../../src/test/regress pg_regress
make[1]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/test/regress“ wird betreten
make -C ../../../src/port all
make[2]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/port“ wird betreten
make -C ../backend submake-errcodes
make[3]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/backend“ wird betreten
make[3]: Für das Ziel „submake-errcodes“ ist nichts zu tun.
make[3]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/backend“ wird verlassen
make[2]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/port“ wird verlassen
make -C ../../../src/common all
make[2]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/common“ wird betreten
make -C ../backend submake-errcodes
make[3]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/backend“ wird betreten
make[3]: Für das Ziel „submake-errcodes“ ist nichts zu tun.
make[3]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/backend“ wird verlassen
make[2]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/common“ wird verlassen
make[1]: Verzeichnis „/home/myon/postgresql-10/postgresql-10-10~beta2/src/test/regress“ wird verlassen
rm -rf '/home/myon/postgresql-10/postgresql-10-10~beta2'/tmp_install
/bin/mkdir -p '/home/myon/postgresql-10/postgresql-10-10~beta2'/tmp_install/log
make -C '../../..' DESTDIR='/home/myon/postgresql-10/postgresql-10-10~beta2'/tmp_install install
>'/home/myon/postgresql-10/postgresql-10-10~beta2'/tmp_install/log/install.log2>&1
 
PATH="/home/myon/postgresql-10/postgresql-10-10~beta2/tmp_install/usr/local/pgsql/bin:$PATH"
LD_LIBRARY_PATH="/home/myon/postgresql-10/postgresql-10-10~beta2/tmp_install/usr/local/pgsql/lib"
../../../src/test/regress/pg_regress--temp-instance=./tmp_check --inputdir=. --bindir=     --dbname=pl_regression
--load-extension=plperl --load-extension=plperlu plperl plperl_lc plperl_trigger plperl_shared plperl_elog plperl_util
plperl_initplperlu plperl_array plperl_plperlu
 
============== creating temporary instance            ==============
============== initializing database system           ==============
============== starting postmaster                    ==============
running on port 50848 with PID 17713
============== creating database "pl_regression"      ==============
CREATE DATABASE
ALTER DATABASE
============== installing plperl                      ==============
server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
connection to server was lost
command failed: "psql" -X -c "CREATE EXTENSION IF NOT EXISTS \"plperl\"" "pl_regression"
GNUmakefile:108: die Regel für Ziel „check“ scheiterte
make: *** [check] Fehler 2
[2] 10:29 myon@experimental_k-a-dchroot.falla:~/po/po/src/pl/plperl $ cat log/postmaster.log 
2017-07-31 10:29:54.995 CEST [17713] LOG:  listening on Unix socket "/tmp/pg_regress-JaWXuT/.s.PGSQL.50848"
2017-07-31 10:29:55.015 CEST [17716] LOG:  database system was shut down at 2017-07-31 10:29:54 CEST
2017-07-31 10:29:55.023 CEST [17713] LOG:  database system is ready to accept connections
2017-07-31 10:29:56.068 CEST [17717] LOG:  checkpoint starting: immediate force wait flush-all
2017-07-31 10:29:56.071 CEST [17717] LOG:  checkpoint complete: wrote 3 buffers (0.0%); 0 WAL file(s) added, 0 removed,
0recycled; write=0.000 s, sync=0.000 s, total=0.002 s; sync files=0, longest=0.000 s, average=0.000 s; distance=1 kB,
estimate=1kB
 
2017-07-31 10:29:56.303 CEST [17717] LOG:  checkpoint starting: immediate force wait
2017-07-31 10:29:56.305 CEST [17717] LOG:  checkpoint complete: wrote 0 buffers (0.0%); 0 WAL file(s) added, 0 removed,
0recycled; write=0.000 s, sync=0.000 s, total=0.001 s; sync files=0, longest=0.000 s, average=0.000 s; distance=0 kB,
estimate=1kB
 
Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
2017-07-31 10:29:56.478 CEST [17713] LOG:  received fast shutdown request
2017-07-31 10:29:56.478 CEST [17713] LOG:  aborting any active transactions
2017-07-31 10:29:56.480 CEST [17713] LOG:  worker process: logical replication launcher (PID 17722) exited with exit
code1
 
2017-07-31 10:29:56.481 CEST [17717] LOG:  shutting down
2017-07-31 10:29:56.481 CEST [17717] LOG:  checkpoint starting: shutdown immediate
2017-07-31 10:29:56.484 CEST [17717] LOG:  checkpoint complete: wrote 19 buffers (0.1%); 0 WAL file(s) added, 0
removed,0 recycled; write=0.001 s, sync=0.000 s, total=0.003 s; sync files=0, longest=0.000 s, average=0.000 s;
distance=47kB, estimate=47 kB
 
2017-07-31 10:29:56.494 CEST [17713] LOG:  database system is shut down


Christoph

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Christoph Berg <myon@debian.org> writes:
>>> The only interesting line in log/postmaster.log is a log_line_prefix-less
>>> Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)

Can we see the Perl-related output from configure, particularly the new
lines about CFLAGS?
        regards, tom lane

Re: Tom Lane 2017-07-31 <30582.1501508356@sss.pgh.pa.us>
> Christoph Berg <myon@debian.org> writes:
> >>> The only interesting line in log/postmaster.log is a log_line_prefix-less
> >>> Util.c: loadable library and perl binaries are mismatched (got handshake key 0xd500080, needed 0xd600080)
> 
> Can we see the Perl-related output from configure, particularly the new
> lines about CFLAGS?

$ ./configure --with-perl

checking whether to build Perl modules... yes

checking for perl... /usr/bin/perl
configure: using perl 5.26.0
checking for Perl archlibexp... /usr/lib/x86_64-kfreebsd-gnu/perl/5.26
checking for Perl privlibexp... /usr/share/perl/5.26
checking for Perl useshrplib... true
checking for CFLAGS recommended by Perl... -D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fwrapv -fno-strict-aliasing -pipe
-I/usr/local/include-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
 
checking for CFLAGS to compile embedded Perl... -DDEBIAN
checking for flags to link embedded Perl...   -fstack-protector-strong -L/usr/local/lib
-L/usr/lib/x86_64-kfreebsd-gnu/perl/5.26/CORE-lperl -ldl -lm -lpthread -lc -lcrypt
 

checking for perl.h... yes
checking for libperl... yes

Christoph

On Tue, Aug 8, 2017 at 8:37 AM, Sandeep Thakkar
<sandeep.thakkar@enterprisedb.com> wrote:
> An update from beta3 build: We are no longer seeing this issue (handshake
> failure) on Windows 64bit, but on Windows 32bit it still persists.

Hmm, maybe you should've reported it sooner, so we could've tried to
fix this before beta3 went out.

What was the exact message you saw, including the hex values?

Is the Perl you were building against for plperl the same Perl that
was being used for the build itself?

Do you have the portion of the build log where src/pl/plperl was being built?

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

On Tue, Aug 8, 2017 at 12:15 PM, Sandeep Thakkar
<sandeep.thakkar@enterprisedb.com> wrote:
> I copied and pasted that portion of the build log into file build.log
> (attached) for Windows 32bit and Windows 64bit.

Can you also share the output of 'perl -V' on each system?

Comparing the 32-bit log and the 64-bit log, I see the following differences:

32-bit has extra options /IC:\pgBuild32\uuid\include /Oy- /analyze- /D
_USE_32BIT_TIME_T
64-bit has extra options /D WIN64 /D CONSERVATIVE /D USE_SITECUSTOMIZE
Both builds have several copies of /IC:\pgBuild32\include or
/IC:\pgBuild64\include, but the 64-bit build has an extra one

(I wrote that command on Linux, might need different quoting to work
on Windows.)

I'm suspicious about _USE_32BIT_TIME_T.  The contents of a
PerlInterpreter are defined in Perl's intrpvar.h, and one of those
lines is:

PERLVAR(I, basetime,    Time_t)         /* $^T */

Now, Time_t is defined as time_t elsewhere in the Perl headers, so if
the plperl build and the Perl interpreter build had different ideas
about whether that flag was set, the interpreter sizes would be
different.  Unfortunately for this theory, if I'm interpreting the
screenshot you posted correctly, the sizes are different by exactly 16
bytes, and I can't see how a difference in the size of time_t could
account for more than 8 bytes (4 bytes of actual size change + 4 bytes
of padding).

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

On Thu, Aug 10, 2017 at 1:55 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Tue, Aug 8, 2017 at 12:15 PM, Sandeep Thakkar
> <sandeep.thakkar@enterprisedb.com> wrote:
>> I copied and pasted that portion of the build log into file build.log
>> (attached) for Windows 32bit and Windows 64bit.
>
> Can you also share the output of 'perl -V' on each system?
>
> Comparing the 32-bit log and the 64-bit log, I see the following differences:
>
> 32-bit has extra options /IC:\pgBuild32\uuid\include /Oy- /analyze- /D
> _USE_32BIT_TIME_T
> 64-bit has extra options /D WIN64 /D CONSERVATIVE /D USE_SITECUSTOMIZE
> Both builds have several copies of /IC:\pgBuild32\include or
> /IC:\pgBuild64\include, but the 64-bit build has an extra one
>
> (I wrote that command on Linux, might need different quoting to work
> on Windows.)
>
> I'm suspicious about _USE_32BIT_TIME_T.  The contents of a
> PerlInterpreter are defined in Perl's intrpvar.h, and one of those
> lines is:
>
> PERLVAR(I, basetime,    Time_t)         /* $^T */
>
> Now, Time_t is defined as time_t elsewhere in the Perl headers, so if
> the plperl build and the Perl interpreter build had different ideas
> about whether that flag was set, the interpreter sizes would be
> different.  Unfortunately for this theory, if I'm interpreting the
> screenshot you posted correctly, the sizes are different by exactly 16
> bytes, and I can't see how a difference in the size of time_t could
> account for more than 8 bytes (4 bytes of actual size change + 4 bytes
> of padding).
>

Okay. I too had a look into this issue and my findings are as follows,

The size of PerlInterpreter structure on plperl and perl module are
not the same. The size of PerlInterpreter on plperl is 2744 bytes
whereas on perl it is 2760 bytes i.e. there is the difference of 16
bytes and therefore the handshaking key generated in the two modules
are not the same resulting in a binary mismatch error. To analyse this
problem, I generated the preprocessed output of header file 'perl.h'
on plperl and perl modules and then filtered the contents of "struct
interpreter" from the preprocessed output files and  compared them
using some diff tool. The diff report is attached with this email.

As per the diff report, incase of  plperl module,  the structure
Stat_t is getting mapped to "_stat32i64" whereas incase of perl
module, the same structure is getting mapped to "_stat64" and this is
happening for couple of variables (statbuf & statcache) in intrpvar.h
file. However, if the preprocessor option '_USE_32BIT_TIME_T' is
explicitly passed to the VC compiler when compiling perl source, there
is no diff observed as in both the cases, 'Stat_t ' is gets mapped to
"stat32i64". So, now the question is, why ''_USE_32BIT_TIME_T'  flag
is not implicitly defined when compiling perl source on 32 bit
platform but for postgresql/plperl build it is being defined
implicitly on 32bit platform. I just got few hints from the Makefile,
in perl source code, where is has been mentioned that  the modern
Visual C compiler (VC++ 8.0 onwards) changes time_t from 32-bit to
64-bit, even in 32-bit mode hence, ''_USE_32BIT_TIME_T'   is net being
added to $Config{ccflags}. Here, is what i could see in
GNUMakefile/Makefile.mk in perl source code.

# In VS 2005 (VC++ 8.0) Microsoft changes time_t from 32-bit to
# 64-bit, even in 32-bit mode.  It also provides the _USE_32BIT_TIME_T
# preprocessor option to revert back to the old functionality for
# backward compatibility.  We define this symbol here for older 32-bit
# compilers only (which aren't using it at all) for the sole purpose
# of getting it into $Config{ccflags}.  That way if someone builds
# Perl itself with e.g. VC6 but later installs an XS module using VC8
# the time_t types will still be compatible.

ifeq ($(WIN64),undef)
ifeq ((PREMSVC80),define)
BUILDOPT += -D_USE_32BIT_TIME_T
endif
endif

So, the question is should we allow the preprocessor option
'_USE_32BIT_TIME_T' to be defined implicitly or not in postgresql
provided we are using Visual C compiler version > 8.0. I think this
should make plperl compatible with perl binaries.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Aug 10, 2017 at 8:30 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> So, the question is should we allow the preprocessor option
> '_USE_32BIT_TIME_T' to be defined implicitly or not in postgresql
> provided we are using Visual C compiler version > 8.0. I think this
> should make plperl compatible with perl binaries.

We've got this code in MSBuildProject.pm and VCBuildProject.pm:
       # We have to use this flag on 32 bit targets because the 32bit perls       # are built with it and sometimes
crashif we don't.       my $use_32bit_time_t =         $self->{platform} eq 'Win32' ? '_USE_32BIT_TIME_T;' : '';
 

Based on the discussion upthread, I think we now know that this was
not really the right approach.  IIUC, a given 32-bit Perl might've
been built with _USE_32BIT_TIME_T, or not; in fact, given the comments
you found on the Perl side, it's more than likely that most modern
32-bit Perls are NOT build with this option.  What we actually need to
do here is use _USE_32BIT_TIME_T if and only if it was used when Perl
was built (otherwise we'll end up with a different interpreter size).

The trouble with that is that _USE_32BIT_TIME_T also affects how
PostgreSQL code compiles.  Apparently, given that according to Perl
this was changed by Microsoft in 2005, we're forcing the Microsoft
compilers into a non-default backward compatibility mode by defining
this symbol, and that affects how our entire code base compiles -- and
it just so happens that the result is compatible with older Perl
builds that used _USE_32BIT_TIME_T and not compatible with newer ones
that don't.

Maybe we need to make _USE_32BIT_TIME_T a compile-time configuration
option on Windows, and then cross-check that our setting is compatible
with Perl's setting.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Aug 10, 2017 at 8:30 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>> So, the question is should we allow the preprocessor option
>> '_USE_32BIT_TIME_T' to be defined implicitly or not in postgresql
>> provided we are using Visual C compiler version > 8.0. I think this
>> should make plperl compatible with perl binaries.

> We've got this code in MSBuildProject.pm and VCBuildProject.pm:

>         # We have to use this flag on 32 bit targets because the 32bit perls
>         # are built with it and sometimes crash if we don't.
>         my $use_32bit_time_t =
>           $self->{platform} eq 'Win32' ? '_USE_32BIT_TIME_T;' : '';

> Based on the discussion upthread, I think we now know that this was
> not really the right approach.

Yeah ... however, if that's there, then there's something wrong with
Ashutosh's explanation, because that means we *are* building with
_USE_32BIT_TIME_T in 32-bit builds.  It's just getting there in a
roundabout way.  (Or, alternatively, this code is somehow not doing
anything at all.)

> The trouble with that is that _USE_32BIT_TIME_T also affects how
> PostgreSQL code compiles.

Really?  We try to avoid touching "time_t" at all in most of the code.
I bet that we could drop the above-cited code, and compile only plperl
with _USE_32BIT_TIME_T, taken (if present) from the Perl flags, and
it'd be fine.  At least, that's my first instinct for what to try.
        regards, tom lane

On Thu, Aug 10, 2017 at 10:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Yeah ... however, if that's there, then there's something wrong with
> Ashutosh's explanation, because that means we *are* building with
> _USE_32BIT_TIME_T in 32-bit builds.  It's just getting there in a
> roundabout way.  (Or, alternatively, this code is somehow not doing
> anything at all.)

I don't follow.

>> The trouble with that is that _USE_32BIT_TIME_T also affects how
>> PostgreSQL code compiles.
>
> Really?  We try to avoid touching "time_t" at all in most of the code.
> I bet that we could drop the above-cited code, and compile only plperl
> with _USE_32BIT_TIME_T, taken (if present) from the Perl flags, and
> it'd be fine.  At least, that's my first instinct for what to try.

Oh.  Well, if that's an OK thing to do, then sure, wfm.  I guess we've
got pg_time_t plastered all over the backend but that's not actually
time_t under the hood, so it's fine.  I do see time_t being used in
frontend code, but that won't matter for this.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Aug 10, 2017 at 10:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah ... however, if that's there, then there's something wrong with
>> Ashutosh's explanation, because that means we *are* building with
>> _USE_32BIT_TIME_T in 32-bit builds.  It's just getting there in a
>> roundabout way.  (Or, alternatively, this code is somehow not doing
>> anything at all.)

> I don't follow.

The stanzas you pointed to in the MSVC build scripts should mean that
a 32-bit PG build is using _USE_32BIT_TIME_T, no?  And Ashutosh stated
that he saw _USE_32BIT_TIME_T in "perl -V" output.  So how are they
not ending up compatible?

Now, if that statement was wrong and his 32-bit Perl actually *isn't*
built with _USE_32BIT_TIME_T, then this is clearly what's causing the
problem.

>> Really?  We try to avoid touching "time_t" at all in most of the code.
>> I bet that we could drop the above-cited code, and compile only plperl
>> with _USE_32BIT_TIME_T, taken (if present) from the Perl flags, and
>> it'd be fine.  At least, that's my first instinct for what to try.

> Oh.  Well, if that's an OK thing to do, then sure, wfm.  I guess we've
> got pg_time_t plastered all over the backend but that's not actually
> time_t under the hood, so it's fine.  I do see time_t being used in
> frontend code, but that won't matter for this.

Yeah.  I think this should work as long as plperl itself doesn't use
time_t, or at least doesn't exchange time_t with any other part of the
system, and since we don't use that type in any common APIs that seems
like an OK assumption.
        regards, tom lane

On Thu, Aug 10, 2017 at 8:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Thu, Aug 10, 2017 at 8:30 AM, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>>> So, the question is should we allow the preprocessor option
>>> '_USE_32BIT_TIME_T' to be defined implicitly or not in postgresql
>>> provided we are using Visual C compiler version > 8.0. I think this
>>> should make plperl compatible with perl binaries.
>
>> We've got this code in MSBuildProject.pm and VCBuildProject.pm:
>
>>         # We have to use this flag on 32 bit targets because the 32bit perls
>>         # are built with it and sometimes crash if we don't.
>>         my $use_32bit_time_t =
>>           $self->{platform} eq 'Win32' ? '_USE_32BIT_TIME_T;' : '';
>
>> Based on the discussion upthread, I think we now know that this was
>> not really the right approach.
>
> Yeah ... however, if that's there, then there's something wrong with
> Ashutosh's explanation, because that means we *are* building with
> _USE_32BIT_TIME_T in 32-bit builds.  It's just getting there in a
> roundabout way.  (Or, alternatively, this code is somehow not doing
> anything at all.)

I am extremely sorry if i have communicated the things wrongly, what i
meant was we are always considering _USE_32BIT_TIME_T flag to build
plperl module on Windows 32-bit platform but unfortunately that is not
being considered/defined in perl code in case we use VC++ compiler
version greater than 8.0. and that's the reason for the binary
mismatch error on 32 bit platform.

Here, is what has been mentioned in 'win32/GNUMakefile' in perl source
for version 5.24.1

# In VS 2005 (VC++ 8.0) Microsoft changes time_t from 32-bit to
# 64-bit, even in 32-bit mode.  It also provides the _USE_32BIT_TIME_T
# preprocessor option to revert back to the old functionality for
# backward compatibility.  We define this symbol here for older 32-bit
# compilers only (which aren't using it at all) for the sole purpose
# of getting it into $Config{ccflags}.  That way if someone builds
# Perl itself with e.g. VC6 but later installs an XS module using VC8
# the time_t types will still be compatible.
ifeq ($(WIN64),undef)
ifeq ((PREMSVC80),define)
BUILDOPT    += -D_USE_32BIT_TIME_T
endif
endif


>
>> The trouble with that is that _USE_32BIT_TIME_T also affects how
>> PostgreSQL code compiles.
>
> Really?  We try to avoid touching "time_t" at all in most of the code.
> I bet that we could drop the above-cited code, and compile only plperl
> with _USE_32BIT_TIME_T, taken (if present) from the Perl flags, and
> it'd be fine.  At least, that's my first instinct for what to try.
>

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> On Thu, Aug 10, 2017 at 8:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah ... however, if that's there, then there's something wrong with
>> Ashutosh's explanation, because that means we *are* building with
>> _USE_32BIT_TIME_T in 32-bit builds.  It's just getting there in a
>> roundabout way.  (Or, alternatively, this code is somehow not doing
>> anything at all.)

> I am extremely sorry if i have communicated the things wrongly, what i
> meant was we are always considering _USE_32BIT_TIME_T flag to build
> plperl module on Windows 32-bit platform but unfortunately that is not
> being considered/defined in perl code in case we use VC++ compiler
> version greater than 8.0. and that's the reason for the binary
> mismatch error on 32 bit platform.

Got it.  So in short, it seems like the attached patch ought to fix it
for MSVC builds.  (We'd also need to teach PGAC_CHECK_PERL_EMBED_CCFLAGS
to let _USE_32BIT_TIME_T through on Windows, but let's confirm the theory
first.)

            regards, tom lane

diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index d7638b4..27329f9 100644
*** a/src/tools/msvc/MSBuildProject.pm
--- b/src/tools/msvc/MSBuildProject.pm
*************** EOF
*** 63,83 ****
    </PropertyGroup>
  EOF

-     # We have to use this flag on 32 bit targets because the 32bit perls
-     # are built with it and sometimes crash if we don't.
-     my $use_32bit_time_t =
-       $self->{platform} eq 'Win32' ? '_USE_32BIT_TIME_T;' : '';
-
      $self->WriteItemDefinitionGroup(
          $f, 'Debug',
!         {   defs    => "_DEBUG;DEBUG=1;$use_32bit_time_t",
              opt     => 'Disabled',
              strpool => 'false',
              runtime => 'MultiThreadedDebugDLL' });
      $self->WriteItemDefinitionGroup(
          $f,
          'Release',
!         {   defs    => "$use_32bit_time_t",
              opt     => 'Full',
              strpool => 'true',
              runtime => 'MultiThreadedDLL' });
--- 63,78 ----
    </PropertyGroup>
  EOF

      $self->WriteItemDefinitionGroup(
          $f, 'Debug',
!         {   defs    => "_DEBUG;DEBUG=1",
              opt     => 'Disabled',
              strpool => 'false',
              runtime => 'MultiThreadedDebugDLL' });
      $self->WriteItemDefinitionGroup(
          $f,
          'Release',
!         {   defs    => "",
              opt     => 'Full',
              strpool => 'true',
              runtime => 'MultiThreadedDLL' });
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index a7e3a01..940bef6 100644
*** a/src/tools/msvc/Mkvcbuild.pm
--- b/src/tools/msvc/Mkvcbuild.pm
*************** sub mkvcbuild
*** 522,528 ****
          my @perl_embed_ccflags;
          foreach my $f (split(" ",$Config{ccflags}))
          {
!             if ($f =~ /^-D[^_]/)
              {
                  $f =~ s/\-D//;
                  push(@perl_embed_ccflags, $f);
--- 522,529 ----
          my @perl_embed_ccflags;
          foreach my $f (split(" ",$Config{ccflags}))
          {
!             if ($f =~ /^-D[^_]/ ||
!                 $f =~ /^-D_USE_32BIT_TIME_T/)
              {
                  $f =~ s/\-D//;
                  push(@perl_embed_ccflags, $f);
diff --git a/src/tools/msvc/VCBuildProject.pm b/src/tools/msvc/VCBuildProject.pm
index a8d75d8..669ba17 100644
*** a/src/tools/msvc/VCBuildProject.pm
--- b/src/tools/msvc/VCBuildProject.pm
*************** sub WriteHeader
*** 33,47 ****
   <Configurations>
  EOF

-     # We have to use this flag on 32 bit targets because the 32bit perls
-     # are built with it and sometimes crash if we don't.
-     my $use_32bit_time_t =
-       $self->{platform} eq 'Win32' ? '_USE_32BIT_TIME_T;' : '';
-
-
      $self->WriteConfiguration(
          $f, 'Debug',
!         {   defs     => "_DEBUG;DEBUG=1;$use_32bit_time_t",
              wholeopt => 0,
              opt      => 0,
              strpool  => 'false',
--- 33,41 ----
   <Configurations>
  EOF

      $self->WriteConfiguration(
          $f, 'Debug',
!         {   defs     => "_DEBUG;DEBUG=1",
              wholeopt => 0,
              opt      => 0,
              strpool  => 'false',
*************** EOF
*** 49,55 ****
      $self->WriteConfiguration(
          $f,
          'Release',
!         {   defs     => "$use_32bit_time_t",
              wholeopt => 0,
              opt      => 3,
              strpool  => 'true',
--- 43,49 ----
      $self->WriteConfiguration(
          $f,
          'Release',
!         {   defs     => "",
              wholeopt => 0,
              opt      => 3,
              strpool  => 'true',

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> writes:
> On Thu, Aug 10, 2017 at 9:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Got it.  So in short, it seems like the attached patch ought to fix it
>> for MSVC builds.  (We'd also need to teach PGAC_CHECK_PERL_EMBED_CCFLAGS
>> to let _USE_32BIT_TIME_T through on Windows, but let's confirm the theory
>> first.)

> We built the sources with this patch and were able to create the plperl
> extension on Windows 32bit and 64bit.

Excellent, thanks for testing.  I'll finish up the configure-script part
and push this shortly.
        regards, tom lane

Re: Sandeep Thakkar 2017-08-08 <CANFyU96tucWPECj8Oy5cevuVwyijVoEZ_Dey08eCEhPk9TM85A@mail.gmail.com>
> Hi
> 
> An update from beta3 build: We are no longer seeing this issue (handshake
> failure) on Windows 64bit, but on Windows 32bit it still persists.

HEAD as of 5a5c2feca still has the same problem on kfreebsd. Is there
anything I could dump so we understand the problem better?

Christoph

Christoph Berg <myon@debian.org> writes:
> HEAD as of 5a5c2feca still has the same problem on kfreebsd. Is there
> anything I could dump so we understand the problem better?

Yeah, I did not expect that 5a5c2feca would change anything on
non-Windows.

What we need to do is verify that PL/Perl's idea of
sizeof(PerlInterpreter) is different from Perl's own idea, and then
find out why --- ie, just which fields have different size/alignment
in the two compiles.

You mentioned upthread that configure shows this:

> checking for CFLAGS recommended by Perl... -D_REENTRANT -D_GNU_SOURCE
> -DDEBIAN -fwrapv -fno-strict-aliasing -pipe -I/usr/local/include
> -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
> checking for CFLAGS to compile embedded Perl... -DDEBIAN

If the source of the problem is the same mechanism as it was for the
other platforms, then presumably the issue is that we need one or more
of-D_REENTRANT-D_GNU_SOURCE-D_LARGEFILE_SOURCE-D_FILE_OFFSET_BITS=64
to be defined while building PL/Perl.  Now, it couldn't be -D_GNU_SOURCE
that's at issue, because we turn that on in src/template/linux:

# Force _GNU_SOURCE on; plperl is broken with Perl 5.8.0 otherwise
CPPFLAGS="$CPPFLAGS -D_GNU_SOURCE"

(That ancient comment is pretty interesting in this connection, isn't it.)

And I'd have thought that _LARGEFILE_SOURCE=1 and _FILE_OFFSET_BITS=64
were the default behavior on any modern platform anyway, but maybe
kfreebsd is weird about that.  Anyway, you could try sticking combinations
of these symbols into perl_embed_ccflags in src/Makefile.global and
rebuilding PL/Perl to see if the problem goes away; if that works it would
give us a leg up on where the problem is.
        regards, tom lane

I wrote:
> Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> writes:
>> We built the sources with this patch and were able to create the plperl
>> extension on Windows 32bit and 64bit.

> Excellent, thanks for testing.  I'll finish up the configure-script part
> and push this shortly.

So the early returns from the buildfarm are that this broke baiji,
although a couple of other Windows critters seem to be OK with it.

This presumably means that baiji's version of perl was built with
_USE_32BIT_TIME_T, but $Config{ccflags} isn't admitting to that.
I wonder what Perl version that is exactly, and what it reports for
$Config{ccflags}, and whether there is some other place that we
ought to be looking for the info.
        regards, tom lane

Dave Page <dpage@postgresql.org> writes:
> It's ActiveState Perl 5.8.8. Printing $Config{ccflags} doesn't seem to do
> anything, but perl -V output is below:

That's weird ... you get nothing from

perl -MConfig -e 'print $Config{ccflags}'

?
        regards, tom lane

Dave Page <dpage@postgresql.org> writes:
> Didn't realise I needed the -MConfig bit (told you my perl-fu was weak :-)
> ):

> C:\Perl\bin>perl -MConfig -e "print $Config{ccflags}"
> -nologo -GF -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT
> -DHAVE_DES_FCRYPT -DNO_HASH_SEED -DUSE_SITECUSTOMIZE
> -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
> -DPERL_MSVCRT_READFIX

So it is indeed not advertising anything about _USE_32BIT_TIME_T ...
but then how do other Perl extensions work?  Strange.

I wonder whether slightly-newer versions of ActiveState Perl work.
        regards, tom lane

I wrote:
> Dave Page <dpage@postgresql.org> writes:
>> C:\Perl\bin>perl -MConfig -e "print $Config{ccflags}"
>> -nologo -GF -W3 -MD -Zi -DNDEBUG -O1 -DWIN32 -D_CONSOLE -DNO_STRICT
>> -DHAVE_DES_FCRYPT -DNO_HASH_SEED -DUSE_SITECUSTOMIZE
>> -DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO
>> -DPERL_MSVCRT_READFIX

> So it is indeed not advertising anything about _USE_32BIT_TIME_T ...
> but then how do other Perl extensions work?  Strange.

So we're between a rock and a hard place here.  ActiveState Perl 5.8.8
requires us to use _USE_32BIT_TIME_T, and doesn't appear to display that
requirement in any standard way.  Newer versions of Perl require us not
to use that symbol unless they say so.

Short of declaring this version of Perl unsupported, the only answer
I can think of is to put a kluge into the MSVC build scripts along
the lines of "if it's 32-bit Windows, and the Perl version is before X,
assume we need _USE_32BIT_TIME_T even if $Config{ccflags} doesn't
say so".  It would be nice to have some hard evidence about what X
should be, but we don't know when ActiveState fixed this.  (I poked
around in their bugzilla, without success.)

In the interests of getting the buildfarm green again before I disappear
on vacation, I propose to do the above with X = 5.10.0.  Anybody
have a better idea?
        regards, tom lane

I wrote:
> Short of declaring this version of Perl unsupported, the only answer
> I can think of is to put a kluge into the MSVC build scripts along
> the lines of "if it's 32-bit Windows, and the Perl version is before X,
> assume we need _USE_32BIT_TIME_T even if $Config{ccflags} doesn't
> say so".  It would be nice to have some hard evidence about what X
> should be, but we don't know when ActiveState fixed this.  (I poked
> around in their bugzilla, without success.)

Ah-hah: it wasn't ActiveState that fixed this at all; it was upstream
Perl.  The stanza that Ashutosh found about defining _USE_32BIT_TIME_T
originated in Perl 5.13.4; older Perls simply don't provide it, no
matter how they were built.

We can now isolate the exact reason we're having trouble on baiji:
it's building Postgres with MSVC 2005, which by default will think
time_t is 64 bits, but it must be using a copy of Perl that was
built with an older Microsoft compiler that doesn't think that.
(Dave's "perl -V" output says ccversion='12.00.8804', but I don't
know how to translate that to the marketing version.)  And since it's
pre-5.13.4, Perl itself doesn't know it should advertise this fact.

So it's now looking to me like we should do the above with X = 5.13.4.
That won't be a perfect solution, but it's about the best we can
readily do.  Realistically, nobody out in the wider world is likely
to care about building current PG releases against such old Perl
versions on Windows; if we satisfy our older buildfarm critters,
it's enough for me.
        regards, tom lane

On Thu, Aug 17, 2017 at 12:15:58PM -0400, Tom Lane wrote:
> I wrote:
> > Short of declaring this version of Perl unsupported, the only answer
> > I can think of is to put a kluge into the MSVC build scripts along
> > the lines of "if it's 32-bit Windows, and the Perl version is before X,
> > assume we need _USE_32BIT_TIME_T even if $Config{ccflags} doesn't
> > say so".  It would be nice to have some hard evidence about what X
> > should be, but we don't know when ActiveState fixed this.  (I poked
> > around in their bugzilla, without success.)
> 
> Ah-hah: it wasn't ActiveState that fixed this at all; it was upstream
> Perl.  The stanza that Ashutosh found about defining _USE_32BIT_TIME_T
> originated in Perl 5.13.4; older Perls simply don't provide it, no
> matter how they were built.
> 
> We can now isolate the exact reason we're having trouble on baiji:
> it's building Postgres with MSVC 2005, which by default will think
> time_t is 64 bits, but it must be using a copy of Perl that was
> built with an older Microsoft compiler that doesn't think that.
> (Dave's "perl -V" output says ccversion='12.00.8804', but I don't
> know how to translate that to the marketing version.)  And since it's
> pre-5.13.4, Perl itself doesn't know it should advertise this fact.
> 
> So it's now looking to me like we should do the above with X = 5.13.4.
> That won't be a perfect solution, but it's about the best we can
> readily do.  Realistically, nobody out in the wider world is likely
> to care about building current PG releases against such old Perl
> versions on Windows; if we satisfy our older buildfarm critters,
> it's enough for me.

MinGW default behavior matches "cl -D_USE_32BIT_TIME_T", and MSVC >= 2005
default behavior matches "gcc -D__MINGW_USE_VC2005_COMPAT"[1].  MinGW-built
Perl[2] does not mention _USE_32BIT_TIME_T in $Config{ccflags}, so we
typically must add _USE_32BIT_TIME_T when using MSVC to build 32-bit against
MinGW-built Perl.  I'm considering two ways to achieve this:

1. If $Config{gccversion} is nonempty, add _USE_32BIT_TIME_T.  This will do  the wrong thing if MinGW changes its
defaultto match modern MSVC.  It will  do the wrong thing for a Perl built with "gcc -D__MINGW_USE_VC2005_COMPAT".
 

2. When configuring the build, determine whether to add _USE_32BIT_TIME_T by  running a test program built with and
withoutthat symbol.  Perhaps have  the test program store and retrieve a PL_modglobal value.  (PL_modglobal  maps to a
PerlInterpreterfield that follows the fields sensitive to  _USE_32BIT_TIME_T, and perlapi documents it since 5.8.0 or
earlier.) This  is more principled than (1), but it will be more code and may have weird  interactions with rare Perl
buildoptions.
 

I am inclined toward (2) if it takes no more than roughly a hundred lines of
code, else (1).  Opinions?  I regret investing in 32-bit Windows.  If there's
any OS where a 32-bit PostgreSQL server makes sense today, it's not Windows.


Ideally, $Config{ccflags} would include -D_USE_32BIT_TIME_T for MinGW-built
Perl, like it does for VC6-built Perl.  Considering Perl versions already in
the field, fixing Perl won't change this need to patch PostgreSQL.

Using MinGW to build a PostgreSQL that links to an MSVC-built Perl probably
requires -D__MINGW_USE_VC2005_COMPAT.  I may not bother for now.  We're less
likely to experience that, because Perl binaries advertised on perl.org or in
the PostgreSQL documentation are MinGW-built.


[1] https://gnunet.org/sorting-out-stat-thing

[2] ActivePerl has been MinGW-built for more than five years, and Strawberry
Perl has always been MinGW-built.  I'm guessing Ashutosh tested with an
MSVC-built Perl like
http://get.enterprisedb.com/languagepacks/edb-languagepack-10-3-windows.exe.

Noah Misch <noah@leadboat.com> writes:
> On Thu, Aug 17, 2017 at 12:15:58PM -0400, Tom Lane wrote:
>> ... it's now looking to me like we should do the above with X = 5.13.4.
>> That won't be a perfect solution, but it's about the best we can
>> readily do.  Realistically, nobody out in the wider world is likely
>> to care about building current PG releases against such old Perl
>> versions on Windows; if we satisfy our older buildfarm critters,
>> it's enough for me.

> MinGW default behavior matches "cl -D_USE_32BIT_TIME_T", and MSVC >= 2005
> default behavior matches "gcc -D__MINGW_USE_VC2005_COMPAT"[1].  MinGW-built
> Perl[2] does not mention _USE_32BIT_TIME_T in $Config{ccflags}, so we
> typically must add _USE_32BIT_TIME_T when using MSVC to build 32-bit against
> MinGW-built Perl.  I'm considering two ways to achieve this:

I don't really have an opinion about the relative merits of these changes,
but why do anything?  The existing solution has the buildfarm happy, and
we've not heard any field complaints that I saw.  I'm not sure we should
spend more time on supporting obsolete toolchain combinations that aren't
represented in the buildfarm.
        regards, tom lane

On Wed, Nov 29, 2017 at 11:34:56PM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Thu, Aug 17, 2017 at 12:15:58PM -0400, Tom Lane wrote:
> >> ... it's now looking to me like we should do the above with X = 5.13.4.
> >> That won't be a perfect solution, but it's about the best we can
> >> readily do.  Realistically, nobody out in the wider world is likely
> >> to care about building current PG releases against such old Perl
> >> versions on Windows; if we satisfy our older buildfarm critters,
> >> it's enough for me.
> 
> > MinGW default behavior matches "cl -D_USE_32BIT_TIME_T", and MSVC >= 2005
> > default behavior matches "gcc -D__MINGW_USE_VC2005_COMPAT"[1].  MinGW-built
> > Perl[2] does not mention _USE_32BIT_TIME_T in $Config{ccflags}, so we
> > typically must add _USE_32BIT_TIME_T when using MSVC to build 32-bit against
> > MinGW-built Perl.  I'm considering two ways to achieve this:
> 
> I don't really have an opinion about the relative merits of these changes,
> but why do anything?  The existing solution has the buildfarm happy, and
> we've not heard any field complaints that I saw.  I'm not sure we should
> spend more time on supporting obsolete toolchain combinations that aren't
> represented in the buildfarm.

It's the other way around.  The buildfarm's 32-bit MSVC animals each use an
obsolete Perl.  PostgreSQL is incompatible with today's 32-bit ActivePerl and
Strawberry Perl.

On Thu, Nov 30, 2017 at 1:34 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Noah Misch <noah@leadboat.com> writes:
>> On Thu, Aug 17, 2017 at 12:15:58PM -0400, Tom Lane wrote:
>>> ... it's now looking to me like we should do the above with X = 5.13.4.
>>> That won't be a perfect solution, but it's about the best we can
>>> readily do.  Realistically, nobody out in the wider world is likely
>>> to care about building current PG releases against such old Perl
>>> versions on Windows; if we satisfy our older buildfarm critters,
>>> it's enough for me.
>
>> MinGW default behavior matches "cl -D_USE_32BIT_TIME_T", and MSVC >= 2005
>> default behavior matches "gcc -D__MINGW_USE_VC2005_COMPAT"[1].  MinGW-built
>> Perl[2] does not mention _USE_32BIT_TIME_T in $Config{ccflags}, so we
>> typically must add _USE_32BIT_TIME_T when using MSVC to build 32-bit against
>> MinGW-built Perl.  I'm considering two ways to achieve this:
>
> I don't really have an opinion about the relative merits of these changes,
> but why do anything?  The existing solution has the buildfarm happy, and
> we've not heard any field complaints that I saw.  I'm not sure we should
> spend more time on supporting obsolete toolchain combinations that aren't
> represented in the buildfarm.

I agree with this position. If people are looking for getting better
coverage about weird component combinations, I'd like to think that
they should provide an animal so as support is live and not
investigated afterwards. Remember for example the recent thread about
overlayfs (https://www.postgresql.org/message-id/20171107135454.lbelbbvfgadljmuj@home.ouaza.com).
On top of that this thread deals with rather old components with 32b
stuff on Windows..
-- 
Michael

Noah Misch <noah@leadboat.com> writes:
> On Wed, Nov 29, 2017 at 11:34:56PM -0500, Tom Lane wrote:
>> I don't really have an opinion about the relative merits of these changes,
>> but why do anything?  The existing solution has the buildfarm happy, and
>> we've not heard any field complaints that I saw.  I'm not sure we should
>> spend more time on supporting obsolete toolchain combinations that aren't
>> represented in the buildfarm.

> It's the other way around.  The buildfarm's 32-bit MSVC animals each use an
> obsolete Perl.  PostgreSQL is incompatible with today's 32-bit ActivePerl and
> Strawberry Perl.

Oh, OK.  In that case, we need to get some representatives of these
more modern builds into the buildfarm while we're at it.
        regards, tom lane

On 11/29/2017 11:45 PM, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
>> On Wed, Nov 29, 2017 at 11:34:56PM -0500, Tom Lane wrote:
>>> I don't really have an opinion about the relative merits of these changes,
>>> but why do anything?  The existing solution has the buildfarm happy, and
>>> we've not heard any field complaints that I saw.  I'm not sure we should
>>> spend more time on supporting obsolete toolchain combinations that aren't
>>> represented in the buildfarm.
>> It's the other way around.  The buildfarm's 32-bit MSVC animals each use an
>> obsolete Perl.  PostgreSQL is incompatible with today's 32-bit ActivePerl and
>> Strawberry Perl.
> Oh, OK.  In that case, we need to get some representatives of these
> more modern builds into the buildfarm while we're at it.
>
>             



My 32 bit XP machine currawong/frogmouth builds against perl 5.16.3,
build dated 2014.

cheers

andrew

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

On Wed, Nov 29, 2017 at 08:14:41PM -0800, Noah Misch wrote:
> 1. If $Config{gccversion} is nonempty, add _USE_32BIT_TIME_T.  This will do
>    the wrong thing if MinGW changes its default to match modern MSVC.  It will
>    do the wrong thing for a Perl built with "gcc -D__MINGW_USE_VC2005_COMPAT".
> 
> 2. When configuring the build, determine whether to add _USE_32BIT_TIME_T by
>    running a test program built with and without that symbol.  Perhaps have
>    the test program store and retrieve a PL_modglobal value.  (PL_modglobal
>    maps to a PerlInterpreter field that follows the fields sensitive to
>    _USE_32BIT_TIME_T, and perlapi documents it since 5.8.0 or earlier.)  This
>    is more principled than (1), but it will be more code and may have weird
>    interactions with rare Perl build options.
> 
> I am inclined toward (2) if it takes no more than roughly a hundred lines of
> code, else (1).  Opinions?  I regret investing in 32-bit Windows.  If there's
> any OS where a 32-bit PostgreSQL server makes sense today, it's not Windows.

Here's an implementation of (2).  This is more intricate than I hoped.  One
could argue for (1), but I estimate (2) wins by a nose.  I successfully tested
http://strawberryperl.com/download/5.14.4.1/strawberry-perl-5.14.4.1-32bit.msi
(Perl 5.14.4; MinGW-built; must have -D_USE_32BIT_TIME_T) and
http://get.enterprisedb.com/languagepacks/edb-languagepack-10-3-windows.exe
(Perl 5.24.0; MSVC-built; must not have -D_USE_32BIT_TIME_T).  I also tried
http://strawberryperl.com/download/5.8.9/strawberry-perl-5.8.9.5.msi, which
experienced a StackHash_0a9e crash during PERL_SYS_INIT3(), with or without
-D_USE_32BIT_TIME_T.  I expect that breakage is orthogonal.  I didn't have
ready access to obsolete MSVC-built Perl, so it will be interesting to see how
the buildfarm likes this.

On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Wed, Nov 29, 2017 at 11:34:56PM -0500, Tom Lane wrote:
> >> I don't really have an opinion about the relative merits of these changes,
> >> but why do anything?  The existing solution has the buildfarm happy, and
> >> we've not heard any field complaints that I saw.  I'm not sure we should
> >> spend more time on supporting obsolete toolchain combinations that aren't
> >> represented in the buildfarm.
> 
> > It's the other way around.  The buildfarm's 32-bit MSVC animals each use an
> > obsolete Perl.  PostgreSQL is incompatible with today's 32-bit ActivePerl and
> > Strawberry Perl.
> 
> Oh, OK.  In that case, we need to get some representatives of these
> more modern builds into the buildfarm while we're at it.

Yep.  Among machines already in the buildfarm, the one running member
woodlouse is the best candidate for this.  Its owner could install
http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
and setup another animal on the same machine that builds 32-bit and enables
Perl.  Christian, are you interested in doing this?

* Noah Misch wrote:

> On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
>> Oh, OK.  In that case, we need to get some representatives of these
>> more modern builds into the buildfarm while we're at it.
>
> Yep.  Among machines already in the buildfarm, the one running member
> woodlouse is the best candidate for this.  Its owner could install
> http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
> and setup another animal on the same machine that builds 32-bit and enables
> Perl.  Christian, are you interested in doing this?

Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run is
successful.

Although I have to admit, I fail to see the need for Windows x86 builds, too. Who in their right mind would want them
today?

--
Christian

On Sun, Dec 10, 2017 at 12:36:13PM +0000, Christian Ullrich wrote:
> * Noah Misch wrote:
> > On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
> >> Oh, OK.  In that case, we need to get some representatives of these
> >> more modern builds into the buildfarm while we're at it.
> > 
> > Yep.  Among machines already in the buildfarm, the one running member
> > woodlouse is the best candidate for this.  Its owner could install
> > http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
> > and setup another animal on the same machine that builds 32-bit and enables
> > Perl.  Christian, are you interested in doing this?
> 
> Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run
issuccessful.
 

Thanks!

> Although I have to admit, I fail to see the need for Windows x86 builds, too. Who in their right mind would want them
today?

I can't see installing a 32-bit Windows postmaster in 2018.  The 32-bit libpq
might be useful.  Download statistics for the binary distributions would be
informative.  On the other hand, removing 32-bit Windows support would
eliminate three veteran buildfarm animals, and maintenance was cheap in the
few years before this thread.  I don't think today is the day to remove
support, but it's coming one of these years.

On Sun, Dec 10, 2017 at 11:46:08AM -0800, Noah Misch wrote:
> On Sun, Dec 10, 2017 at 12:36:13PM +0000, Christian Ullrich wrote:
> > * Noah Misch wrote:
> > > On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
> > >> Oh, OK.  In that case, we need to get some representatives of these
> > >> more modern builds into the buildfarm while we're at it.
> > > 
> > > Yep.  Among machines already in the buildfarm, the one running member
> > > woodlouse is the best candidate for this.  Its owner could install
> > > http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
> > > and setup another animal on the same machine that builds 32-bit and enables
> > > Perl.  Christian, are you interested in doing this?
> > 
> > Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run
issuccessful.
 
> 
> Thanks!

Did the animal assignment come through?  I don't see such an animal reporting.

* From: Noah Misch [mailto:noah@leadboat.com]

> > > Ready to go, waiting for animal assignment. For now, I can
> confirm that it works, that is, the buildfarm --test run is
> successful.

> Did the animal assignment come through?  I don't see such an animal
> reporting.

No, not yet. Sorry, I lost track of it, or I would have mentioned it again earlier.

-- 
Christian

On 01/11/2018 12:08 AM, Noah Misch wrote:
> On Sun, Dec 10, 2017 at 11:46:08AM -0800, Noah Misch wrote:
>> On Sun, Dec 10, 2017 at 12:36:13PM +0000, Christian Ullrich wrote:
>>> * Noah Misch wrote:
>>>> On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
>>>>> Oh, OK.  In that case, we need to get some representatives of these
>>>>> more modern builds into the buildfarm while we're at it.
>>>> Yep.  Among machines already in the buildfarm, the one running member
>>>> woodlouse is the best candidate for this.  Its owner could install
>>>> http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
>>>> and setup another animal on the same machine that builds 32-bit and enables
>>>> Perl.  Christian, are you interested in doing this?
>>> Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run
issuccessful.
 
>> Thanks!
> Did the animal assignment come through?  I don't see such an animal reporting.


Looks like it's still in the queue. Will approve now.

cheers

andrew

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

* Christian Ullrich wrote:

> * Noah Misch wrote:
> 
>> On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
>>> Oh, OK.  In that case, we need to get some representatives of these
>>> more modern builds into the buildfarm while we're at it.
>>
>> Yep.  Among machines already in the buildfarm, the one running member
>> woodlouse is the best candidate for this.  Its owner could install
>> http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
>> and setup another animal on the same machine that builds 32-bit and enables
>> Perl.  Christian, are you interested in doing this?
> 
> Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run
issuccessful.
 

Up and running now, name is whelk, first report on REL9_6_STABLE.

Sorry it took me another ten days to complete the configuration.

-- 
Christian

On Sun, Jan 21, 2018 at 11:34:07AM +0100, Christian Ullrich wrote:
> * Christian Ullrich wrote:
> >* Noah Misch wrote:
> >>On Wed, Nov 29, 2017 at 11:45:35PM -0500, Tom Lane wrote:
> >>>Oh, OK.  In that case, we need to get some representatives of these
> >>>more modern builds into the buildfarm while we're at it.
> >>
> >>Yep.  Among machines already in the buildfarm, the one running member
> >>woodlouse is the best candidate for this.  Its owner could install
> >>http://strawberryperl.com/download/5.26.1.1/strawberry-perl-5.26.1.1-32bit.msi
> >>and setup another animal on the same machine that builds 32-bit and enables
> >>Perl.  Christian, are you interested in doing this?
> >
> >Ready to go, waiting for animal assignment. For now, I can confirm that it works, that is, the buildfarm --test run
issuccessful.
 
> 
> Up and running now, name is whelk, first report on REL9_6_STABLE.
> 
> Sorry it took me another ten days to complete the configuration.

This is great.  Thanks.

Buildfarm metadata reports whelk using "Microsoft Visual C++ 2010", but the
run logs show it's using MSVC 2013, like woodlouse does.  Would you update
that buildfarm metadata (with update_personality.pl)?