Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perl-5.10.0-33733 assertion with JSON::XS-2.2 #9300

Closed
p5pRT opened this issue Apr 23, 2008 · 17 comments
Closed

perl-5.10.0-33733 assertion with JSON::XS-2.2 #9300

p5pRT opened this issue Apr 23, 2008 · 17 comments

Comments

@p5pRT
Copy link

p5pRT commented Apr 23, 2008

Migrated from rt.perl.org#53244 (status was 'resolved')

Searchable as RT53244$

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2008

From david@davidfavor.com

Created by david@davidfavor.com

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.

Perl Info

Flags:
     category=library
     severity=critical

Site configuration information for perl 5.10.0:

Configured by David Favor at Tue Apr 22 22:28:31 CDT 2008.

Summary of my perl5 (revision 5 version 10 subversion 0 patch 33733) configuration:
   Platform:
     osname=linux, osvers=2.6.24.3-50.fc8, archname=i686-linux-thread-multi
     uname='linux net1.coolsurf.com 2.6.24.3-50.fc8 #1 smp thu mar 20 14:47:10 edt 2008 i686 i686 
i386 gnulinux '
     config_args='-Dprefix=/common/pkgs/perl-5.10.0-33733 -ders -Dusedevel -Dcf_by=David Favor 
-Dcc=gcc -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
--param=ssp-buffer-size=4 -m32 -fstack-protector -mtune=generic  -Dloclibpth=/usr/local/lib 
-Dlocincpth=/usr/local/include -Duseshrplib -Dusethreads -Duseithreads -Duselargefiles -Dd_dosuid 
-Dd_semctl_semun -Dotherlibdirs=/tools/pmlib:/usr/local/pmlib -DDEBUGGING -Ui_db -Ui_ndbm -Di_gdbm 
-Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly 
-Dpager=/usr/bin/less -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto 
-Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto'
     hint=recommended, useposix=true, d_sigaction=define
     useithreads=define, usemultiplicity=define
     useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
     use64bitint=undef, use64bitall=undef, uselongdouble=undef
     usemymalloc=n, bincompat5005=undef
   Compiler:
     cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING -fno-strict-aliasing -pipe 
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
     optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions --param=ssp-buffer-size=4 
-m32 -fstack-protector -mtune=generic ',
     cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBUGGING -fno-strict-aliasing -pipe 
-I/usr/local/include -I/usr/include/gdbm'
     ccversion='', gccversion='4.1.2 20070925 (Red Hat 4.1.2-33)', gccosandvers=''
     intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
     d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
     ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
     alignbytes=4, prototype=define
   Linker and Libraries:
     ld='gcc', ldflags =' -L/usr/local/lib'
     libpth=/usr/local/lib /lib /usr/lib
     libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
     perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
     libc=/lib/libc-2.7.so, so=so, useshrplib=true, libperl=libperl.so
     gnulibc_version='2.7'
   Dynamic Linking:
     dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E 
-Wl,-rpath,/common/pkgs/perl-5.10.0-33733/lib/5.10.0/i686-linux-thread-multi/CORE'
     cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions 
--param=ssp-buffer-size=4 -m32 -fstack-protector -mtune=generic  -L/usr/local/lib'

Locally applied patches:
     MAINT33535


@INC for perl 5.10.0:
     /common/pkgs/perl-5.10.0-33733/lib/5.10.0/i686-linux-thread-multi
     /common/pkgs/perl-5.10.0-33733/lib/5.10.0
     /common/pkgs/perl-5.10.0-33733/lib/site_perl/5.10.0/i686-linux-thread-multi
     /common/pkgs/perl-5.10.0-33733/lib/site_perl/5.10.0
     /tools/pmlib
     /usr/local/pmlib
     .


Environment for perl 5.10.0:
     HOME=/root
     LANG=en_US
     LANGUAGE (unset)
     LD_LIBRARY_PATH (unset)
     LOGDIR (unset)
 
PATH=/common/tools/bin:/tools/bin:/usr/local/bin:/usr/lib/ccache:/bin:/usr/bin:/root/bin:/common/tools/sbin:/tools/sbin:/usr/local/sbin:/sbin:/usr/sbin:/root/sbin:/common/tools/rhmemtools:/common/tools/oneshot:/common/tools/rhmemconvert
     PERL_BADLANG (unset)
     SHELL=/bin/bash

-- 
Love feeling your best ever, all day, every day?
Click http://RadicalHealth.com for the easy way.

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2008

From nospam-abuse@bloodgate.com

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&#8203;: XS\.xs&#8203;:1418&#8203;: decode\_json&#8203;:
   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

@p5pRT
Copy link
Author

p5pRT commented Apr 23, 2008

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Apr 25, 2008

From @nwc10

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&#8203;: XS\.xs&#8203;:1418&#8203;: decode\_json&#8203;:
   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

Inline Patch
--- XS.xs~	2008-04-05 19:14:48.000000000 +0100
+++ XS.xs	2008-04-25 12:10:22.000000000 +0100
@@ -1415,7 +1415,7 @@
   SvGETMAGIC (string);
   SvUPGRADE (string, SVt_PV);
 
-  if (SvCUR (string) > json->max_size && json->max_size)
+  if (SvPOKp(string) && 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);
 

@p5pRT
Copy link
Author

p5pRT commented Apr 25, 2008

From schmorp@schmorp.de

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
  -=====/_/_//_/\_,_/ /_/\_\

@p5pRT
Copy link
Author

p5pRT commented Apr 25, 2008

From schmorp@schmorp.de

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
  -=====/_/_//_/\_,_/ /_/\_\

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2008

From @jandubois

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

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2008

From schmorp@schmorp.de

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
  -=====/_/_//_/\_,_/ /_/\_\

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2008

From @jandubois

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

@p5pRT
Copy link
Author

p5pRT commented Apr 26, 2008

From schmorp@schmorp.de

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
  -=====/_/_//_/\_,_/ /_/\_\

@p5pRT
Copy link
Author

p5pRT commented Apr 29, 2008

From @samv

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.

@p5pRT
Copy link
Author

p5pRT commented May 9, 2008

From @nwc10

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

@p5pRT
Copy link
Author

p5pRT commented May 9, 2008

From @nwc10

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

@p5pRT
Copy link
Author

p5pRT commented May 9, 2008

From @nwc10

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

@p5pRT
Copy link
Author

p5pRT commented May 10, 2008

From schmorp@schmorp.de

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
  -=====/_/_//_/\_,_/ /_/\_\

@p5pRT
Copy link
Author

p5pRT commented Oct 10, 2009

From @obra

It looks like this argument wasn't actually a bug. Resolving.

@p5pRT
Copy link
Author

p5pRT commented Oct 10, 2009

@obra - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant