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

blead: local $_ doesn't mix with @_ => Modification of a read-only value attempted #6765

Closed
p5pRT opened this issue Sep 13, 2003 · 13 comments
Closed

Comments

@p5pRT
Copy link

p5pRT commented Sep 13, 2003

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

Searchable as RT23803$

@p5pRT
Copy link
Author

p5pRT commented Sep 13, 2003

From stas@stason.org

This is a bug report for perl from stas@​stason.org,
generated with the help of perlbug 1.34 running under perl v5.9.0.


This script​:

foo("bbb");
sub foo {
  for (@​_) {
  for my $key (1..2) {
  local $_ = 'x';
  s/(.*)/+$1/;
  }
  print;
  }
}

generates​:

Modification of a read-only value attempted at test line 7

under blead@​21198. It works fine with 5.8.1@​21184 and lower.

This problem breaks the mod_perl 1.0 build.



Flags​:
  category=core
  severity=high


Site configuration information for perl v5.9.0​:

Configured by stas at Fri Sep 12 14​:42​:18 PDT 2003.

Summary of my perl5 (revision 5.0 version 9 subversion 0 patch 21198)
configuration​:
  Platform​:
  osname=linux, osvers=2.4.21-0.18mdkcustom, archname=i686-linux
  uname='linux rabbit.stason.org 2.4.21-0.18mdkcustom #6 mon jun 16
16​:26​:34 est 2003 i686 unknown unknown gnulinux '
  config_args='-des -Dprefix=/home/stas/perl/blead -Doptimize=-g
-Duseshrplib -Dusedevel -DDEBUG_LEAKING_SCALARS'
  hint=recommended, useposix=true, d_sigaction=define
  usethreads=undef useithreads=undef usemultiplicity=undef
  useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
  use64bitint=undef use64bitall=undef uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -I/usr/include/gdbm',
  optimize='-g',
  cppflags='-DDEBUGGING -fno-strict-aliasing -I/usr/local/include
-I/usr/include/gdbm'
  ccversion='', gccversion='3.3.1 (Mandrake Linux 9.2 3.3.1-1mdk)',
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='cc', ldflags =' -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so
  gnulibc_version='2.3.2'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic
-Wl,-rpath,/home/stas/perl/blead/lib/5.9.0/i686-linux/CORE'
  cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:
  DEVEL20173


@​INC for perl v5.9.0​:
  /home/stas/perl/blead/lib/5.9.0/i686-linux
  /home/stas/perl/blead/lib/5.9.0
  /home/stas/perl/blead/lib/site_perl/5.9.0/i686-linux
  /home/stas/perl/blead/lib/site_perl/5.9.0
  /home/stas/perl/blead/lib/site_perl
  .


Environment for perl v5.9.0​:
  HOME=/home/stas
  LANG=en_AU
  LANGUAGE=en_AU​:en_US​:en
  LC_ADDRESS=en_AU
  LC_COLLATE=en_AU
  LC_CTYPE=en_AU
  LC_IDENTIFICATION=en_AU
  LC_MEASUREMENT=en_AU
  LC_MESSAGES=en_AU
  LC_MONETARY=en_AU
  LC_NAME=en_AU
  LC_NUMERIC=en_AU
  LC_PAPER=en_AU
  LC_TELEPHONE=en_AU
  LC_TIME=en_AU
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)

PATH=/usr//bin​:/bin​:/usr/bin​:.​:/usr/local/bin​:/usr/X11R6/bin​:/usr/games​:/home/stas/bin​:/home/stas/bin​:/usr/local/bin​:/usr/X11R6/bin​:/usr/java/j2re1.4.0/bin/
  PERLDOC_PAGER=less -R
  PERL_BADLANG (unset)
  SHELL=/bin/tcsh

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 14, 2003

From @andk

On 13 Sep 2003 20​:28​:20 -0000, Stas Bekman (via RT) <perlbug-followup@​perl.org> said​:

Blame 20479.

Note that replacing line 6 with C< s/x/y/ > works.

----Program----
foo("bbb");
sub foo {
  for (@​_) {
  for my $key (1..2) {
  local $_ = 'x';
  s/(.*)/+$1/;
  }
  print;
  }
}

----Output of .../prIMRVC/perl-5.8.0@​20478/bin/perl----
bbb
----EOF ($?='0')----
----Output of .../psrxuv0/perl-5.8.0@​20479/bin/perl----
Modification of a read-only value attempted at tests/stas-modireadonly.pl line 5.

----EOF ($?='65280')----

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Sep 14, 2003

From stas@stason.org

Andreas J Koenig wrote​:

On 13 Sep 2003 20​:28​:20 -0000, Stas Bekman (via RT) <perlbug-followup@​perl.org> said​:

Blame 20479.

Thanks Andreas. So is it a feature or a bug?

Change 20479 by rgs@​rgs-home on 2003/08/04 20​:26​:17

  Fix bug #23141 : localization of readonly magic scalars
  now produces an error "Modification of a read-only value
  attempted", instead of silently failing.

it doesn't happen in maint perl, Rafael?

Note that replacing line 6 with C< s/x/y/ > works.

----Program----
foo("bbb");
sub foo {
for (@​_) {
for my $key (1..2) {
local $_ = 'x';
s/(.*)/+$1/;
}
print;
}
}

----Output of .../prIMRVC/perl-5.8.0@​20478/bin/perl----
bbb
----EOF ($?='0')----
----Output of .../psrxuv0/perl-5.8.0@​20479/bin/perl----
Modification of a read-only value attempted at tests/stas-modireadonly.pl line 5.

----EOF ($?='65280')----

--

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 14, 2003

From @rgs

Stas Bekman wrote​:

Andreas J Koenig wrote​:

Blame 20479.

Thanks Andreas. So is it a feature or a bug?

Change 20479 by rgs@​rgs-home on 2003/08/04 20​:26​:17

Fix bug \#23141 : localization of readonly magic scalars
now produces an error "Modification of a read\-only value
attempted"\, instead of silently failing\.

See <http​://bugs6.perl.org/rt2/Ticket/Display.html?id=23141>.

it doesn't happen in maint perl, Rafael?

This kind of change is not suitable for a maint branch. Jarkko didn't
integrate it.

Note that replacing line 6 with C< s/x/y/ > works.

----Program----
foo("bbb");
sub foo {
for (@​_) {
for my $key (1..2) {
local $_ = 'x';
s/(.*)/+$1/;

Interesting. This substitution sets 'g' magic on $_. And localisation
(and unlocalisation) don't touch magic, so $_ (which is aliased to the
readonly $_[0]) keeps this magic.

Then, at the 2nd iteration of the loop, the localisation fails, as the
patch intended it.

The obvious workaround is to iterate over a copy of @​_. This foo()
function had the spurious side-effect to set 'g' magic on its arguments.
This kind of bug might be serious enough to justify my original patch
(see the discussion about bug #23141.)

However, s/// adding magic to its argument is probably suboptimal
behaviour. A cursory inspection of the sources shows that 'g' magic is
often special-cased, for example in mg_dup() and sv_unmagic(). Moreover
the only action implemented by this magic is to remove the "study()ied"
flag when the scalar is set.

     \}
     print;
 \}

}

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From stas@stason.org

Rafael Garcia-Suarez wrote​:

Stas Bekman wrote​:

Andreas J Koenig wrote​:

Blame 20479.

Thanks Andreas. So is it a feature or a bug?

Change 20479 by rgs@​rgs-home on 2003/08/04 20​:26​:17

Fix bug #23141 : localization of readonly magic scalars
now produces an error "Modification of a read-only value
attempted", instead of silently failing.

See <http​://bugs6.perl.org/rt2/Ticket/Display.html?id=23141>.

it doesn't happen in maint perl, Rafael?

This kind of change is not suitable for a maint branch. Jarkko didn't
integrate it.

Note that replacing line 6 with C< s/x/y/ > works.

----Program----
foo("bbb");
sub foo {
for (@​_) {
for my $key (1..2) {
local $_ = 'x';
s/(.*)/+$1/;

Interesting. This substitution sets 'g' magic on $_. And localisation
(and unlocalisation) don't touch magic, so $_ (which is aliased to the
readonly $_[0]) keeps this magic.

Then, at the 2nd iteration of the loop, the localisation fails, as the
patch intended it.

The obvious workaround is to iterate over a copy of @​_. This foo()
function had the spurious side-effect to set 'g' magic on its arguments.
This kind of bug might be serious enough to justify my original patch
(see the discussion about bug #23141.)

But it's not a bug in the code I've posted, it's a bug in perl, no? Am I
wrong? Sure, I can workaround it, but is it documented somewhere that it's
illegal to localize @​_? I admit I haven't checked but I'm not sure where it
could be documented.

However, s/// adding magic to its argument is probably suboptimal
behaviour. A cursory inspection of the sources shows that 'g' magic is
often special-cased, for example in mg_dup() and sv_unmagic(). Moreover
the only action implemented by this magic is to remove the "study()ied"
flag when the scalar is set.

Sorry, I lost you in the guts :(

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From @rgarcia

Stas Bekman wrote​:

----Program----
foo("bbb");
sub foo {
for (@​_) {
for my $key (1..2) {
local $_ = 'x';
s/(.*)/+$1/;

Interesting. This substitution sets 'g' magic on $_. And localisation
(and unlocalisation) don't touch magic, so $_ (which is aliased to the
readonly $_[0]) keeps this magic.

Then, at the 2nd iteration of the loop, the localisation fails, as the
patch intended it.

The obvious workaround is to iterate over a copy of @​_. This foo()
function had the spurious side-effect to set 'g' magic on its arguments.
This kind of bug might be serious enough to justify my original patch
(see the discussion about bug #23141.)

But it's not a bug in the code I've posted, it's a bug in perl, no? Am I

Let's say it's a suboptimal interference with the guts to the program.

The s/// adds some magic to "bbb". My patch introduced the fact that
readonlyness of magical scalars is perserved when localisation occurs.
This is intended to prevent against obscure bugs that occur when
modifying a magical scalar that doesn't have a set method : it's state
may be corrupt, or the change be ignored, as the following exaple
demonstrates​:

  $ perl5.8.0 -le '1=~/(.)/;for($1){local$_=2;print}'
  1

This lead to obscure behaviour when using dark magic, like in your case.
To fix this, I'll have to investigate how 'g' magic works. This is not
entirely clear to me now--

wrong? Sure, I can workaround it, but is it documented somewhere that it's
illegal to localize @​_? I admit I haven't checked but I'm not sure where it
could be documented.

No, it's the modification of the localised $_ that fails here. Not the
local().

HTH.

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From stas@stason.org

Rafael Garcia-Suarez wrote​:

Stas Bekman wrote​:

----Program----
foo("bbb");
sub foo {
for (@​_) {
for my $key (1..2) {
local $_ = 'x';
s/(.*)/+$1/;

Interesting. This substitution sets 'g' magic on $_. And localisation
(and unlocalisation) don't touch magic, so $_ (which is aliased to the
readonly $_[0]) keeps this magic.

Then, at the 2nd iteration of the loop, the localisation fails, as the
patch intended it.

The obvious workaround is to iterate over a copy of @​_. This foo()
function had the spurious side-effect to set 'g' magic on its arguments.
This kind of bug might be serious enough to justify my original patch
(see the discussion about bug #23141.)

But it's not a bug in the code I've posted, it's a bug in perl, no? Am I

Let's say it's a suboptimal interference with the guts to the program.

The s/// adds some magic to "bbb". My patch introduced the fact that
readonlyness of magical scalars is perserved when localisation occurs.
This is intended to prevent against obscure bugs that occur when
modifying a magical scalar that doesn't have a set method : it's state
may be corrupt, or the change be ignored, as the following exaple
demonstrates​:

$ perl5\.8\.0 \-le '1=~/\(\.\)/;for\($1\)\{local$\_=2;print\}'
1

This lead to obscure behaviour when using dark magic, like in your case.
To fix this, I'll have to investigate how 'g' magic works. This is not
entirely clear to me now--

wrong? Sure, I can workaround it, but is it documented somewhere that it's
illegal to localize @​_? I admit I haven't checked but I'm not sure where it
could be documented.

No, it's the modification of the localised $_ that fails here. Not the
local().

In other words, what you saying is that your patch alerts about the problem
(something that was quietly failing before) and hopefully someone will fix the
problem itself. Retracting the patch, won't solve the problem. Meanwhile I'd
better use some other variation of that code that doesn't trigger it.

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From @rgarcia

Stas Bekman wrote​:

In other words, what you saying is that your patch alerts about the problem
(something that was quietly failing before) and hopefully someone will fix the
problem itself. Retracting the patch, won't solve the problem.

Yes, it will hide it.

I've been looking at your problem -- I understand why it happens,
now I've to find a way to modify g-magic and/or localisation to
fix it.

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From stas@stason.org

Rafael Garcia-Suarez (via RT) wrote​:

Stas Bekman wrote​:

In other words, what you saying is that your patch alerts about the problem
(something that was quietly failing before) and hopefully someone will fix the
problem itself. Retracting the patch, won't solve the problem.

Yes, it will hide it.

I've been looking at your problem -- I understand why it happens,
now I've to find a way to modify g-magic and/or localisation to
fix it.

ok, so I just need to wait a bit ;) Thanks Rafael!

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From @andk

On Tue, 16 Sep 2003 01​:58​:48 -0700, Stas Bekman <stas@​stason.org> said​:

  > Rafael Garcia-Suarez (via RT) wrote​:

Stas Bekman wrote​:

In other words, what you saying is that your patch alerts about the
problem (something that was quietly failing before) and hopefully
someone will fix the problem itself. Retracting the patch, won't
solve the problem.
Yes, it will hide it.
I've been looking at your problem -- I understand why it happens,
now I've to find a way to modify g-magic and/or localisation to
fix it.

  > ok, so I just need to wait a bit ;) Thanks Rafael!

I'd say, switch to using lexicals instead of $_ and sleep better! Why
not

  use strict;
  foo("bbb");
  sub foo {
  for my $arg (@​_) {
  for my $key (1..2) {
  my $arg = 'x';
  $arg =~ s/(.*)/+$1/;
  print $arg;
  }
  print $arg;
  }
  }

? And while we are at it, why not give the inner $arg a different name
to avoid confusion? OK, I know this is a test case, so may have some
other reason.

I tend to avoid $_ in large programs or modules or any code the gets
distributed.

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Sep 16, 2003

From stas@stason.org

Andreas J Koenig wrote​:

On Tue, 16 Sep 2003 01​:58​:48 -0700, Stas Bekman <stas@​stason.org> said​:

Rafael Garcia-Suarez (via RT) wrote​:

Stas Bekman wrote​:

In other words, what you saying is that your patch alerts about the
problem (something that was quietly failing before) and hopefully
someone will fix the problem itself. Retracting the patch, won't
solve the problem.
Yes, it will hide it.
I've been looking at your problem -- I understand why it happens,
now I've to find a way to modify g-magic and/or localisation to
fix it.

ok, so I just need to wait a bit ;) Thanks Rafael!

I'd say, switch to using lexicals instead of $_ and sleep better! Why
not

use strict;
foo\("bbb"\);
sub foo \{
     for my $arg \(@&#8203;\_\) \{
         for my $key \(1\.\.2\) \{
             my $arg = 'x';
             $arg =~ s/\(\.\*\)/\+$1/;
             print $arg;
         \}
         print $arg;
     \}
\}

? And while we are at it, why not give the inner $arg a different name
to avoid confusion? OK, I know this is a test case, so may have some
other reason.

the real code is in the Apache/ExtUtils.pm's import function of mod_perl 1.0,
currently it doesn't build against blead perl because of that. I have posted a
simple example that reproduces the problem. Sure I'll fix it not to use $_.

__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2003

From @rgs

This is fixed (or worked around) by change #21323 :

When localising a magic value, propagate the readonly flag
only if this scalar has \0 magic or has magic without a
'set' method. (follows change #20479 for bug #23141.)

@p5pRT p5pRT closed this as completed Sep 24, 2003
@p5pRT
Copy link
Author

p5pRT commented Sep 24, 2003

@rgs - Status changed from 'new' to 'resolved'

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

No branches or pull requests

1 participant