Closed p5pRT closed 15 years ago
JSON::XS and other XS modules fail with assertions of the form:
t/02_error................perl: XS.xs:1418: decode_json: Assertion `!((((_svi)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((_svi)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((_svi)->sv_flags & 0xff)) == SVt_PVLV))' failed.
JSON::XS-2.1 tests clean with perl-5.8.8 latest and perl-5.10.0-33733.
JSON::XS-2.2 tests clean with perl-5.8.8 latest and fails with all of perl-5.10.0-33733 and other old patch levels I have laying around\, back through several 336xx series patches.
On Wednesday 23 April 2008 15:32:31 david@davidfavor.com wrote:
# New Ticket Created by david@davidfavor.com # Please include the string: [perl #53244] # in the subject line of all future correspondence about this issue. # \<URL: http://rt.perl.org/rt3/Ticket/Display.html?id=53244 >
This is a bug report for perl from david@davidfavor.com\, generated with the help of perlbug 1.36 running under perl 5.10.0.
----------------------------------------------------------------- [Please enter your report here]
JSON::XS and other XS modules fail with assertions of the form:
t/02\_error\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.perl​: XS\.xs​:1418​: decode\_json​: Assertion \`\!\(\(\(\(\_svi\)\->sv\_flags & \(0x00004000|0x00008000\)\) ==
0x00008000) && (((svtype)((_svi)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((_svi)->sv_flags & 0xff)) == SVt_PVLV))' failed.
JSON::XS-2.1 tests clean with perl-5.8.8 latest and perl-5.10.0-33733.
JSON::XS-2.2 tests clean with perl-5.8.8 latest and fails with all of perl-5.10.0-33733 and other old patch levels I have laying around\, back through several 336xx series patches.
I am unsure what you now expect us (or somebody) to do. It looks like there was something broken between post-5.8.8 and pre-5.10-33733\, and thus the bug is already fixed.
Did I misunderstood something?
All the best\,
Tels
SHELL=/bin/bash
-- Signed on Wed Apr 23 19:39:53 2008 with key 0x93B84C15. Get one of my photo posters: http://bloodgate.com/posters PGP key on http://bloodgate.com/tels.asc or per email.
"Proctor & Gamble unveiled a new soap this week. Although it looks normal\, the soap is actually hollow\, which eliminates those little pieces that are always left at the end."
-- SNL
The RT System itself - Status changed from 'new' to 'open'
On Wed\, Apr 23\, 2008 at 06:32:31AM -0700\, david @ davidfavor. com wrote:
JSON::XS and other XS modules fail with assertions of the form:
t/02\_error\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.\.perl​: XS\.xs​:1418​: decode\_json​: Assertion \`\!\(\(\(\(\_svi\)\->sv\_flags & \(0x00004000|0x00008000\)\) == 0x00008000\) && \(\(\(svtype\)\(\(\_svi\)\->sv\_flags & 0xff\)\) == SVt\_PVGV || \(\(svtype\)\(\(\_svi\)\->sv\_flags & 0xff\)\) == SVt\_PVLV\)\)' failed\.
JSON::XS-2.1 tests clean with perl-5.8.8 latest and perl-5.10.0-33733.
JSON::XS-2.2 tests clean with perl-5.8.8 latest and fails with all of perl-5.10.0-33733 and other old patch levels I have laying around\, back through several 336xx series patches.
I'm not quite sure why you consider a module that was released *after* 5.10.0 that has a test failure to be a bug in 5.10.0\, given that the version of the module at the time of the release passed.
The assertion failure is due to a change in decode_json\, from
if (json->flags & F_MAXSIZE && SvCUR (string) > DEC_SIZE (json->flags)) croak ("attempted decode of JSON text of %lu bytes size\, but max_size is set to %lu"\, (unsigned long)SvCUR (string)\, (unsigned long)DEC_SIZE (json->flags));
to
if (SvCUR (string) > json->max_size && json->max_size) croak ("attempted decode of JSON text of %lu bytes size\, but max_size is set to %lu"\, (unsigned long)SvCUR (string)\, (unsigned long)json->max_size);
It can be fixed with the appended patch.
You mention "other XS modules". Which other XS modules?
Nicholas Clark
On Fri\, Apr 25\, 2008 at 02:58:37PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
I'm not quite sure why you consider a module that was released *after* 5.10.0 that has a test failure to be a bug in 5.10.0\, given that the version of the module at the time of the release passed.
Because the module works fine with 5.10.0\, so it is a regression?
The assertion failure is due to a change in decode_json\, from
Both versions use SvCUR\, so this change doesn't cause it\, the problem was either there all the time or not there at all.
So earlier versions of the module would have run into the same problem with post-5.10.0 code.
You mention "other XS modules". Which other XS modules?
I saw that\, too\, but can't remember which ones it was. Since it doesn't happen with any release version of perl I didn't think much about it.
Also\, if SvCUR no longer works safely on scalars and I have to resort to undocumented behaviour (Checks the private setting. Use "SvPOK".) then I would expect this issue to be much more common.
I am happy to check any private flags\, of course\, but since this was never documented\, I am pretty sure many moduels will fail.
-- The choice of a Deliantra\, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / pcg@goof.com -=====/_/_//_/\_\,_/ /_/\_\
On Fri\, Apr 25\, 2008 at 02:58:37PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
It can be fixed with the appended patch.
This is especially troubling as the code goes to the expense of upgrading to SVt_PV before. If XS code cannot use SvCUR anymore after upgrading to SVt_PV then I would consider this a serious regression.
At least the new requirement of having to check a private flag (which isn't explained anywhere) before calling SVCUR even on pv's should be clearly documented as a change in behaviour.
-- The choice of a Deliantra\, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / pcg@goof.com -=====/_/_//_/\_\,_/ /_/\_\
On Fri\, 25 Apr 2008\, Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 02:58:37PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
It can be fixed with the appended patch.
This is especially troubling as the code goes to the expense of upgrading to SVt_PV before. If XS code cannot use SvCUR anymore after upgrading to SVt_PV then I would consider this a serious regression.
I don't think you should rely on SvCUR() to have any particular value if SvPOK() isn't true. I don't think this was ever guaranteed.
At least the new requirement of having to check a private flag (which isn't explained anywhere) before calling SVCUR even on pv's should be clearly documented as a change in behaviour.
I don't think a CPAN module should _ever_ check the private flags. They are for core Perl usage only. At least that's how it used to be. Isn't checking SvPOK() before accessing SvCUR() doing the right thing for you?
Cheers\, -Jan
On Fri\, Apr 25\, 2008 at 05:20:48PM -0700\, Jan Dubois \jand@​activestate\.com wrote:
I don't think a CPAN module should _ever_ check the private flags. They are for core Perl usage only. At least that's how it used to be. Isn't checking SvPOK() before accessing SvCUR() doing the right thing for you?
upgrading to a PV was always doing the right thing for me. The code in question seems to dot he right thing\, too\, in all perl releases I have tried\, including 5.10.
I also don't really think that SvPOK must be true to acecss SvCUR of a PV. Doesn't make much sense to me.
If\, of course\, this is now required\, I will update my modules accordingly\, but this is definitely a new limitation (i.e. you cnanot call SvCUR on a PV anymore).
-- The choice of a Deliantra\, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / pcg@goof.com -=====/_/_//_/\_\,_/ /_/\_\
On Fri\, 25 Apr 2008\, Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 05:20:48PM -0700\, Jan Dubois \jand@​activestate\.com wrote:
I don't think a CPAN module should _ever_ check the private flags. They are for core Perl usage only. At least that's how it used to be. Isn't checking SvPOK() before accessing SvCUR() doing the right thing for you?
upgrading to a PV was always doing the right thing for me. The code in question seems to dot he right thing\, too\, in all perl releases I have tried\, including 5.10.
I also don't really think that SvPOK must be true to acecss SvCUR of a PV. Doesn't make much sense to me.
What is the meaning of SvCUR() if your SV is just SvROK() and the PVX slot contains a reference to another SV instead of a string buffer?
perl -MDevel::Peek -e "$a='foo'; $a=\$b; Dump $a" SV = PV(0x226f24) at 0x182a21c REFCNT = 1 FLAGS = (ROK) RV = 0x182a32c SV = NULL(0x0) at 0x182a32c REFCNT = 2 FLAGS = () PV = 0x182a32c "" CUR = 0 LEN = 0
SvCUR() is supposed to return the "length of the string in the SV"\, but when SvPOK() is not set\, then there is no string in the SV\, so the concept of a current length doesn't make any sense. What do you expect the semantics of SvCUR() to be in this case? Note that SvCUR() is not actually the length of the string if you would print the SV\, which would print " SCALAR(0x182a32c)" and not the empty string "".
The SvLEN() field however does have a meaning\, even when SvPOK() isn't set: when SvLEN() is non-zero\, then the buffer pointed to by PV must be freed using Sysfree() when the SV refcount goes to 0 (and that a buffer of SvLEN() bytes is already available for you if you want to turn SvPOK() on).
Cheers\, -Jan
On Fri\, Apr 25\, 2008 at 10:42:57PM -0700\, Jan Dubois \jand@​activestate\.com wrote:
What is the meaning of SvCUR() if your SV is just SvROK() and the PVX slot contains a reference to another SV instead of a string buffer?
It gives me CUR:
perl -MDevel::Peek -e "$a='foo'; $a=\$b; Dump $a" CUR = 0
SvCUR() is supposed to return the "length of the string in the SV"\, but when SvPOK() is not set\, then there is no string in the SV\, so the concept of a current length doesn't make any sense.
Well\, PV's can contain some memory pointer\, a length and the currently used length of this memory range.
Lots of modules upgrade to pv\, grow to get some memory\, and only then set svpok for example.
expect the semantics of SvCUR() to be in this case? Note that SvCUR()
Just give me CUR.
is not actually the length of the string if you would print the SV\, which would print " SCALAR(0x182a32c)" and not the empty string "".
You said there is no string in the pv\, so how can it suddenly have a length? :)
In any case\, what I was pointing out that this simply was a regression. As I said\, when perl changes the meaning of SvCUR from "accesses CUR" to "asserts unless some private flag is set"\, then I will happily oblige.
I don't think this is a singular case\, though\, and I see no wrong ina cecssing the CUR slot of a PV when it exists.
-- The choice of a Deliantra\, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / pcg@goof.com -=====/_/_//_/\_\,_/ /_/\_\
Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 05:20:48PM -0700\, Jan Dubois \jand@​activestate\.com wrote:
I don't think a CPAN module should _ever_ check the private flags. They are for core Perl usage only. At least that's how it used to be. Isn't checking SvPOK() before accessing SvCUR() doing the right thing for you?
upgrading to a PV was always doing the right thing for me. The code in question seems to dot he right thing\, too\, in all perl releases I have tried\, including 5.10.
I also don't really think that SvPOK must be true to acecss SvCUR of a PV. Doesn't make much sense to me.
Marc\,
A couple of things - it was pointed out on IRC that the builds have differing -DDEBUGGING build options\, and that on the other versions\, the assertions which are being raised now might simply not be there.
The documentation that describes that a string must be POK before you read SvCUR of it is in perlguts. Don't forget than a SV is a union of the various SV types\, and that you need to check the bits to see what the meaning of the subsequent members are. Without SvPOK\, you're interpreting a field that is probably not what you expect. Nicholas' use of the private member is I think a red herring - but I'm not entirely sure on that.
However I think there is more to this than that - I'm also a bit confused as to why the sv_upgrade doesn't set the appropriate bits. Is it not just that the debugging version of the SvCUR macro is checking the wrong bits?
Sam.
On Fri\, Apr 25\, 2008 at 05:20:48PM -0700\, Jan Dubois wrote:
On Fri\, 25 Apr 2008\, Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 02:58:37PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
It can be fixed with the appended patch.
This is especially troubling as the code goes to the expense of upgrading to SVt_PV before. If XS code cannot use SvCUR anymore after upgrading to SVt_PV then I would consider this a serious regression.
I don't think you should rely on SvCUR() to have any particular value if SvPOK() isn't true. I don't think this was ever guaranteed.
At least the new requirement of having to check a private flag (which isn't explained anywhere) before calling SVCUR even on pv's should be clearly documented as a change in behaviour.
I don't think a CPAN module should _ever_ check the private flags. They are for core Perl usage only. At least that's how it used to be. Isn't checking SvPOK() before accessing SvCUR() doing the right thing for you?
I believe that it is necessary if one wants to be polymorphic based on the type of value\, if the value has magic\, such as if it's tainted.
This stepping through Perl_sv_setsv_flags for perl -T -e '$a = $^X'
3448 if (SvGMAGICAL(sstr) && (flags & SV_GMAGIC)) { (gdb) 3449 mg_get(sstr); (gdb) n 3450 if (SvTYPE(sstr) != stype) { (gdb) call Perl_sv_dump(my_perl\, sstr) SV = PVMG(0x810240) at 0x800e70 REFCNT = 2 FLAGS = (GMG\,SMG\,pPOK) IV = 0 NV = 0 PV = 0x201be0 "/Volumes/Stuff/p4perl/maint-5.8/perl/perl"\0 CUR = 41 LEN = 44 MAGIC = 0x201c10 MG_VIRTUAL = &PL_vtbl_taint MG_TYPE = PERL_MAGIC_taint(t) MG_LEN = 1
SvPOK(sstr) is never going to be true\, even after the mg_get().
Nicholas Clark
On Sat\, Apr 26\, 2008 at 01:52:17AM +0200\, Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 02:58:37PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
I'm not quite sure why you consider a module that was released *after* 5.10.0 that has a test failure to be a bug in 5.10.0\, given that the version of the module at the time of the release passed.
Because the module works fine with 5.10.0\, so it is a regression?
It's not a regression. The same code was present in 5.10.0 release (and in fact my test was done with a fresh build of 5.10.0 downloaded from CPAN.
The assertions are only enabled if perl is built with the C pre-processor macro DEBUGGING defined\, which in turn is enabled automatically by Configure if $optimize contains -g. (None of that has changed for years)
What did change was that I added quite a few assertions in various macros catch code that uses the macros when they're not valid. It's never been enforced before\, but because I was making quite a few changes to the SV layout (to save lots of memory)\, I wanted to be sure that code wasn't doing things that were going to break once 5.10 came out. The changes are annotated here:
http://public.activestate.com/cgi-bin/perlbrowse?filename=sv.h&show_blame=Show+Annotated+File
Specific changes to SvCUR() were made with changes 27328 and 29219\, which date from Feburary 2006 and November 2006 respectively. So they've been present for a long time\, and were useful as part of Andreas' smoking of CPAN modules with blead in the years leading up to 5.10.0
Nicholas Clark
On Sat\, Apr 26\, 2008 at 08:01:42AM +0200\, Marc Lehmann wrote:
On Fri\, Apr 25\, 2008 at 10:42:57PM -0700\, Jan Dubois \jand@​activestate\.com wrote:
What is the meaning of SvCUR() if your SV is just SvROK() and the PVX slot contains a reference to another SV instead of a string buffer?
It gives me CUR:
perl -MDevel::Peek -e "$a='foo'; $a=\$b; Dump $a" CUR = 0
SvCUR() is supposed to return the "length of the string in the SV"\, but when SvPOK() is not set\, then there is no string in the SV\, so the concept of a current length doesn't make any sense.
Well\, PV's can contain some memory pointer\, a length and the currently used length of this memory range.
Lots of modules upgrade to pv\, grow to get some memory\, and only then set svpok for example.
expect the semantics of SvCUR() to be in this case? Note that SvCUR()
Just give me CUR.
That's not what SvCUR() is about. It's direct access to the structure. If you want the length\, you need sv_len()
is not actually the length of the string if you would print the SV\, which would print " SCALAR(0x182a32c)" and not the empty string "".
You said there is no string in the pv\, so how can it suddenly have a length? :)
In any case\, what I was pointing out that this simply was a regression. As I said\, when perl changes the meaning of SvCUR from "accesses CUR" to "asserts unless some private flag is set"\, then I will happily oblige.
The documentation certainly needs correcting.
I don't think this is a singular case\, though\, and I see no wrong ina cecssing the CUR slot of a PV when it exists.
But it won't always contain a value that bears any relation to the length that SvPV() and variants can return. For example:
$ perl -MDevel::Peek -e '$a = "Long"; $a = 4; Dump $a' SV = PVIV(0x802020) at 0x800bb8 REFCNT = 1 FLAGS = (IOK\,pIOK) IV = 4 PV = 0x201ad0 "Long"\0 CUR = 4 LEN = 8
or with overloaded references:
$ cat ~/test/overload.pl #!perl -w use strict;
package String;
use overload '""'\, sub { ${$_[0]} };
package main;
use Devel::Peek;
my $ref = ""; $ref = \do {my $a = "***Text***"; $a}; bless $ref\, "String";
Dump $ref;
print "$ref\n";
Dump $ref;
__END__ $ perl ~/test/overload.pl SV = PV(0x801060) at 0x800c0c REFCNT = 1 FLAGS = (PADBUSY\,PADMY\,ROK\,OVERLOAD) RV = 0x800168 SV = PVMG(0x80ba60) at 0x800168 REFCNT = 1 FLAGS = (OBJECT\,POK\,pPOK) IV = 0 NV = 0 PV = 0x207270 "***Text***"\0 CUR = 10 LEN = 12 STASH = 0x800bac "String" PV = 0x800168 "" CUR = 0 LEN = 0 ***Text*** SV = PV(0x801060) at 0x800c0c REFCNT = 1 FLAGS = (PADBUSY\,PADMY\,ROK\,OVERLOAD) RV = 0x800168 SV = PVMG(0x80ba60) at 0x800168 REFCNT = 1 FLAGS = (OBJECT\,POK\,pPOK) IV = 0 NV = 0 PV = 0x207270 "***Text***"\0 CUR = 10 LEN = 12 STASH = 0x800bac "String" PV = 0x800168 "" CUR = 0 LEN = 0
(both those are 5.8.8)
Nicholas Clark
On Fri\, May 09\, 2008 at 11:04:44PM +0100\, Nicholas Clark \nick@​ccl4\.org wrote:
Because the module works fine with 5.10.0\, so it is a regression?
It's not a regression.
I agree. It's breaking a working module when -g was specified at an unopportune place.
The assertions are only enabled if perl is built with the C pre-processor macro DEBUGGING defined\, which in turn is enabled automatically by Configure if $optimize contains -g. (None of that has changed for years)
I didn't know\, and\, sorry to say so\, but after compiler vendors worked so very hard to guarentee that "-g" doesn't change the semantics of some code I think it is a very bad idea for perl to do so\, especially when it breaks programs.
But then\, I have personally no issue with that (fortunately\, I alwyys added -g to ccflags\, not optimize) :)
catch code that uses the macros when they're not valid. It's never been enforced before\, but because I was making quite a few changes to the SV layout (to save lots of memory)\, I wanted to be sure that code wasn't doing
While we are at it\, I cannot reproduce most of those memory savings. While perldelta says\, for example:
"use POSIX;" now takes about 200K less memory.
I actually get an _additional_ 0.4MB memory use for use POSIX\, compared to 5.8.8\, both built with identical settings:
TTY STAT TIME MAJFL TRS DRS RSS %MEM COMMAND pts/0 S+ 0:00 0 1626 19309 1364 0.0 perl5.10.0 -esleep 60 pts/0 S+ 0:00 0 1626 24717 3420 0.0 perl5.10.0 -MPOSIX -esleep 60 pts/0 S+ 0:00 0 10 24893 1536 0.0 perl5.8.8 -esleep 60 pts/0 S+ 0:00 2 10 28377 3216 0.0 perl5.8.8 -MPOSIX -esleep 60
Note that perl without any modules is indeed smaller.
And this is not related to POSIX: (almost) any module I load uses more ram with 5.10 then with an identically built 5.8.8.
things that were going to break once 5.10 came out.
Apparently this was a failure (but good work\, and a good idea).
The changes are annotated here:
http://public.activestate.com/cgi-bin/perlbrowse?filename=sv.h&show_blame=Show+Annotated+File
I must admit I don't understand where to get the annotations from that page (but I don't have to\, so its not important). Do you mean the comments that get extrated into perlapi as annotations?
Specific changes to SvCUR() were made with changes 27328 and 29219\, which date from Feburary 2006 and November 2006 respectively. So they've been present for a long time\, and were useful as part of Andreas' smoking of CPAN modules with blead in the years leading up to 5.10.0
Not sure what the message here is. Lots of code in perl is very old\, that has little relevance to its correctness or usefulness. Or wether it breaks code when not compiled in by default.
I must admit\, your mail confused me.
-- The choice of a Deliantra\, the free code+content MORPG -----==- _GNU_ http://www.deliantra.net ----==-- _ generation ---==---(_)__ __ ____ __ Marc Lehmann --==---/ / _ \/ // /\ \/ / pcg@goof.com -=====/_/_//_/\_\,_/ /_/\_\
It looks like this argument wasn't actually a bug. Resolving.
@obra - Status changed from 'open' to 'resolved'
Migrated from rt.perl.org#53244 (status was 'resolved')
Searchable as RT53244$