Обсуждение: [HACKERS] pl/perl extension fails on Windows
Hi,
--
I compiled PG 10 beta1/beta2 with "--with-perl" option on Windows and the extension crashes the database.
--
postgres=# create extension plperl;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
postgres=#
It doesn't produce crashdump (in $DATA/crashdumps) but the log contains the following error:
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.
The Perl that is used during compilation and on the target machine is same. So probably plperl is not able to load the perl library. It works fine on Linux and MacOS.
Sandeep Thakkar
EDB
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 Wed, Jul 12, 2017 at 4:35 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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?
Well crake is a Fedora box - and we have no problems on Linux, only on Windows.
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
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
Вложения
On Thu, Jul 13, 2017 at 10:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.
Okay, I tried to explore on this a bit and my findings are as follows.
On Linux, the handshake key being generated in plperl code i.e. inside XS_EXTERNAL() in Util.c (generated from Util.xs during build time) and perl code (inside Perl_xs_handshake function) are same which means the following condition inside Perl_xs_handshake() becomes true and therefore, there is no mismatch error.
got = INT2PTR(void*, (UV)(key & HSm_KEY_MATCH));
need = (void *)(HS_KEY(FALSE, FALSE, "", "") & HSm_KEY_MATCH);
if (UNLIKELY(got != need))
goto bad_handshake;
However, on Windows, the handshake key generated in plperl code inside XS_EXTERNAL() and in perl code i.e. inside Perl_xs_handshake() are different thereby resulting in a mismatch error.
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.
This is how PerlInterpreter is defined in Perl source,
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. But, then the point is, why no such difference is observed on Linux. Well, as of now, i haven't found the reason behind it and i am still investigating on it.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www. enterprisedb.com
>
> regards, tom lane
> 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.
Okay, I tried to explore on this a bit and my findings are as follows.
On Linux, the handshake key being generated in plperl code i.e. inside XS_EXTERNAL() in Util.c (generated from Util.xs during build time) and perl code (inside Perl_xs_handshake function) are same which means the following condition inside Perl_xs_handshake() becomes true and therefore, there is no mismatch error.
got = INT2PTR(void*, (UV)(key & HSm_KEY_MATCH));
need = (void *)(HS_KEY(FALSE, FALSE, "", "") & HSm_KEY_MATCH);
if (UNLIKELY(got != need))
goto bad_handshake;
However, on Windows, the handshake key generated in plperl code inside XS_EXTERNAL() and in perl code i.e. inside Perl_xs_handshake() are different thereby resulting in a mismatch error.
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.
This is how PerlInterpreter is defined in Perl source,
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. But, then the point is, why no such difference is observed on Linux. Well, as of now, i haven't found the reason behind it and i am still investigating on it.
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.
>
> regards, tom lane
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 Thu, Jul 27, 2017 at 7:51 PM, 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?
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
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www. enterprisedb.com
>
> regards, tom lane
> 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?
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
--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.
>
> 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
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.
On Mon, Jul 31, 2017 at 10:15 PM, Christoph Berg <myon@debian.org> wrote:
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
--
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
EDB
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 8:19 PM, Robert Haas <robertmhaas@gmail.com> wrote:
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.
Yes, that would have been better. The patch was tested only on Windows 64bit and we never thought that it won't work on 32bit.
What was the exact message you saw, including the hex values?
Attach is the screenshot of the contents of the postmaster log that was sent to me by one of my team members who was testing this.
Is the Perl you were building against for plperl the same Perl that
was being used for the build itself?
Yes.
Do you have the portion of the build log where src/pl/plperl was being built?
I copied and pasted that portion of the build log into file build.log (attached) for Windows 32bit and Windows 64bit.
Thanks.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Sandeep Thakkar
EDB
Вложения
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
Hi
--
On Thu, Aug 10, 2017 at 9:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.)
We built the sources with this patch and were able to create the plperl extension on Windows 32bit and 64bit.
regards, tom lane
Sandeep Thakkar
EDB
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
On Mon, Aug 14, 2017 at 9:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.
It's ActiveState Perl 5.8.8. Printing $Config{ccflags} doesn't seem to do anything, but perl -V output is below:
C:\Program Files\Microsoft Visual Studio 8\VC>c:\perl\bin\perl -V
Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
Platform:
osname=MSWin32, osvers=4.0, archname=MSWin32-x86-multi-thread
uname=''
config_args='undef'
hint=recommended, useposix=true, d_sigaction=undef
usethreads=define use5005threads=undef useithreads=define usemultiplicity=de
fine
useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
use64bitint=undef use64bitall=undef uselongdouble=undef
usemymalloc=n, bincompat5005=undef
Compiler:
cc='cl', 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',
optimize='-MD -Zi -DNDEBUG -O1',
cppflags='-DWIN32'
ccversion='12.00.8804', gccversion='', gccosandvers=''
intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', lseeksi
ze=8
alignbytes=8, prototype=define
Linker and Libraries:
ld='link', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf -libpath:"C:
\Perl\lib\CORE" -machine:x86'
libpth=\lib
libs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comdlg32
.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib ws2_
32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib
perllibs= oldnames.lib kernel32.lib user32.lib gdi32.lib winspool.lib comd
lg32.lib advapi32.lib shell32.lib ole32.lib oleaut32.lib netapi32.lib uuid.lib
ws2_32.lib mpr.lib winmm.lib version.lib odbc32.lib odbccp32.lib msvcrt.lib
libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib
gnulibc_version=''
Dynamic Linking:
dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
cccdlflags=' ', lddlflags='-dll -nologo -nodefaultlib -debug -opt:ref,icf -
libpath:"C:\Perl\lib\CORE" -machine:x86'
Characteristics of this binary (from libperl):
Compile-time options: MULTIPLICITY PERL_IMPLICIT_CONTEXT
PERL_IMPLICIT_SYS PERL_MALLOC_WRAP
PL_OP_SLAB_ALLOC USE_ITHREADS USE_LARGE_FILES
USE_PERLIO USE_SITECUSTOMIZE
Locally applied patches:
ActivePerl Build 820 [274739]
Iin_load_module moved for compatibility with build 806
PerlEx support in CGI::Carp
Less verbose ExtUtils::Install and Pod::Find
Patch for CAN-2005-0448 from Debian with modifications
Rearrange @INC so that 'site' is searched before 'perl'
Partly reverted 24733 to preserve binary compatibility
29930 win32.c typo in #define MULTIPLICITY
29868 win32_async_check() can still loop indefinitely
29690,29732 ANSIfy the PATH environment variable on Windows
29689 Add error handling to win32_ansipath
29675 Use short pathnames in $^X and @INC
29607,29676 allow blib.pm to be used for testing Win32 module
29605 Implement killpg() for MSWin32
29598 cwd() to return the short pathname
29597 let readdir() return the alternate filename
29590 Don't destroy the Unicode system environment on Perl startup
29528 get ext/Win32/Win32.xs to compile on cygwin
29509,29510,29511 Move Win32::* functions into Win32 module
29483 Move Win32 from win32/ext/Win32 to ext/Win32
29481 Makefile.PL changes to compile Win32.xs using cygwin
28671 Define PERL_NO_DEV_RANDOM on Windows
28376 Add error checks after execing PL_cshname or PL_sh_path
28305 Pod::Html should not convert "foo" into ``foo''
27833 Change anchor generation in Pod::Html for '=item item 2'
27832,27847 fix Pod::Html::depod() for multi-line strings
27719 Document the functions htmlify() and anchorify() in Pod::Html
27619 Bug in Term::ReadKey being triggered by a bug in Term::ReadLine
27549 Move DynaLoader.o into libperl.so
27528 win32_pclose() error exit doesn't unlock mutex
27527 win32_async_check() can loop indefinitely
27515 ignore directories when searching @INC
27359 Fix -d:Foo=bar syntax
27210 Fix quote typo in c2ph
27203 Allow compiling swigged C++ code
27200 Make stat() on Windows handle trailing slashes correctly
27133 Initialise lastparen in the regexp structure
27061 L<PerlIO> and Pod::Html
27034 Avoid "Prototype mismatch" warnings with autouse
26970 Make Passive mode the default for Net::FTP
26921 Avoid getprotobyname/number calls in IO::Socket::INET
26897,26903 Make common IPPROTO_* constants always available
26670 Make '-s' on the shebang line parse -foo=bar switches
26637 Make Borland and MinGW happy with change 26379
26536 INSTALLSCRIPT versus INSTALLDIRS
26379 Fix alarm() for Windows 2003
26087 Storable 0.1 compatibility
25861 IO::File performace issue
25084 long groups entry could cause memory exhaustion
24699 ICMP_UNREACHABLE handling in Net::Ping
Built under MSWin32
Compiled at Jan 23 2007 15:57:46
@INC:
c:/perl/site/lib
c:/perl/lib
.
C:\Program Files\Microsoft Visual Studio 8\VC>
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
On Thu, Aug 17, 2017 at 2:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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}'
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
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)?