Thread: Core dump running PL/Perl installcheck with bleadperl [PATCH]
I encountered a core dump running PL/Perl installcheck with a very recent git HEAD of PostgreSQL and a not quite so recent git HEAD of perl. The cause is a subtle difference between SvTYPE(sv) == SVt_RV and SvROK(sv). The former is checking a low-level implementation detail while the later is directly checking "does this sv contains a reference". The attached patch fixes the problem by changing the SvTYPE check to use SvROK instead. Although I only tripped over one case, the patch changes all four uses of SvTYPE(sv) == SVt_RV. The remaining uses of SvTYPE are ok. Tim.
Attachment
Tim Bunce <Tim.Bunce@pobox.com> writes:
> I encountered a core dump running PL/Perl installcheck with a very
> recent git HEAD of PostgreSQL and a not quite so recent git HEAD of perl.
> The cause is a subtle difference between SvTYPE(sv) == SVt_RV and
> SvROK(sv). The former is checking a low-level implementation detail
> while the later is directly checking "does this sv contains a reference".
Hmm. Seems like this patch begs the question: if checking SvTYPE(*svp)
isn't safe, why is it safe to look at SvTYPE(SvRV(*svp))? Shouldn't the
tests against SVt_PVHV be made more abstract as well?
regards, tom lane
On Sun, Mar 07, 2010 at 12:11:26PM -0500, Tom Lane wrote: > Tim Bunce <Tim.Bunce@pobox.com> writes: > > I encountered a core dump running PL/Perl installcheck with a very > > recent git HEAD of PostgreSQL and a not quite so recent git HEAD of perl. > > > The cause is a subtle difference between SvTYPE(sv) == SVt_RV and > > SvROK(sv). The former is checking a low-level implementation detail > > while the later is directly checking "does this sv contains a reference". > > Hmm. Seems like this patch begs the question: if checking SvTYPE(*svp) > isn't safe, why is it safe to look at SvTYPE(SvRV(*svp))? Shouldn't the > tests against SVt_PVHV be made more abstract as well? Some SvTYPE values, like SVt_RV, allow the SV to hold one of a number of different kinds of things. Others, like SVt_PVHV, don't. No, I don't like it either but that's the way the "Jenga tower made of yaks" (to use a phrase recently coined by one of the perl maintainers) has grown. Something like an SvRVHVOK(sv) would be welcome sugar. Tim.
Tim Bunce <Tim.Bunce@pobox.com> writes:
> The attached patch fixes the problem by changing the SvTYPE check to use
> SvROK instead. Although I only tripped over one case, the patch changes
> all four uses of SvTYPE(sv) == SVt_RV. The remaining uses of SvTYPE are ok.
Applied back to 8.0. 7.4 seems not to contain any tests of this form.
regards, tom lane
Environment: Windows Vista, PostgreSQL 8.4 (1-click installer), Visual Studio 2005 sp1. I have a bare-bones DLL built as per the above, compiling the 'add_one' and 'copytext' samples found at http://www.postgresql.org/docs/8.4/interactive/xfunc-c.html (version 1 calling convention), compiled as 'C'. I can use 'add_one' just fine from within SQL, but if I use 'copytext', an access violation occurs as soon as palloc() is called. Could anyone suggest what the problem might be? Failing that, are there any other (creative?) ways to return strings from a C-language function without using palloc? Thanks in advance for any leads Kevin.
"Kevin Flanagan" <kevin-f@linkprior.com> writes: > Environment: Windows Vista, PostgreSQL 8.4 (1-click installer), Visual > Studio 2005 sp1. > I have a bare-bones DLL built as per the above, compiling the 'add_one' and > 'copytext' samples found at > http://www.postgresql.org/docs/8.4/interactive/xfunc-c.html (version 1 > calling convention), compiled as 'C'. I can use 'add_one' just fine from > within SQL, but if I use 'copytext', an access violation occurs as soon as > palloc() is called. > Could anyone suggest what the problem might be? Hard to tell without seeing the actual code and a stack trace, but I'd bet that you haven't fully resolved the build process problems you mentioned earlier. I'm thinking this may be a symptom of linkage failure, since palloc is probably the first place in the above-described sequence where your DLL is going to call back into the core backend. Another possibility is that you mistranscribed the example somehow. Maybe you forgot the PG_FUNCTION_INFO_V1(copytext) macro? > Failing that, are there any other (creative?) ways to return strings from a > C-language function without using palloc? If you can't make those examples work, you have fundamental problems you need to fix, not find a "creative workaround". regards, tom lane
Re: Access violation from palloc, Visual Studio 2005, C-language function
From
"Kevin Flanagan"
Date:
Well - >> Hard to tell without seeing the actual code and a stack trace, but I'd bet that you haven't fully resolved the build process problems you mentioned earlier. << I've attached a zip of the (tiny) project, and a text file with the contents of the module containing the C-language functions. The only difference from sample code is that (as pointed out by Takahiro Itagaki in his post here of 8th March) the function implementations need decorating with __declspec(dllexport). (I hope the attachments don't break mailing list policy.) >> I'm thinking this may be a symptom of linkage failure, since palloc is probably the first place in the above-described sequence where your DLL is going to call back into the core backend. << Hmm. But isn't palloc found in postgres.lib, which my DLL statically links to? Or is that not the lib I'm supposed to link to? (found in c c:\Program Files\PostgreSQL\8.4\lib) If I don't include it as an input to the linker, I get "unresolved external symbol _MemoryContextAlloc referenced in function _copytext" and other unresolved externals ... >> Another possibility is that you mistranscribed the example somehow. Maybe you forgot the PG_FUNCTION_INFO_V1(copytext) macro? << No, that's there. >> > Failing that, are there any other (creative?) ways to return strings from a > C-language function without using palloc? If you can't make those examples work, you have fundamental problems you need to fix, not find a "creative workaround". << I certainly agree in principle, but when you have a deadline to meet, sometimes you can be under great pressure to find a temporary workaround ... with the emphasis on temporary. Thanks in advance for any further leads Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Access violation from palloc, Visual Studio 2005, C-language function
From
"Kevin Flanagan"
Date:
Forgot to include the call stack info, such as it is - screen shot attached. Kevin. -----Original Message----- From: pgsql-hackers-owner@postgresql.org [mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Kevin Flanagan Sent: 10 March 2010 09:29 To: 'Tom Lane' Cc: 'PostgreSQL-development' Subject: Re: [HACKERS] Access violation from palloc, Visual Studio 2005, C-language function Well - >> Hard to tell without seeing the actual code and a stack trace, but I'd bet that you haven't fully resolved the build process problems you mentioned earlier. << I've attached a zip of the (tiny) project, and a text file with the contents of the module containing the C-language functions. The only difference from sample code is that (as pointed out by Takahiro Itagaki in his post here of 8th March) the function implementations need decorating with __declspec(dllexport). (I hope the attachments don't break mailing list policy.) >> I'm thinking this may be a symptom of linkage failure, since palloc is probably the first place in the above-described sequence where your DLL is going to call back into the core backend. << Hmm. But isn't palloc found in postgres.lib, which my DLL statically links to? Or is that not the lib I'm supposed to link to? (found in c c:\Program Files\PostgreSQL\8.4\lib) If I don't include it as an input to the linker, I get "unresolved external symbol _MemoryContextAlloc referenced in function _copytext" and other unresolved externals ... >> Another possibility is that you mistranscribed the example somehow. Maybe you forgot the PG_FUNCTION_INFO_V1(copytext) macro? << No, that's there. >> > Failing that, are there any other (creative?) ways to return strings > from a > C-language function without using palloc? If you can't make those examples work, you have fundamental problems you need to fix, not find a "creative workaround". << I certainly agree in principle, but when you have a deadline to meet, sometimes you can be under great pressure to find a temporary workaround ... with the emphasis on temporary. Thanks in advance for any further leads Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
"Kevin Flanagan" <kevin-f@linkprior.com> writes:
>>> Hard to tell without seeing the actual code and a stack trace, but I'd
>>> bet that you haven't fully resolved the build process problems you
>>> mentioned earlier.
> I've attached a zip of the (tiny) project, and a text file with the contents
> of the module containing the C-language functions. The only difference from
> sample code is that (as pointed out by Takahiro Itagaki in his post here of
> 8th March) the function implementations need decorating with
> __declspec(dllexport).
Mph. I don't actually believe that, nor do I believe the #define
BUILDING_DLL you put in, because neither of those are needed in any of
our contrib modules. What I suspect at this point is that the reference
to CurrentMemoryContext in the palloc() macro is being bollixed by
having the wrong value for BUILDING_DLL. However, not having a Windows
build environment to experiment with, I'll have to defer to somebody
with more experience in that.
(I wonder BTW if we should rename BUILDING_DLL, because it seems a bit
misnamed. AIUI it's supposed to be set while building the core backend,
not while building loadable modules.)
regards, tom lane
Re: Access violation from palloc, Visual Studio 2005, C-language function
From
"Kevin Flanagan"
Date:
Aha. I'd read that the build process for the contrib modules involved
generating a .DEF file for the necessary exports. I had the impression that
defining BUILDING_DLL was an alternative, addressing (part) of the issue
(that is, PG_FUNCTION_INFO_V1 declares functions as 'extern PGDLLIMPORT',
and if you define BUILDING_DLL, then PGDLLIMPORT is defined as ' __declspec
(dllexport)'). But you're quite right, if I take out the BUILDING_DLL
definition, and put the __declspec (dllexport) stuff in piecemeal, the
access violation goes away. Thank goodness.
Thanks, that really helped me out.
Kevin.
-----Original Message-----
From: pgsql-hackers-owner@postgresql.org
[mailto:pgsql-hackers-owner@postgresql.org] On Behalf Of Tom Lane
Sent: 10 March 2010 18:51
To: Kevin Flanagan
Cc: 'PostgreSQL-development'
Subject: Re: [HACKERS] Access violation from palloc, Visual Studio 2005,
C-language function
"Kevin Flanagan" <kevin-f@linkprior.com> writes:
>>> Hard to tell without seeing the actual code and a stack trace, but I'd
>>> bet that you haven't fully resolved the build process problems you
>>> mentioned earlier.
> I've attached a zip of the (tiny) project, and a text file with the
contents
> of the module containing the C-language functions. The only difference
from
> sample code is that (as pointed out by Takahiro Itagaki in his post here
of
> 8th March) the function implementations need decorating with
> __declspec(dllexport).
Mph. I don't actually believe that, nor do I believe the #define
BUILDING_DLL you put in, because neither of those are needed in any of
our contrib modules. What I suspect at this point is that the reference
to CurrentMemoryContext in the palloc() macro is being bollixed by
having the wrong value for BUILDING_DLL. However, not having a Windows
build environment to experiment with, I'll have to defer to somebody
with more experience in that.
(I wonder BTW if we should rename BUILDING_DLL, because it seems a bit
misnamed. AIUI it's supposed to be set while building the core backend,
not while building loadable modules.)
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
"Kevin Flanagan" <kevin-f@linkprior.com> writes:
> Aha. I'd read that the build process for the contrib modules involved
> generating a .DEF file for the necessary exports. I had the impression that
> defining BUILDING_DLL was an alternative, addressing (part) of the issue
> (that is, PG_FUNCTION_INFO_V1 declares functions as 'extern PGDLLIMPORT',
> and if you define BUILDING_DLL, then PGDLLIMPORT is defined as ' __declspec
> (dllexport)'). But you're quite right, if I take out the BUILDING_DLL
> definition, and put the __declspec (dllexport) stuff in piecemeal, the
> access violation goes away. Thank goodness.
I remember having complained that that part of the PG_FUNCTION_INFO_V1
macro seemed backwards, and never really getting a satisfactory
explanation of why it isn't (ie, why it shouldn't be designed to expand
as __declspec (dllexport) instead). But anyway, I think the
conventional wisdom for exporting functions from a loadable module is
to use "dlltool --export-all" rather than bothering with being
selective.
regards, tom lane