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

Readonly hash keys stay readonly after copying #11940

Closed
p5pRT opened this issue Feb 6, 2012 · 11 comments
Closed

Readonly hash keys stay readonly after copying #11940

p5pRT opened this issue Feb 6, 2012 · 11 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Feb 6, 2012

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

Searchable as RT109986$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 6, 2012

From @dmcbride

Created by @dmcbride

I'm using Readonly​::XS, but I don't think this has anything to do
with the problem other than exposing it. Note that this works fine
under perl5.8.8, and I don't see anything in the perl*delta files for
5.10.0 or 5.10.1 to indicate this type of change.

====
#!/usr/bin/perl

package Foo;

my @​readonlies = qw( blah baz );
use Readonly;
use warnings qw(FATAL all);
use strict;

sub new
{
  my $class = shift;
  my $opts = shift;
  my $self = bless {%$opts, _type => $class}, $class;

  #line "RO"
  Readonly $self->{$_} => $self->{$_} for @​readonlies;

  #line "C"
  #$self->{$_}++ for @​readonlies;
  $self;
}

package main;

use Test​::More;
use Test​::Exception;

my %tries = (
  foo1 => {
  foo2 => {
  foo3 => { },
  },
  }
  );

for my $blah (keys %tries)
{
  my $l1 = $tries{$blah};
  for my $baz (keys %$l1)
  {
  my $l2 = $l1->{$baz};
  for my $bar (keys %$l2)
  {
  lives_and {
  # line "O"
  my %opts = (blah => $blah, baz => $baz, bar => $bar);
  ok(!Readonly​::is_sv_readonly($opts{$_}),"$_ is not readonly") for keys %opts;
  my $obj = Foo->new(\%opts);
  ok($obj, "Created object");
  } "creating object";
  }
  }
}

done_testing();

I've marked three lines, "RO", "C", and "O".

In perl 5.8.8, the above works fine (with Readonly​::XS installed).

In perl 5.10.1 (I don't have 5.10.0 to try with) and later, if I quote
$blah and $baz on line "O", and ensure line "C" is commented out, then
everything proceeds okay, and the test returns successful.

If I run the above as-is in 5.10.1, I get Readonly detecting that
$self->{blah} is readonly already, and dies. Since I've copied the
keys a couple of times, I would not expect them to retain their
readonly status.

If I comment out line "RO" but uncomment line "C", I get the failures
from the first ok after line "O", but line "C" succeeds (they're
readonly, but I can modify them?). If they're still marked readonly
(which they shouldn't be), I should not be able to change them. So
perl seems to get the modification right despite the readonly flag
still being there.

If I put $blah and $baz in quotes on line "O", the "RO" line will kick
in and line "C", if uncommented, dies.

I can't explain this behaviour, but it does seem to be wrong.

If this is really a problem in R​::XS, let me know and I'll open that
defect instead.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.10.1:

Configured by dmcbride at Tue Mar 16 23:49:56 MDT 2010.

Summary of my perl5 (revision 5 version 10 subversion 1) configuration:
   
  Platform:
    osname=linux, osvers=2.6.33-gentoo, archname=x86_64-linux-thread-multi
    uname='linux naboo 2.6.33-gentoo #2 smp sun mar 7 20:47:32 mst 2010 x86_64 amd phenom(tm) 9850 quad-core processor authenticamd gnulinux '
    config_args=''
    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='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector'
    ccversion='', gccversion='4.3.4', 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='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64 /usr/local/lib64
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.10.1.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.10.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.10.1:
    /home/dmcbride/perl/5.10.1/lib/5.10.1/x86_64-linux-thread-multi
    /home/dmcbride/perl/5.10.1/lib/5.10.1
    /home/dmcbride/perl/5.10.1/lib/site_perl/5.10.1/x86_64-linux-thread-multi
    /home/dmcbride/perl/5.10.1/lib/site_perl/5.10.1
    .


Environment for perl 5.10.1:
    HOME=/home/dmcbride
    LANG=en_US.utf8
    LANGUAGE=
    LC_ALL=en_US.utf8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/dmcbride/bin:/usr/lib/distcc/bin:/usr/bin:/bin:/opt/bin:/usr/x86_64-pc-linux-gnu/i686-pc-linux-gnu/gcc-bin/4.5.3:/usr/x86_64-pc-linux-gnu/gcc-bin/4.5.3:/share/cvs/bin:/usr/local/bin:/usr/games/bin:/share/bin:/share/darin/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2012

From @jkeenan

On Mon Feb 06 15​:40​:39 2012, dmcbride@​naboo.to.org wrote​:

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

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

I'm using Readonly​::XS, but I don't think this has anything to do
with the problem other than exposing it.

I'm inclined to attribute the problem to Readonly​::XS.

I tested your code on both Linux and Darwin, Perl 5.14.2 in both cases.
I did not have Readonly​::XS installed on either box. I tried several
of the "commented/uncommented" variations you described. In each case
all tests passed.

Then I installed Readonly​::XS on both boxes. I then started to get all
the test failures you reported.

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2012

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2012

From @Leont

On Tue, Feb 7, 2012 at 12​:40 AM, dmcbride@​naboo.to.org
<perlbug-followup@​perl.org> wrote​:

I'm using Readonly​::XS, but I don't think this has anything to do
with the problem other than exposing it.  Note that this works fine
under perl5.8.8, and I don't see anything in the perl*delta files for
5.10.0 or 5.10.1 to indicate this type of change.

It does. Readonly​::is_sv_readonly is redefined when Readonly​::XS is
loaded. In particular it always returns false if R​::XS isn't available

If I run the above as-is in 5.10.1, I get Readonly detecting that
$self->{blah} is readonly already, and dies.  Since I've copied the
keys a couple of times, I would not expect them to retain their
readonly status.

If I comment out line "RO" but uncomment line "C", I get the failures
from the first ok after line "O", but line "C" succeeds (they're
readonly, but I can modify them?).  If they're still marked readonly
(which they shouldn't be), I should not be able to change them.  So
perl seems to get the modification right despite the readonly flag
still being there.

If I put $blah and $baz in quotes on line "O", the "RO" line will kick
in and line "C", if uncommented, dies.

I can't explain this behaviour, but it does seem to be wrong.

You shouldn't be using is_sv_readonly in the first place. Checking
just for the flag without any of the other flags will get you false
positives in a number of situations, including hash keys and
copy-on-write strings.

If this is really a problem in R​::XS, let me know and I'll open that
defect instead.

He's known to ignore bug reports (see the queues for Reaonly and
Readonly​::XS), not sure that's useful.

Leon

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 11, 2012

From @jkeenan

On Sat Feb 11 10​:22​:53 2012, LeonT wrote​:

If this is really a problem in R​::XS, let me know and I'll open that
defect instead.

He's known to ignore bug reports (see the queues for Reaonly and
Readonly​::XS), not sure that's useful.

It may not be useful, but since neither Readonly nor Readonly​::XS is
part of the Perl 5 core distribution, opening a ticket in
https://rt.cpan.org/Dist/Display.html?Queue=Readonly-XS is probably the
correct thing to do.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 12, 2012

From @cpansprout

On Sat Feb 11 08​:54​:21 2012, jkeenan wrote​:

On Mon Feb 06 15​:40​:39 2012, dmcbride@​naboo.to.org wrote​:

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

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

I'm using Readonly​::XS, but I don't think this has anything to do
with the problem other than exposing it.

I'm inclined to attribute the problem to Readonly​::XS.

There are two problems. Readonly​::is_sv_readonly is not a documented
public interface, so anyone relying on is it doing something unreliable.

Secondly, Readonly​::XS itself is using undocumented Perl interfaces.
SvREADONLY doesn’t necessarily indicate that a scalar is read-only. But
it *does* indicate that a POK scalar has a read-only string buffer.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 12, 2012

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

@p5pRT p5pRT closed this Feb 12, 2012
@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 13, 2012

From @dmcbride

On Sunday February 12 2012 2​:27​:01 PM you wrote​:

On Sat Feb 11 08​:54​:21 2012, jkeenan wrote​:

On Mon Feb 06 15​:40​:39 2012, dmcbride@​naboo.to.org wrote​:

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

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

I'm using Readonly​::XS, but I don't think this has anything to do
with the problem other than exposing it.

I'm inclined to attribute the problem to Readonly​::XS.

There are two problems. Readonly​::is_sv_readonly is not a documented
public interface, so anyone relying on is it doing something unreliable.

Yes, I was only using that interface because I looked inside the Readonly
methods to see where I was getting the error, and noticed it was checking
is_sv_readonly and dying if it was already readonly. My original problem was
setting keys in a $self hash as readonly and ending up with it croaking, which
was ... unexpected.

Secondly, Readonly​::XS itself is using undocumented Perl interfaces.
SvREADONLY doesn’t necessarily indicate that a scalar is read-only. But
it *does* indicate that a POK scalar has a read-only string buffer.

So, I suppose the logical question is​: how do we definitively indicate that a
scalar is read-only? Because then Internals​::SvREADONLY is also running into
very similar problems as it uses what looks to me like precisely the same
interface for doing precisely the same thing. If I eliminate Readonly from
the whole scenario, I still get some weird things going on. The test attached
here shows Internals​::SvREADONLY returning "yes" (1) for a value not marked as
readonly elsewhere in the code.

The only difference between Readonly​::XS and Internals​::SvREADONLY that I can
tell is that R​::XS checks if the readonly flag is already set prior to calling
the XS code that calls SvREADONLY_on. The curious bit is how I​::SvR also
tells me that a scalar is readonly, but I can change it...

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 13, 2012

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 14, 2012

From @cpansprout

On Sun Feb 12 22​:47​:35 2012, dmcbride@​cpan.org wrote​:

On Sunday February 12 2012 2​:27​:01 PM you wrote​:

On Sat Feb 11 08​:54​:21 2012, jkeenan wrote​:

On Mon Feb 06 15​:40​:39 2012, dmcbride@​naboo.to.org wrote​:

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

-----------------------------------------------------------------

[Please describe your issue here]

I'm using Readonly​::XS, but I don't think this has anything to
do
with the problem other than exposing it.

I'm inclined to attribute the problem to Readonly​::XS.

There are two problems. Readonly​::is_sv_readonly is not a
documented
public interface, so anyone relying on is it doing something
unreliable.

Yes, I was only using that interface because I looked inside the
Readonly
methods to see where I was getting the error, and noticed it was
checking
is_sv_readonly and dying if it was already readonly. My original
problem was
setting keys in a $self hash as readonly and ending up with it
croaking, which
was ... unexpected.

Secondly, Readonly​::XS itself is using undocumented Perl interfaces.
SvREADONLY doesn’t necessarily indicate that a scalar is read-only.
But
it *does* indicate that a POK scalar has a read-only string buffer.

So, I suppose the logical question is​: how do we definitively indicate
that a
scalar is read-only? Because then Internals​::SvREADONLY is also
running into
very similar problems as it uses what looks to me like precisely the
same
interface for doing precisely the same thing. If I eliminate Readonly
from
the whole scenario, I still get some weird things going on. The test
attached
here shows Internals​::SvREADONLY returning "yes" (1) for a value not
marked as
readonly elsewhere in the code.

The only difference between Readonly​::XS and Internals​::SvREADONLY
that I can
tell is that R​::XS checks if the readonly flag is already set prior to
calling
the XS code that calls SvREADONLY_on. The curious bit is how I​::SvR
also
tells me that a scalar is readonly, but I can change it...

Internals​::SvREADONLY has been fixed in bleadperl, and I believe most of
the fixes were backported to 5.14.2. (It still has trouble with
read-only regexps in 5.14.2.)

If you really need to know whether a variable is read-only in older
versions (read​: all stable releases to date), you can assign to it and
see what happens. :-) Or you can use B.pm to check the flags.

--

Father Chrysostomos

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Feb 14, 2012

From @cpansprout

On Mon Feb 13 16​:14​:35 2012, sprout wrote​:

On Sun Feb 12 22​:47​:35 2012, dmcbride@​cpan.org wrote​:

On Sunday February 12 2012 2​:27​:01 PM you wrote​:

On Sat Feb 11 08​:54​:21 2012, jkeenan wrote​:

On Mon Feb 06 15​:40​:39 2012, dmcbride@​naboo.to.org wrote​:

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

-----------------------------------------------------------------

[Please describe your issue here]

I'm using Readonly​::XS, but I don't think this has anything to
do
with the problem other than exposing it.

I'm inclined to attribute the problem to Readonly​::XS.

There are two problems. Readonly​::is_sv_readonly is not a
documented
public interface, so anyone relying on is it doing something
unreliable.

Yes, I was only using that interface because I looked inside the
Readonly
methods to see where I was getting the error, and noticed it was
checking
is_sv_readonly and dying if it was already readonly. My original
problem was
setting keys in a $self hash as readonly and ending up with it
croaking, which
was ... unexpected.

Secondly, Readonly​::XS itself is using undocumented Perl interfaces.
SvREADONLY doesn’t necessarily indicate that a scalar is read-only.
But
it *does* indicate that a POK scalar has a read-only string buffer.

So, I suppose the logical question is​: how do we definitively indicate
that a
scalar is read-only? Because then Internals​::SvREADONLY is also
running into
very similar problems as it uses what looks to me like precisely the
same
interface for doing precisely the same thing. If I eliminate Readonly
from
the whole scenario, I still get some weird things going on. The test
attached
here shows Internals​::SvREADONLY returning "yes" (1) for a value not
marked as
readonly elsewhere in the code.

The only difference between Readonly​::XS and Internals​::SvREADONLY
that I can
tell is that R​::XS checks if the readonly flag is already set prior to
calling
the XS code that calls SvREADONLY_on. The curious bit is how I​::SvR
also
tells me that a scalar is readonly, but I can change it...

Internals​::SvREADONLY has been fixed in bleadperl, and I believe most of
the fixes were backported to 5.14.2. (It still has trouble with
read-only regexps in 5.14.2.)

If you really need to know whether a variable is read-only in older
versions (read​: all stable releases to date), you can assign to it and
see what happens. :-) Or you can use B.pm to check the flags.

The read-only flag is 0x08000000, which only applies if SVf_FAKE
(0x01000000) is off, or if the scalar is a regexp or glob (reftype \$sv
=~ /^(?​:GLOB|REGEXP)\z/).

--

Father Chrysostomos

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

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.