Re: PL/Perl Does not Like vstrings

Lists: pgsql-hackers
From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: PL/Perl Does not Like vstrings
Date: 2012-01-04 05:47:50
Message-ID: 1125F328-4154-4B8B-B2F9-1D4AB63A3632@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Is this perhaps by design?

Oy, this doesn’t look good:

$ do LANGUAGE plperl $$ elog(NOTICE, $^V) $$;
ERROR: server conn crashed?
ERROR: server conn crashed?
The connection to the server was lost. Attempting reset: Succeeded.
(pgxn(at)localhost:5900) 06:44:42 [pgxn]
$

Best,

David


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 08:44:12
Message-ID: 4F04115C.90105@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 12:47 AM, David E. Wheeler wrote:
> Is this perhaps by design?
>
> Oy, this doesn’t look good:
>
> $ do LANGUAGE plperl $$ elog(NOTICE, $^V) $$;
> ERROR: server conn crashed?
> ERROR: server conn crashed?
> The connection to the server was lost. Attempting reset: Succeeded.
> (pgxn(at)localhost:5900) 06:44:42 [pgxn]
> $
>

Try

elog(NOTICE, "$^V")

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 16:15:51
Message-ID: 15638.1325693751@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 01/04/2012 12:47 AM, David E. Wheeler wrote:
>> Oy, this doesnt look good:
>> $ do LANGUAGE plperl $$ elog(NOTICE, $^V) $$;
>> The connection to the server was lost. Attempting reset: Succeeded.

> Try
> elog(NOTICE, "$^V")

Isn't this a Perl bug? It seems to be crashing in SvPVutf8, which
means that either Perl passed something that's not an SV to a function
declared to accept SVs, or that SvPVutf8 fails on some SVs. Either
way, Perl is failing to satisfy the POLA if you ask me.

regards, tom lane


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 17:07:36
Message-ID: 045835BB-1D22-4816-9416-0BC2816F185A@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 4, 2012, at 12:44 AM, Andrew Dunstan wrote:

> Try
>
> elog(NOTICE, "$^V")

Yeah, I used

elog(NOTICE, version->new($^V))

Which was fine. But still, it should’t segfault.

David


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 17:08:10
Message-ID: FC77D97F-4F46-4156-B54F-9315363202B2@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 4, 2012, at 8:15 AM, Tom Lane wrote:

> Isn't this a Perl bug? It seems to be crashing in SvPVutf8, which
> means that either Perl passed something that's not an SV to a function
> declared to accept SVs, or that SvPVutf8 fails on some SVs. Either
> way, Perl is failing to satisfy the POLA if you ask me.

Possibly, though I know nothing of Perl’s internals. Someone able to come up with a simple test case?

Thanks,

David


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 17:22:48
Message-ID: 4F048AE8.3000501@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 11:15 AM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> On 01/04/2012 12:47 AM, David E. Wheeler wrote:
>>> Oy, this doesn’t look good:
>>> $ do LANGUAGE plperl $$ elog(NOTICE, $^V) $$;
>>> The connection to the server was lost. Attempting reset: Succeeded.
>> Try
>> elog(NOTICE, "$^V")
> Isn't this a Perl bug? It seems to be crashing in SvPVutf8, which
> means that either Perl passed something that's not an SV to a function
> declared to accept SVs, or that SvPVutf8 fails on some SVs. Either
> way, Perl is failing to satisfy the POLA if you ask me.

Ummmm, not sure.

The docs (perldoc perlvar) seem to suggest $^V isn't an SV (i.e. a
scalar) but some other sort of animal:

$^V The revision, version, and subversion of the Perl interpreter,
represented as a "version" object.

This variable first appeared in perl 5.6.0; earlier versions of
perl will see an undefined value. Before perl 5.10.0 $^V was
represented as a v-string.

$^V can be used to determine whether the Perl interpreter
executing a script is in the right range of versions. For
example:

warn "Hashes not randomized!\n" if !$^V or $^V lt v5.8.1

To convert $^V into its string representation use "sprintf()"'s
"%vd" conversion:

printf "version is v%vd\n", $^V; # Perl's version

But Util.xs::util_elog() expects an SV and doesn't check whether or not
it actually has one. I've found a few other ways of crashing this call
(e.g. by passing a typeglob), so maybe we need to test that we actually
have an SV. I think SvOK() is what we'd use for that - perl gurus please
confirm.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 17:56:30
Message-ID: 17681.1325699790@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The docs (perldoc perlvar) seem to suggest $^V isn't an SV (i.e. a
> scalar) but some other sort of animal:

Yeah, it's a version object, but I'd have thought that SvPV and friends
would automatically stringify such an object. Otherwise, practically
any kind of perl extension could be crashed by passing it one, no?

> But Util.xs::util_elog() expects an SV and doesn't check whether or not
> it actually has one. I've found a few other ways of crashing this call
> (e.g. by passing a typeglob), so maybe we need to test that we actually
> have an SV. I think SvOK() is what we'd use for that - perl gurus please
> confirm.

I looked at that last night but it appeared that SvOK would be perfectly
happy. (Didn't actually try it, though, I was just eyeballing the flags
in gdb.)

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 20:13:28
Message-ID: 4F04B2E8.1050808@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 12:56 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> The docs (perldoc perlvar) seem to suggest $^V isn't an SV (i.e. a
>> scalar) but some other sort of animal:
> Yeah, it's a version object, but I'd have thought that SvPV and friends
> would automatically stringify such an object. Otherwise, practically
> any kind of perl extension could be crashed by passing it one, no?
>
>> But Util.xs::util_elog() expects an SV and doesn't check whether or not
>> it actually has one. I've found a few other ways of crashing this call
>> (e.g. by passing a typeglob), so maybe we need to test that we actually
>> have an SV. I think SvOK() is what we'd use for that - perl gurus please
>> confirm.
> I looked at that last night but it appeared that SvOK would be perfectly
> happy. (Didn't actually try it, though, I was just eyeballing the flags
> in gdb.)
>
>

I tested it and you're right, it doesn't help. I don't see what else we
can do about it. There doesn't appear to be any test for an SV in the API.

cheers

andrew


From: Alex Hunsaker <badalex(at)gmail(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 20:27:07
Message-ID: CAFaPBrTk4ZxNd9ygu_bDgN-vD20bmFtefwTpBON-DkjdZyT-zQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jan 4, 2012 at 13:13, Andrew Dunstan <andrew(at)dunslane(dot)net> wrote:
>
>
> On 01/04/2012 12:56 PM, Tom Lane wrote:

>> I looked at that last night but it appeared that SvOK would be perfectly
>> happy.  (Didn't actually try it, though, I was just eyeballing the flags
>> in gdb.)

>
> I tested it and you're right, it doesn't help. I don't see what else we can
> do about it. There doesn't appear to be any test for an SV in the API.

I think about the best we can do is something along the lines of:

sv2cstr()
{
...
if (Perl_vverify(sv))
return utf_u2e(SvPV(sv));
...
}

I dont the the utf_u2e is strictly needed (other than that it strdups)
as I don't think versions can have utf8 chars, or at least that $^V
will not have utf8 chars (and even if it did it would only cause
problems if they had codepoints in >128 <255).

We would still have issues with typeglobs...


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 20:56:41
Message-ID: 21820.1325710601@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 01/04/2012 12:56 PM, Tom Lane wrote:
>> I looked at that last night but it appeared that SvOK would be perfectly
>> happy. (Didn't actually try it, though, I was just eyeballing the flags
>> in gdb.)

> I tested it and you're right, it doesn't help. I don't see what else we
> can do about it. There doesn't appear to be any test for an SV in the API.

I think what's being passed *is* an SV --- at least, the contents look
reasonable in gdb --- but for some reason SvPVutf8 isn't coping with
this particular kind of SV. Googling suggests that SvPVutf8 used to
fail on READONLY SVs, of which this is one if I'm reading the flag bits
correctly; but that was supposedly fixed years ago. I believe we've hit
some other undocumented limitation of that function, which the Perl guys
may or may not acknowledge as a bug once we've tracked it down better.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 21:36:38
Message-ID: 4F04C666.8090905@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 03:56 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> On 01/04/2012 12:56 PM, Tom Lane wrote:
>>> I looked at that last night but it appeared that SvOK would be perfectly
>>> happy. (Didn't actually try it, though, I was just eyeballing the flags
>>> in gdb.)
>> I tested it and you're right, it doesn't help. I don't see what else we
>> can do about it. There doesn't appear to be any test for an SV in the API.
> I think what's being passed *is* an SV --- at least, the contents look
> reasonable in gdb --- but for some reason SvPVutf8 isn't coping with
> this particular kind of SV. Googling suggests that SvPVutf8 used to
> fail on READONLY SVs, of which this is one if I'm reading the flag bits
> correctly; but that was supposedly fixed years ago. I believe we've hit
> some other undocumented limitation of that function, which the Perl guys
> may or may not acknowledge as a bug once we've tracked it down better.
>
>

Well, the crash is apparently solved by the following, which your
investigation suggested to me:

diff --git a/src/pl/plperl/Util.xs b/src/pl/plperl/Util.xs
index 7d0102b..0785e2e 100644
--- a/src/pl/plperl/Util.xs
+++ b/src/pl/plperl/Util.xs
@@ -41,7 +41,7 @@ do_util_elog(int level, SV *msg)

PG_TRY();
{
- cmsg = sv2cstr(msg);
+ cmsg = sv2cstr(newSVsv(msg));
elog(level, "%s", cmsg);
pfree(cmsg);
}

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 22:05:29
Message-ID: 23270.1325714729@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 01/04/2012 03:56 PM, Tom Lane wrote:
>> I think what's being passed *is* an SV --- at least, the contents look
>> reasonable in gdb --- but for some reason SvPVutf8 isn't coping with
>> this particular kind of SV. Googling suggests that SvPVutf8 used to
>> fail on READONLY SVs, of which this is one if I'm reading the flag bits
>> correctly; but that was supposedly fixed years ago. I believe we've hit
>> some other undocumented limitation of that function, which the Perl guys
>> may or may not acknowledge as a bug once we've tracked it down better.

> Well, the crash is apparently solved by the following, which your
> investigation suggested to me:
> - cmsg = sv2cstr(msg);
> + cmsg = sv2cstr(newSVsv(msg));

That's kinda grotty ... and leaky ...

I installed perl-debuginfo and soon found that SvPVutf8 leads to here:

(gdb) s
9066 Perl_croak(aTHX_ "Can't coerce readonly %s to string in %s",
(gdb) bt
#0 Perl_sv_pvn_force_flags (my_perl=0x17f3170, sv=0x18b6c50,
lp=0x7fffb0c8e2f8, flags=<optimized out>) at sv.c:9066
#1 0x00000038c30c7003 in Perl_sv_utf8_upgrade_flags_grow (my_perl=0x17f3170,
sv=0x18b6c50, flags=2, extra=0) at sv.c:3228
#2 0x00000038c30c7778 in Perl_sv_2pvutf8 (my_perl=0x17f3170, sv=0x18b6c50,
lp=0x7fffb0c8e370) at sv.c:3079
#3 0x00007f4308447614 in sv2cstr (sv=0x18b6c50) at plperl_helpers.h:54
#4 0x00007f430844771f in do_util_elog (level=18, msg=0x18b6c50) at Util.xs:44
#5 0x00007f4308447bdc in XS__elog (my_perl=0x17f3170, cv=0x181e008)
at Util.xs:105
#6 0x00000038c30b548f in Perl_pp_entersub (my_perl=0x17f3170) at pp_hot.c:3046
#7 0x00000038c30ac796 in Perl_runops_standard (my_perl=0x17f3170) at run.c:41
#8 0x00000038c30480ae in Perl_call_sv (my_perl=0x17f3170, sv=0x19843e0,
flags=10) at perl.c:2647
#9 0x00007f4308440f3e in plperl_call_perl_func (desc=0x7fffb0c8e8b0,
fcinfo=0x7fffb0c8fe10) at plperl.c:2018
#10 0x00007f430843fa99 in plperl_inline_handler (fcinfo=0x7fffb0c902a0)
at plperl.c:1751

which leads to a few conclusions:

1. SvPVutf8 fails on readonly SVs, despite the fact that no such
limitation is documented and that this was supposedly fixed in 2004, cf
http://www.nntp.perl.org/group/perl.perl5.porters/2004/03/msg89505.html
We ought to hold somebody's feet to the fire about that. I don't really
expect any response beyond documenting the limitation in perlapi, but
at least they ought to do that.

2. A slightly cleaner fix for this should be to duplicate the SV and
then release the copy around the SvPVutf8 call, only if it's readonly.
"Fixing" it in do_util_elog is entirely the wrong thing.

3. Perl_croak inside a PG_TRY block fails quite nastily. I think we
might be well advised to move the sv2cstr(msg) call outside the PG_TRY,
but I'm wondering whether there is not a more general structural problem
in plperl concerning nesting of PG and Perl error recovery.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 22:48:07
Message-ID: 4F04D727.5090604@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 05:05 PM, Tom Lane wrote:
>
>> Well, the crash is apparently solved by the following, which your
>> investigation suggested to me:
>> - cmsg = sv2cstr(msg);
>> + cmsg = sv2cstr(newSVsv(msg));
> That's kinda grotty ... and leaky ...
>

Of course it is. It wasn't meant as a solution but as validation of your
suspicions about the nature of the problem. (The leakiness could be
solved, though.)

cheers

andrew


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-04 23:15:01
Message-ID: 8F0C5426-FC43-46E7-935A-BC0E50E268B2@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 4, 2012, at 2:48 PM, Andrew Dunstan wrote:

>> That's kinda grotty ... and leaky ...
>>
>
> Of course it is. It wasn't meant as a solution but as validation of your suspicions about the nature of the problem. (The leakiness could be solved, though.)

From #p5p on irc.perl.org:

[10:58pm]dg:interesting, so SvPV() handles string overloading, but SvPVutf8() doesn't, yet: "Like C<SvPV>, but converts sv to utf8 first if necessary."
[10:58pm]dg:oh, only for readonly objects
[10:58pm]dg:probably why no-one has noticed, as version is probably the only readonly thing with string overloading
[11:08pm]TonyC:it doesn't need string overloading
[11:09pm]TonyC:https://gist.github.com/1562734
[11:12pm]TonyC:theory: using sv_mortalcopy() instead of newSVsv() should prevent the leak in that workaround, assuming there's no FREETMPS between the call and use of the return value

Useful?

David


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 01:10:09
Message-ID: 4F04F871.40502@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 06:15 PM, David E. Wheeler wrote:
> [11:12pm]TonyC:theory: using sv_mortalcopy() instead of newSVsv()
> should prevent the leak in that workaround, assuming there's no
> FREETMPS between the call and use of the return value

That's the solution to leakiness I had in mind.

Tom said:

> 2. A slightly cleaner fix for this should be to duplicate the SV and
> then release the copy around the SvPVutf8 call, only if it's readonly.
> "Fixing" it in do_util_elog is entirely the wrong thing.

How do we tell if it's readonly?

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 01:32:20
Message-ID: 2779.1325727140@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom said:
>> 2. A slightly cleaner fix for this should be to duplicate the SV and
>> then release the copy around the SvPVutf8 call, only if it's readonly.
>> "Fixing" it in do_util_elog is entirely the wrong thing.

> How do we tell if it's readonly?

SvREADONLY(sv) macro.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 14:42:58
Message-ID: 4F05B6F2.1000000@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/04/2012 08:32 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> Tom said:
>>> 2. A slightly cleaner fix for this should be to duplicate the SV and
>>> then release the copy around the SvPVutf8 call, only if it's readonly.
>>> "Fixing" it in do_util_elog is entirely the wrong thing.
>> How do we tell if it's readonly?
> SvREADONLY(sv) macro.
>
>

OK, so this seems to fix The issue David found:

diff --git a/src/pl/plperl/plperl_helpers.h b/src/pl/plperl/plperl_helpers.h
index ac0a97d..f0e1afa 100644
--- a/src/pl/plperl/plperl_helpers.h
+++ b/src/pl/plperl/plperl_helpers.h
@@ -51,7 +51,10 @@ sv2cstr(SV *sv)
/*
* get a utf8 encoded char * out of perl. *note* it may not be
valid utf8!
*/
- val = SvPVutf8(sv, len);
+ if (SvREADONLY(sv))
+ val = SvPVutf8(sv_mortalcopy(sv), len);
+ else
+ val = SvPVutf8(sv, len);

/*
* we use perls length in the event we had an embedded null byte to
ensure

but it doesn't fix the one I found which passes a typeglob to elog():

do '$foo=1; elog(NOTICE,*foo);' language plperl;

That still crashes, but doesn't if we use sv_mortalcopy unconditionally.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 15:34:26
Message-ID: 14808.1325777666@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 01/04/2012 08:32 PM, Tom Lane wrote:
>> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>>> How do we tell if it's readonly?
>> SvREADONLY(sv) macro.

> but it doesn't fix the one I found which passes a typeglob to elog():
> do '$foo=1; elog(NOTICE,*foo);' language plperl;

Mmm, I was wondering about that one.

> That still crashes, but doesn't if we use sv_mortalcopy unconditionally.

Unconditional sv_mortalcopy sounds like the thing to do then, but a
comment would help. And if this isn't a Perl bug, I would like to
know what is.

BTW, shouldn't we be making some attempt to drop the result of the
sv_mortalcopy? Or is it just assumed that it will be garbage-collected
before the memory leak gets too bad?

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:08:09
Message-ID: 4F05D8F9.3010909@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/05/2012 10:34 AM, Tom Lane wrote:
>
> Unconditional sv_mortalcopy sounds like the thing to do then, but a
> comment would help. And if this isn't a Perl bug, I would like to
> know what is.

Indeed. David, can you file a bug on this?

>
> BTW, shouldn't we be making some attempt to drop the result of the
> sv_mortalcopy? Or is it just assumed that it will be garbage-collected
> before the memory leak gets too bad?
>
>

Yes, it will be garbage collected before long. See discussion in perldoc
perlguts.

cheers

andrew


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:17:42
Message-ID: ECDE3804-D66D-4304-BA2A-748EE02B2122@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 5, 2012, at 9:08 AM, Andrew Dunstan wrote:

>> Unconditional sv_mortalcopy sounds like the thing to do then, but a
>> comment would help. And if this isn't a Perl bug, I would like to
>> know what is.
>
> Indeed. David, can you file a bug on this?

I could, but don’t know what to say, and wouldn’t be able to answer any questions intelligently. I suggest you or Tom file one. Just run `perlbug` and follow the prompts. I’m sure p5p would appreciate a good bug report that I would not be able to send.

Best,

David


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:28:47
Message-ID: 6E866333-1A67-4639-B744-9C423DD8EAB0@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 5, 2012, at 7:34 AM, Tom Lane wrote:

>> That still crashes, but doesn't if we use sv_mortalcopy unconditionally.
>
> Unconditional sv_mortalcopy sounds like the thing to do then, but a
> comment would help. And if this isn't a Perl bug, I would like to
> know what is.

Question: Is this an issue anywhere else in PL/Perl, or just elog()? What about SPI parameters or return values?

david=# DO LANGUAGE PLPERL $$
david$# my $plan = spi_prepare('SELECT $1', 'TEXT');
david$# spi_query_prepared($plan, $^V);
david$# spi_freeplan($plan);
david$# return;
david$# $$;
ERROR: cannot convert Perl hash to non-composite type text at line 3.
CONTEXT: PL/Perl anonymous code block

No segfault, at least, though that’s a rather bizarre error message. AFAIK, $^V isn’t a hash. This works, though:

DO LANGUAGE PLPERL $$
my $plan = spi_prepare('SELECT $1', 'TEXT');
spi_query_prepared($plan, v1);
spi_freeplan($plan);
return;
$$;
DO

Best,

David


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:41:28
Message-ID: 504.1325785288@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

"David E. Wheeler" <david(at)justatheory(dot)com> writes:
> On Jan 5, 2012, at 7:34 AM, Tom Lane wrote:
>> Unconditional sv_mortalcopy sounds like the thing to do then, but a
>> comment would help. And if this isn't a Perl bug, I would like to
>> know what is.

> Question: Is this an issue anywhere else in PL/Perl, or just elog()?

I would imagine you could reproduce it by returning the same kinds of
objects as function results, since the actual problem is in utf8 to
database-encoding conversion.

> No segfault, at least, though thats a rather bizarre error message. AFAIK, $^V isnt a hash. This works, though:
> spi_query_prepared($plan, v1);

Is that actually a vstring? I confess I'd never heard of the things
before this thread, but I remember reading somewhere that you need
multiple dots in a string before it's considered a vstring and not
something else.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:50:14
Message-ID: 4F05E2D6.2060305@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/05/2012 12:28 PM, David E. Wheeler wrote:
> On Jan 5, 2012, at 7:34 AM, Tom Lane wrote:
>
>>> That still crashes, but doesn't if we use sv_mortalcopy unconditionally.
>> Unconditional sv_mortalcopy sounds like the thing to do then, but a
>> comment would help. And if this isn't a Perl bug, I would like to
>> know what is.
> Question: Is this an issue anywhere else in PL/Perl, or just elog()? What about SPI parameters or return values?

The fix that has been applied, as Tom suggested, is at the point where
we call SvPVutf8(), so that's not just for elog().

>
> david=# DO LANGUAGE PLPERL $$
> david$# my $plan = spi_prepare('SELECT $1', 'TEXT');
> david$# spi_query_prepared($plan, $^V);
> david$# spi_freeplan($plan);
> david$# return;
> david$# $$;
> ERROR: cannot convert Perl hash to non-composite type text at line 3.
> CONTEXT: PL/Perl anonymous code block
>
> No segfault, at least, though that’s a rather bizarre error message. AFAIK, $^V isn’t a hash. This works, though:

As documented, it's not a scalar, and you need to stop treating it as
one. If you want it as a scalar, which is what you'd need here, enclose
it in quotes, or use the stuff shown in perldoc perlvar.

cheers

andrew


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:51:19
Message-ID: 6A6DE41F-234A-4D4A-A095-C0E4D454C915@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 5, 2012, at 9:41 AM, Tom Lane wrote:

> I would imagine you could reproduce it by returning the same kinds of
> objects as function results, since the actual problem is in utf8 to
> database-encoding conversion.
>
>> No segfault, at least, though that‚s a rather bizarre error message. AFAIK, $^V isn‚t a hash. This works, though:
>> spi_query_prepared($plan, v1);
>
> Is that actually a vstring? I confess I'd never heard of the things
> before this thread, but I remember reading somewhere that you need
> multiple dots in a string before it's considered a vstring and not
> something else.

Yes, it is. You can declare v-strings either by prepending a "v" to one or more dot-separated integers, or just 3 or more dotted integers.

http://austin.pm.org/presentations/march2000/raty/vstrings.html

I believe the latter syntax is deprecated, though; it turned out to be confusing and pretty roundly hated. It's preferred to use the v syntax.

Best,

David


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:53:40
Message-ID: F026D4D1-56B0-4F6C-AAC1-3B803612A7C6@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 5, 2012, at 9:50 AM, Andrew Dunstan wrote:

> The fix that has been applied, as Tom suggested, is at the point where we call SvPVutf8(), so that's not just for elog().

Great, thanks.

> As documented, it's not a scalar, and you need to stop treating it as one. If you want it as a scalar, which is what you'd need here, enclose it in quotes, or use the stuff shown in perldoc perlvar.

Oh, right $^V is a version object.

Best,

David


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 17:55:15
Message-ID: 4F05E403.9060702@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/05/2012 12:41 PM, Tom Lane wrote:
> Is that actually a vstring? I confess I'd never heard of the things
> before this thread, but I remember reading somewhere that you need
> multiple dots in a string before it's considered a vstring and not
> something else.
>

perldoc perlvar says:

The revision, version, and subversion of the Perl interpreter,
represented as a "version" object.

This variable first appeared in perl 5.6.0; earlier versions of perl
will see an undefined value. Before perl 5.10.0 $^V was represented
as a v-string.

I'm quite sure David isn't running on something older than 5.10.0.

cheers

andrew


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 18:08:50
Message-ID: 4F05E732.5060705@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/05/2012 12:17 PM, David E. Wheeler wrote:
> On Jan 5, 2012, at 9:08 AM, Andrew Dunstan wrote:
>
>>> Unconditional sv_mortalcopy sounds like the thing to do then, but a
>>> comment would help. And if this isn't a Perl bug, I would like to
>>> know what is.
>> Indeed. David, can you file a bug on this?
> I could, but don’t know what to say, and wouldn’t be able to answer any questions intelligently. I suggest you or Tom file one. Just run `perlbug` and follow the prompts. I’m sure p5p would appreciate a good bug report that I would not be able to send.
>

OK, done.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 18:09:14
Message-ID: 3447.1325786954@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The fix that has been applied, as Tom suggested, is at the point where
> we call SvPVutf8(), so that's not just for elog().

That patch looks good, but do we want to look into the other point about
error recovery? The real bottom line here is that when SvPVutf8 threw a
croak(), we didn't recover sanely at all --- in fact, it was almost
impossible to tell what had happened even in gdb, until I installed
perl debugging symbols and single-stepped through things. I would have
hoped that we'd at least see the croak message. This seems a bit
nervous-making, as plperl sure calls a lot of stuff that in principle
could croak.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "David E(dot) Wheeler" <david(at)justatheory(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 18:13:45
Message-ID: 4F05E859.1000701@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 01/05/2012 01:09 PM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> The fix that has been applied, as Tom suggested, is at the point where
>> we call SvPVutf8(), so that's not just for elog().
> That patch looks good, but do we want to look into the other point about
> error recovery? The real bottom line here is that when SvPVutf8 threw a
> croak(), we didn't recover sanely at all --- in fact, it was almost
> impossible to tell what had happened even in gdb, until I installed
> perl debugging symbols and single-stepped through things. I would have
> hoped that we'd at least see the croak message. This seems a bit
> nervous-making, as plperl sure calls a lot of stuff that in principle
> could croak.
>
>

Yes, I think so. That seemed like a separate issue, though, which is why
I just applied the workaround to the reported problem. I had similar
issues with gdb - it wasn't a pleasant experience.

cheers

andrew


From: "David E(dot) Wheeler" <david(at)justatheory(dot)com>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PL/Perl Does not Like vstrings
Date: 2012-01-05 21:47:58
Message-ID: 50B66A85-40B8-47C4-AE1D-054F23C46A40@justatheory.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Jan 5, 2012, at 9:55 AM, Andrew Dunstan wrote:

> perldoc perlvar says:
>
> The revision, version, and subversion of the Perl interpreter,
> represented as a "version" object.
>
> This variable first appeared in perl 5.6.0; earlier versions of perl
> will see an undefined value. Before perl 5.10.0 $^V was represented
> as a v-string.
>
> I'm quite sure David isn't running on something older than 5.10.0.

Perl 5.14.2:

> perl -E 'say ref $^V'
version

Perl 5.12.4:

> perl -E 'say ref $^V'
version

Perl 5.10.1:

> perl -E 'say ref $^V'
version

Perl 5.8.9:

> perl -le 'print ref $^V'

Best,

David