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

Storable -- segmentation fault #15125

Closed
p5pRT opened this issue Jan 10, 2016 · 14 comments
Closed

Storable -- segmentation fault #15125

p5pRT opened this issue Jan 10, 2016 · 14 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 10, 2016

Migrated from rt.perl.org#127232 (status was 'rejected')

Searchable as RT127232$

@p5pRT
Copy link
Author

p5pRT commented Jan 10, 2016

From mthurn@cpan.org

Created by mthurn@cpan.org

When my object contains a child JSON object,
Storable causes a segmentation fault at the end of my program,
after all Perl code has finished executing (even my END blocks).
The bug only manifests if the stored object has been retrieved from disk.
Here is a short program that demonstrates​:

package Foo;
use base 'Storable';
sub new { bless {}, 'Foo' }

package main;
use JSON;
my $o = new Foo;
$o->{item} = new JSON;
$o->store('foo2.store');
use Storable;
my $ro = retrieve('foo2.store');
__END__

Perl Info

Flags:
    category=library
    severity=medium
    module=Storable

This perlbug was built using Perl 5.10.1 in the Fedora build system.
It is being executed now by Perl 5.10.1 - Tue Nov 10 12:04:36 UTC 2015.

Site configuration information for perl 5.10.1:

Configured by Red Hat, Inc. at Tue Nov 10 12:04:36 UTC 2015.

Summary of my perl5 (revision 5 version 10 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=2.6.32-220.el6.x86_64, archname=x86_64-linux-thread-multi
    uname='linux c6b8.bsys.dev.centos.org 2.6.32-220.el6.x86_64 #1 smp tue dec 6 19:48:22 gmt 2011 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 
-m64 -mtune=generic -DDEBUGGING=-g -Dversion=5.10.1 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. 
-Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivli
b=/usr/share/perl5 -Darchlib=/usr/lib64/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Dvendorarch=/usr/lib64/perl5/vendor_perl -Din
c_version_list=5.10.0 -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Dus
eithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinst
allusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -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 -Dscriptdir=/usr/bin -Dusesite
customize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOU
RCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.7 20120313 (Red Hat 4.4.7-16)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -fstack-protector'
    libpth=/usr/local/lib64 /lib64 /usr/lib64
    libs=-lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lresolv -lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.12'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/lib64/perl5/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buff
er-size=4 -m64 -mtune=generic'

Locally applied patches:
    


@INC for perl 5.10.1:
    /usr/local/lib64/perl5
    /usr/local/share/perl5
    /usr/lib64/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib64/perl5
    /usr/share/perl5
    .


Environment for perl 5.10.1:
    HOME=/home/martin
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/usr/local/apache-maven-3.3.3/bin:/usr/lib64/qt-3.3/bin:/usr/local/apache-maven-3.3.3/bin:/usr/local/bin:/usr/bin:/bin:/usr/
local/sbin:/usr/sbin:/sbin:/opt/gradle/latest/bin:/home/martin/bin:/opt/gradle/latest/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jan 10, 2016

From @jkeenan

On Sun Jan 10 12​:19​:23 2016, mthurn@​cpan.org wrote​:

This is a bug report for perl from mthurn@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.10.1 on
CentOS 6.7.

-----------------------------------------------------------------
[Please describe your issue here]

When my object contains a child JSON object,
Storable causes a segmentation fault at the end of my program,
after all Perl code has finished executing (even my END blocks).
The bug only manifests if the stored object has been retrieved from
disk.
Here is a short program that demonstrates​:

package Foo;
use base 'Storable';
sub new { bless {}, 'Foo' }

package main;
use JSON;
my $o = new Foo;
$o->{item} = new JSON;
$o->store('foo2.store');
use Storable;
my $ro = retrieve('foo2.store');
__END__

1. What is the version of Storable at which you experienced this failure?

2. Note that perl-5.10.1 was released in 2007 and is long out of support. The versions of Perl which are currently supported are 5.22 and 5.20. In addition, the version of Storable which accompanied the perl-5.10.1 was 2.20; the version of Storable released with perl-5.22.0 was 2.53. So both Perl and Storable have progressed considerably since then.

3. That being said, I was able to reproduce the segmentation fault with perl-5.22.0, Storable 2.53 and JSON 2.90.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jan 10, 2016

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

@p5pRT
Copy link
Author

p5pRT commented Jan 11, 2016

From @shlomif

On Sun Jan 10 15​:41​:17 2016, jkeenan wrote​:

On Sun Jan 10 12​:19​:23 2016, mthurn@​cpan.org wrote​:

This is a bug report for perl from mthurn@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.10.1 on
CentOS 6.7.

-----------------------------------------------------------------
[Please describe your issue here]

When my object contains a child JSON object,
Storable causes a segmentation fault at the end of my program,
after all Perl code has finished executing (even my END blocks).
The bug only manifests if the stored object has been retrieved from
disk.
Here is a short program that demonstrates​:

package Foo;
use base 'Storable';
sub new { bless {}, 'Foo' }

package main;
use JSON;
my $o = new Foo;
$o->{item} = new JSON;
$o->store('foo2.store');
use Storable;
my $ro = retrieve('foo2.store');
__END__

1. What is the version of Storable at which you experienced this
failure?

2. Note that perl-5.10.1 was released in 2007 and is long out of
support. The versions of Perl which are currently supported are 5.22
and 5.20. In addition, the version of Storable which accompanied the
perl-5.10.1 was 2.20; the version of Storable released with perl-
5.22.0 was 2.53. So both Perl and Storable have progressed
considerably since then.

3. That being said, I was able to reproduce the segmentation fault
with perl-5.22.0, Storable 2.53 and JSON 2.90.

And I was able to reproduce the segfault with bleadperl commit 5dcc841 and the latest JSON​::MaybeXS and Cpanel​::JSON​::XS from CPAN​:

<<<<<<<<<<<
package Foo;
use base 'Storable';
sub new { bless {}, 'Foo' }

package main;
use JSON​::MaybeXS;
my $o = Foo->new;
$o->{item} = JSON->new;
$o->store('foo2.store');
use Storable;
my $ro = retrieve('foo2.store');

@p5pRT
Copy link
Author

p5pRT commented Jan 11, 2016

From @arc

Martin Thurn <perlbug-followup@​perl.org> wrote​:

When my object contains a child JSON object,
Storable causes a segmentation fault at the end of my program,
after all Perl code has finished executing (even my END blocks).
The bug only manifests if the stored object has been retrieved from disk.

I think this needs JSON​::XS or Cpanel​::JSON​::XS installed to trigger.
Here's a reduction that demonstrates the same behaviour​:

use Cpanel​::JSON​::XS;
use Storable qw<freeze thaw>;
thaw( freeze( Cpanel​::JSON​::XS->new ) );

But I don't think this is a bug that can be fixed in anything that p5p
maintains. As far as I can tell, what's happening is as follows.

First, an instance of Cpanel​::JSON​::XS is created. Under the hood, the
XS parts of that module allocate an internal C structure to serve as
the guts of that instance, and populate it appropriately. The instance
returned does contain the relevant guts, but in a way that's hidden
from callers who don't know where to look for it.

This instance is then frozen and re-thawed using Storable. Storable
has no way to know about the hidden guts, so it reconstructs an object
that it thinks is a copy of the original, but in fact doesn't contain
the hidden guts.

Then the thawed instance is discarded, so its destructor gets run. The
destructor is also in XS, but it doesn't notice that the hidden guts
are absent. When the destructor tries to look at the hidden guts, this
causes a segfault.

I believe the only way to fix this is for Cpanel​::JSON​::XS and
JSON​::XS to implement Storable's hooks for freezing and thawing class
instances​:

https://metacpan.org/pod/Storable#Hooks

In addition, it's possible that it would be worth those modules being
a little more defensive about the instances they're given (though I
admit I haven't thought about this in detail). For example, a patch
along these lines could be applied to Cpanel​::JSON​::XS​:

Inline Patch
diff --git i/typemap w/typemap
index 66d2b4a..e001065 100644
--- i/typemap
+++ w/typemap
@@ -10,6 +10,9 @@ T_JSON
            && (SvSTASH (SvRV ($arg)) == JSON_STASH || sv_derived_from
($arg, \"Cpanel::JSON::XS\"))   \)\)   croak \(\\"object is not of type Cpanel​::JSON​::XS\\"\); \+ if \(SvSTASH \(SvRV \($arg\)\) == JSON\_STASH \+ && SvLEN \(SvRV \($arg\)\) \< sizeof\(JSON\)\) \+ croak \(\\"object is masquerading as a Cpanel​::JSON​::XS\\"\);   /\*\*/   $var = \(JSON \*\)SvPVX \(SvRV \($arg\)\);

Regardless, I think these changes should be taken up with the
maintainers of the modules in question. Cpanel​::JSON​::XS uses Github
issues​:

https://github.com/rurban/Cpanel-JSON-XS/issues

and the author of JSON​::XS prefers reports to be emailed directly​:

https://metacpan.org/pod/JSON::XS#BUGS

Thanks.

--
Aaron Crane ** http​://aaroncrane.co.uk/

@p5pRT p5pRT closed this as completed Jan 11, 2016
@p5pRT
Copy link
Author

p5pRT commented Jan 11, 2016

@arc - Status changed from 'open' to 'rejected'

@p5pRT
Copy link
Author

p5pRT commented Jan 13, 2016

From kingpin@dcswcc.org

This is probably moot given the email traffic I've seen today, but FWIW
this is Storable 2.20

- - Martin

On Sun, Jan 10, 2016 at 6​:41 PM, James E Keenan via RT <
perlbug-followup@​perl.org> wrote​:

On Sun Jan 10 12​:19​:23 2016, mthurn@​cpan.org wrote​:

This is a bug report for perl from mthurn@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.10.1 on
CentOS 6.7.

-----------------------------------------------------------------
[Please describe your issue here]

When my object contains a child JSON object,
Storable causes a segmentation fault at the end of my program,
after all Perl code has finished executing (even my END blocks).
The bug only manifests if the stored object has been retrieved from
disk.
Here is a short program that demonstrates​:

package Foo;
use base 'Storable';
sub new { bless {}, 'Foo' }

package main;
use JSON;
my $o = new Foo;
$o->{item} = new JSON;
$o->store('foo2.store');
use Storable;
my $ro = retrieve('foo2.store');
__END__

1. What is the version of Storable at which you experienced this failure?

2. Note that perl-5.10.1 was released in 2007 and is long out of support.
The versions of Perl which are currently supported are 5.22 and 5.20. In
addition, the version of Storable which accompanied the perl-5.10.1 was
2.20; the version of Storable released with perl-5.22.0 was 2.53. So both
Perl and Storable have progressed considerably since then.

3. That being said, I was able to reproduce the segmentation fault with
perl-5.22.0, Storable 2.53 and JSON 2.90.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2016

From bkb@cpan.org

On Mon Jan 11 10​:28​:38 2016, arc wrote​:

I think this needs JSON​::XS or Cpanel​::JSON​::XS installed to trigger.
Here's a reduction that demonstrates the same behaviour​:

use Cpanel​::JSON​::XS;
use Storable qw<freeze thaw>;
thaw( freeze( Cpanel​::JSON​::XS->new ) );

It doesn't require JSON​::XS or Cpanel​::JSON​::XS to trigger. Any XS module with a blessed object will do, this has nothing whatsoever to do with Cpanel​::JSON​::XS, JSON​::XS, or any other module except Storable. For example​:

  use JSON​::Create;
  thaw( freeze( JSON​::Create->new ) );
  $ perl storable.pl
  ./json-create-perl.c​:1496​: n_mallocs = -1
  Segmentation fault (core dumped)

But I don't think this is a bug that can be fixed in anything that p5p
maintains. As far as I can tell, what's happening is as follows.

First, an instance of Cpanel​::JSON​::XS is created. Under the hood, the
XS parts of that module allocate an internal C structure to serve as
the guts of that instance, and populate it appropriately. The instance
returned does contain the relevant guts, but in a way that's hidden
from callers who don't know where to look for it.

This is called encapsulation of objects.

This instance is then frozen and re-thawed using Storable. Storable
has no way to know about the hidden guts, so it reconstructs an object
that it thinks is a copy of the original, but in fact doesn't contain
the hidden guts.

Storable breaks the encapsulation of the object, then it copies that into a new object, then it blesses the copy into the original class, and you're suggesting that the author of JSON​::XS or Cpanel​::JSON​::XS should do something about that chain of events?

Then the thawed instance is discarded, so its destructor gets run. The
destructor is also in XS, but it doesn't notice that the hidden guts
are absent. When the destructor tries to look at the hidden guts, this
causes a segfault.

All of which is an exceedingly good example of why it's not a great idea to break object encapsulation of Perl objects, then bless copies of the fake object got by coping the broken encapsulation into the class of the original object. I don't know exactly what kind of structure is used by JSON​::XS, but typically it would be a pointer into memory which is represented as an IV within Perl.

In addition, it's possible that it would be worth those modules being
a little more defensive about the instances they're given (though I
admit I haven't thought about this in detail).

The authors of JSON​::XS and Cpanel​::JSON​::XS, and any other XS module should not be forced to defensively program against this kind of misbehaviour by Storable. There is no possible way that a C programmer, given a pointer into random memory (the IV from Perl), can check that it is a valid instance of anything whatsoever, since that always involves accessing the memory, and there is no guarantee that even accessing the memory is possible.

Regardless, I think these changes should be taken up with the
maintainers of the modules in question.

Storable is the module which should be "in question" here. JSON​::XS and Cpanel​::JSON​::XS are not misbehaving at all and do not need to be modified. The misbehaviour is to break object encapsulation and then bless the broken copy of the object into a class it doesn't belong to.

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2016

From @bulk88

I am not respond to any particular post in this ticket.

There are 4 ways to store C resources (pointers or self-alloced-C structs) in Perl.

Pointers only, in a SVIV
Pointers and C structs, in a SVPV that is 4, 8 or more (self alloced C structs) bytes long, but filled with binary gibberish ("packed pointers")
Pointers and C structs, in a MG struct, completely unreachable from PP, cons, must search the linked list of magic each time in an XSUB

#4 is rare, inside out objects with the pointer or C struct stored with one of the above methods in a global perl var, serializing the object is impossible with inside out objects because they are indexed by HV*s which change between perl process runs

There is a number 5 way but such an object is insane.

Pointers only, in the XSANY of a XSUB CV *, your object is reference to a blessed (X) subroutine, dtor is free magic attached to the CV *. I dont think Storable can deparse an XSUB and recreate it.

--
bulk88 ~ bulk88 at hotmail.com

@p5pRT
Copy link
Author

p5pRT commented Jan 26, 2016

From @arc

Ben Bullock via RT <perlbug-followup@​perl.org> wrote​:

On Mon Jan 11 10​:28​:38 2016, arc wrote​:

I think this needs JSON​::XS or Cpanel​::JSON​::XS installed to trigger.

It doesn't require JSON​::XS or Cpanel​::JSON​::XS to trigger.

I meant that the original reporter's test case yields no segfault in a
freshly-installed Perl with the JSON module (and no others) installed
from from CPAN.

Any XS module with a blessed object will do,

Well, I dare say there are XS modules with blessed objects to which
this doesn't apply, but I certainly agree that there are plenty of XS
modules which would be affected in the same way.

Storable breaks the encapsulation of the object, then it copies that
into a new object, then it blesses the copy into the original class,
and you're suggesting that the author of JSON​::XS or Cpanel​::JSON​::XS
should do something about that chain of events?

I'm suggesting that, if someone wants to be able to use Storable on
objects that it can't natively dump, something somewhere will have to
implement the available Storable hooks to make that possible.
Furthermore, I think that Storable itself is not the right place to do
that​: if the same analysis were applied to all affected modules,
Storable would grow without bound in the long run.

The authors of JSON​::XS and Cpanel​::JSON​::XS, and any other XS
module should not be forced to defensively program against this kind
of misbehaviour by Storable.

Your position that Storable breaks encapsulation by trying to dump and
restore arbitrary objects is not without merit, but I can't see any
way that this can be changed while preserving its existing behaviour
at all. Or perhaps there's something I'm missing; what sort of change
to Storable would you suggest?

--
Aaron Crane ** http​://aaroncrane.co.uk/

@p5pRT
Copy link
Author

p5pRT commented Jan 27, 2016

From bkb@cpan.org

On Tue Jan 26 12​:44​:52 2016, arc wrote​:

I'm suggesting that, if someone wants to be able to use Storable on
objects that it can't natively dump, something somewhere will have to
implement the available Storable hooks to make that possible.
Furthermore, I think that Storable itself is not the right place to do
that​: if the same analysis were applied to all affected modules,
Storable would grow without bound in the long run.

As bulk88 points out in his very informative post there are a number of different ways of making an object, but the method using IVs as described here​:

http​://www.lemoda.net/perl/perl-xs-object/index.html

will inevitably lead to errors of this kind. Unless you can demonstrate that the above method of blessing a T_PTROBJ is illegal somehow according to the Perl specifications it seems to me that Storable should not be doing what it is doing.

It's possible for modules to defensively program against being sent bad objects, for example by containing a per-module hash table with random "safety" keys and values with the actual "dangerous" pointer values, then using the "safe" random keys as the data within the blessed objects, and checking every input object with the table and rejecting unknown keys. Then at least the segmentation fault error could be avoided. However, it's not clear to me why all XS module programmers should be forced to do this for the benefit of a module which breaks encapsulation and blesses a random pointer into the class without using the module's constructor.

My opinion is that the user who wants to serialize those objects should be responsible for serializing them, something like this​:

https://metacpan.org/pod/distribution/JSON-Create/lib/JSON/Create.pod#obj
https://metacpan.org/pod/distribution/JSON-Create/lib/JSON/Create.pod#obj_handler

The authors of JSON​::XS and Cpanel​::JSON​::XS, and any other XS
module should not be forced to defensively program against this kind
of misbehaviour by Storable.

Your position that Storable breaks encapsulation by trying to dump and
restore arbitrary objects is not without merit, but I can't see any
way that this can be changed while preserving its existing behaviour
at all. Or perhaps there's something I'm missing; what sort of change
to Storable would you suggest?

The part which causes the segmentation fault is blessing the copy of the object into the class JSON​::XS within Storable rather than the breaking encapsulation part, so I would suggest either not blessing objects into classes or giving the user responsibility for object serialization as described above. The method I described at the top, of making "secret" objects hidden behind a hash of random keys could just as easily be applied to a pure Perl module which would break Storable in a similar way, although without the segmentation fault errors.

@p5pRT
Copy link
Author

p5pRT commented Apr 24, 2016

From @arc

My sincere apologies for the very long delay in this reply.

Ben Bullock via RT <perlbug-followup@​perl.org> wrote​:

As bulk88 points out in his very informative post there are a number of different ways of making an object, but the method using IVs as described here​:

http​://www.lemoda.net/perl/perl-xs-object/index.html

will inevitably lead to errors of this kind. Unless you can demonstrate that the above method of blessing a T_PTROBJ is illegal somehow according to the Perl specifications it seems to me that Storable should not be doing what it is doing.

For the record, I certainly don't think that T_PTROBJ and related
implementation techniques are invalid or inappropriate.

The part which causes the segmentation fault is blessing the copy of the object into the class JSON​::XS within Storable rather than the breaking encapsulation part, so I would suggest either not blessing objects into classes or giving the user responsibility for object serialization as described above. The method I described at the top, of making "secret" objects hidden behind a hash of random keys could just as easily be applied to a pure Perl module which would break Storable in a similar way, although without the segmentation fault errors.

This could certainly be made to work. However, I don't think it's
possible to change Storable's default behaviour here. Most Perl
classes are simple blessed hash refs, with all their data readily
visible to Storable; and users therefore expect to be able to use
Storable to store and retrieve such objects today, without doing
anything special. Changing this would certainly break large parts of
Storable's test suite, and my very strong expectation is that it would
also break large amounts of extant code that uses Storable.

Storable does already call classes' STORABLE_freeze and STORABLE_thaw
hooks when they're available (which I think counts as "giving the user
responsibility for object serialization" as you suggest). Would your
needs be met by giving Storable an option to have it refuse to
serialise and deserialise blessed references for classes without such
hooks? This would at least let code calling Storable opt in to
enforced delegation of responsibility for serialisation and
deserialisation.

--
Aaron Crane ** http​://aaroncrane.co.uk/

@p5pRT
Copy link
Author

p5pRT commented Apr 25, 2016

From @demerphq

On 24 April 2016 at 13​:58, Aaron Crane <arc@​cpan.org> wrote​:

My sincere apologies for the very long delay in this reply.

Ben Bullock via RT <perlbug-followup@​perl.org> wrote​:

As bulk88 points out in his very informative post there are a number of different ways of making an object, but the method using IVs as described here​:

http​://www.lemoda.net/perl/perl-xs-object/index.html

will inevitably lead to errors of this kind. Unless you can demonstrate that the above method of blessing a T_PTROBJ is illegal somehow according to the Perl specifications it seems to me that Storable should not be doing what it is doing.

For the record, I certainly don't think that T_PTROBJ and related
implementation techniques are invalid or inappropriate.

The part which causes the segmentation fault is blessing the copy of the object into the class JSON​::XS within Storable rather than the breaking encapsulation part, so I would suggest either not blessing objects into classes or giving the user responsibility for object serialization as described above. The method I described at the top, of making "secret" objects hidden behind a hash of random keys could just as easily be applied to a pure Perl module which would break Storable in a similar way, although without the segmentation fault errors.

This could certainly be made to work. However, I don't think it's
possible to change Storable's default behaviour here. Most Perl
classes are simple blessed hash refs, with all their data readily
visible to Storable; and users therefore expect to be able to use
Storable to store and retrieve such objects today, without doing
anything special. Changing this would certainly break large parts of
Storable's test suite, and my very strong expectation is that it would
also break large amounts of extant code that uses Storable.

Storable does already call classes' STORABLE_freeze and STORABLE_thaw
hooks when they're available (which I think counts as "giving the user
responsibility for object serialization" as you suggest). Would your
needs be met by giving Storable an option to have it refuse to
serialise and deserialise blessed references for classes without such
hooks? This would at least let code calling Storable opt in to
enforced delegation of responsibility for serialisation and
deserialisation.

As a serialization author I would love a reliable way to tell if an
object/class is based around T_PTROBJ and similar constructs.

For instance, imagine every namespace with XS code in it was
registered as "HAS_XS". A serializer module could then choose how to
handle objects blessed into such a namespace. (Yes I know in practice
this is not going to happen, but imagine...)

Arguably this should be fixed by having a better way to create XS
objects that do not leak pointers into perl space, or allow one to use
perl code to create XS objects that point at arbitrary memory. I have
no idea how we would do it though.

Yves

--
perl -Mre=debug -e "/just|another|perl|hacker/"

@p5pRT
Copy link
Author

p5pRT commented Apr 25, 2016

From zefram@fysh.org

demerphq wrote​:

Arguably this should be fixed by having a better way to create XS
objects that do not leak pointers into perl space,

Already available​: "magic" annotations can be attached to any Perl value,
and are by default invisible to pure Perl code. Extracting the C pointer
is a smidgen slower than for T_PTROBJ, though. A serialiser can see
that there is magic that it doesn't understand, and barf or whatever.

-zefram

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