local $_ calls STORE when $_ is aliased to a tied hash element

[p5pRT]

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

Searchable as RT84774$

[p5pRT]

From @jandubois

Created by @jandubois

It seems wrong that local() tries to write to its argument when the
main purpose of calling it is to protect the original value.

The following program runs fine when you remove the "local" line​:

$ cat local.pl
use strict;
use warnings;

sub TIEHASH { bless $_[1], $_[0] }
sub FETCH { 42 }
sub STORE { die "readonly" }

tie my %hash, __PACKAGE__, {};

for ($hash{key}) {
  local $_ = 2;
  print "$_\n";
}
$ perl local.pl
readonly at local.pl line 6.
readonly at local.pl line 6.

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl 5.12.3:

Configured by ActiveState at Wed Feb  9 14:44:37 PST 2011.

Summary of my perl5 (revision 5 version 12 subversion 3) configuration:
   
  Platform:
    osname=darwin, osvers=9.8.0, archname=darwin-thread-multi-2level
    uname='darwin kitty 9.8.0 darwin kernel version 9.8.0: wed jul 15 16:55:01 pdt 2009; root:xnu-1228.15.4~1release_i386 i386 '
    config_args='-ders -Dcc=gcc -Dusethreads -Duseithreads -Ud_sigsetjmp -Uinstallusrbinperl -Ulocincpth= -Uloclibpth= -Ud_poll -Ui_poll -Ud_sockatmark -Ud_sockatmarkproto -Ui_libutil -Aldflags=-Wl,-search_paths_first -Alddlflags=-Wl,-search_paths_first -Accflags=-arch ppc -arch i386 -Aldflags=-arch ppc -arch i386 -Alddlflags=-arch ppc -arch i386 -Accflags=-nostdinc -B/Developer/SDKs/MacOSX10.4u.sdk/usr/include/gcc -B/Developer/SDKs/MacOSX10.4u.sdk/usr/lib/gcc -isystem/Developer/SDKs/MacOSX10.4u.sdk/usr/include -F/Developer/SDKs/MacOSX10.4u.sdk/System/Library/Frameworks -mmacosx-version-min=10.4 -Aldflags=-Wl,-syslibroot,/Developer/SDKs/MacOSX10.4u.sdk -mmacosx-version-min=10.4 -Alddlflags=-Wl,-syslibroot,/Developer/SDKs/MacOSX10.4u.sdk -mmacosx-version-min=10.4 -Accflags=-DUSE_SITECUSTOMIZE -Duselargefiles -Accflags=-fno-merge-constants -Dprefix=/usr/local/ActivePerl-5.12 -Dprivlib=/usr/local/ActivePerl-5.12/lib -Darchlib=/usr/local/ActivePerl-5.12/lib -Dsiteprefix=/usr/local/ActivePerl-5.12/site -Dsitelib=/usr/local/ActivePerl-5.12/site/lib -Dsitearch=/usr/local/ActivePerl-5.12/site/lib -Dsed=/usr/bin/sed -Duseshrplib -Dcf_by=ActiveState -Dcf_email=support@ActiveState.com'
    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 ='-fno-common -DPERL_DARWIN -arch i386 -DUSE_SITECUSTOMIZE -fno-merge-constants -fno-strict-aliasing -pipe',
    optimize='-O3',
    cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -arch ppc -nostdinc -B/Developer/SDKs/MacOSX10.4u.sdk/usr/include/gcc -B/Developer/SDKs/MacOSX10.4u.sdk/usr/lib/gcc -isystem/Developer/SDKs/MacOSX10.4u.sdk/usr/include -F/Developer/SDKs/MacOSX10.4u.sdk/System/Library/Frameworks -mmacosx-version-min=10.4 -DUSE_SITECUSTOMIZE -fno-merge-constants -fno-strict-aliasing -pipe'
    ccversion='', gccversion='i686-apple-darwin10-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5664)', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -Wl,-search_paths_first -arch i386'
    libpth=/lib /usr/lib /usr/local/lib
    libs=-ldbm -ldl -lm -lc
    perllibs=-ldl -lm -lc
    libc=, so=dylib, useshrplib=true, libperl=libperl.dylib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -Wl,-search_paths_first -arch i386'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY
    c6fbf28 [perl #71806] perldb does not setup %dbline with the shebang option -d
    1fd8fa4 Add Wolfram Humann to AUTHORS
    f120055 make string-append on win32 100 times faster
    a2a8d15 Define _USE_32BIT_TIME_T for VC6 and VC7
    007cfe1 Don't pretend to support really old VC++ compilers
    6d8f7c9 Get rid of obsolete PerlCRT.dll support
    d956618 Make Term::ReadLine::findConsole fall back to STDIN if /dev/tty can't be opened
    321e50c Escape patch strings before embedding them in patchlevel.h


@INC for perl 5.12.3:
    /Users/jan/Library/ActivePerl-5.12/lib
    /usr/local/ActivePerl-5.12/site/lib
    /usr/local/ActivePerl-5.12/lib
    .


Environment for perl 5.12.3:
    DYLD_LIBRARY_PATH (unset)
    HOME=/Users/jan
    LANG=en_CA.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/Users/jan/bin:/Users/jan/Library/ActivePerl-5.12/bin:/Users/jan/Library/Python/2.7/bin:/usr/local/ActivePerl-5.12/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/X11/bin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

[p5pRT]

From @ikegami

This was fixed and reverted due to backwards compatibility reasons. For
example, it broke File​::chdir. See

See
http​://perl5.git.perl.org/perl.git/commit/89adf4e8e0de17fedf78328b32920f47635850d0

"local *_;" will create a new, untied $_ (and @​_, and %_, etc).

[p5pRT]

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

[p5pRT]

From @Leont

On Wed, Feb 23, 2011 at 10​:22 PM, Eric Brine via RT
<perlbug-followup@​perl.org> wrote​:

This was fixed and reverted due to backwards compatibility reasons. For
example, it broke File​::chdir. See

See
http​://perl5.git.perl.org/perl.git/commit/89adf4e8e0de17fedf78328b32920f47635850d0

I'm wondering if adding a LOCALIZE method to ties could solve this.
The underlying magic supports it, so why not ties too?

Leon

[p5pRT]

From @jandubois

On Wed, 23 Feb 2011, Leon Timmermans wrote​:

On Wed, Feb 23, 2011 at 10​:22 PM, Eric Brine via RT
<perlbug-followup@​perl.org> wrote​:

This was fixed and reverted due to backwards compatibility reasons. For
example, it broke File​::chdir. See

See
http​://perl5.git.perl.org/perl.git/commit/89adf4e8e0de17fedf78328b32920f47635850d0

I'm wondering if adding a LOCALIZE method to ties could solve this.
The underlying magic supports it, so why not ties too?

But how would you trigger the call to LOCALIZE?

I don't want

  local $_;

to call LOCALIZE on the existing value of $_. I expect it
to put the existing value on the savestack and leave $_ with
a new SV, and I expect the old value to be automatically
restored when execution leaves the current scope.

So how would LOCALIZE come into play? Would that be triggered
by a new builtin, e.g.

  localize $_;

which does the same thing as local(), except on tied scalars it would
copy the existing magic to the new SV?

Cheers,
-Jan

[p5pRT]

From @ikegami

On Thu, Feb 24, 2011 at 12​:16 AM, Jan Dubois <jand@​activestate.com> wrote​:

I don't want

local $_;

to call LOCALIZE on the existing value of $_. I expect it
to put the existing value on the savestack and leave $_ with
a new SV

How do you propose to address backwards compatibility?

[p5pRT]

From @jandubois

On Wed, 23 Feb 2011, Eric Brine wrote​:

On Thu, Feb 24, 2011 at 12​:16 AM, Jan Dubois <jand@​activestate.com> wrote​:

I don't want

local $_;

to call LOCALIZE on the existing value of $_. I expect it
to put the existing value on the savestack and leave $_ with
a new SV

How do you propose to address backwards compatibility?

I claim that the current behavior is a bug. Any time we fix a bug,
programs that rely on the presence of the bug get broken. There is no
guarantee of backwards compatibility for buggy behavior, otherwise there
can be no bug fixes at all.

If I look at the number of instances of "local $_" in the core and on
CPAN, which are almost all potentially broken right now (although only
under rather rare circumstances), then I think I prefer to fix all of
those and tell the module authors of the few modules (is there actually
another one besides File​::chdir?) that they'll have to fix their
modules instead.

In the end it is something Jesse will have to decide.

Cheers,
-Jan

[p5pRT]

From @wchristian

The original reason why this was reverted to bug behavior as far as i can
see was that the localizing of tied arrays/hashes broke. In this case we
don't really care about that though and instead care only about the
special case of localizing the scalar $_. Now, i'm probably being naive,
but would it be possible to reduce this to special-casing only "local
$scalar" and/or "local $_"?

Similarly​: Can File​::chdir be fixed at all?

[p5pRT]

From @Leont

On Thu, Feb 24, 2011 at 6​:16 AM, Jan Dubois <jand@​activestate.com> wrote​:

On Wed, 23 Feb 2011, Leon Timmermans wrote​:
But how would you trigger the call to LOCALIZE?

I don't want

   local $_;

to call LOCALIZE on the existing value of $_.  I expect it
to put the existing value on the savestack and leave $_ with
a new SV, and I expect the old value to be automatically
restored when execution leaves the current scope.

So how would LOCALIZE come into play?  Would that be triggered
by a new builtin, e.g.

I mean that local would do the equivalent of

  tied($foo)->LOCALIZE($new_variable) if tied($foo) &&
tied($foo)->can('LOCALIZE');

That way we could fix this bug yet and also allow us to unbreak
File​::chdir by adding such a LOCALIZE method. There's a svt_local slot
in the magic vtable specially for this kind of scenario. it shouldn't
be too hard to implement. Or am I overseeing something in this jungle
of magic and indirection?

Leon

[p5pRT]

From @jandubois

On Thu, 24 Feb 2011, Leon Timmermans wrote​:

On Thu, Feb 24, 2011 at 6​:16 AM, Jan Dubois <jand@​activestate.com> wrote​:

On Wed, 23 Feb 2011, Leon Timmermans wrote​:
But how would you trigger the call to LOCALIZE?

I don't want

local $_;

to call LOCALIZE on the existing value of $_. I expect it
to put the existing value on the savestack and leave $_ with
a new SV, and I expect the old value to be automatically
restored when execution leaves the current scope.

So how would LOCALIZE come into play? Would that be triggered
by a new builtin, e.g.

I mean that local would do the equivalent of

tied($foo)->LOCALIZE($new_variable) if tied($foo) &&
tied($foo)->can('LOCALIZE');

That way we could fix this bug yet and also allow us to unbreak
File​::chdir by adding such a LOCALIZE method. There's a svt_local slot
in the magic vtable specially for this kind of scenario. it shouldn't
be too hard to implement. Or am I overseeing something in this jungle
of magic and indirection?

That doesn't really fix the problem though.

However, I just noticed that this issue is already documented in perlsub​:

| Notably, if you want to work with a brand new value of the default
| scalar $_, and avoid the potential problem listed above about $_
| previously carrying a magic value, you should use "local *_" instead of
| "local $_".

And given that local() has to maintain the magic of various punctuation
variables anyways, I think there is not much point in fixing it just
for tied variables.

So I think it may be best to change the code that uses "local $_" in
the core to "local *_" instead. Unfortunately there is a ton of code on
CPAN that will continue to do the wrong thing. Google codesearch
finds over 200_000 instance for /local\s+$_/​:

  https://www.google.com/codesearch?as_q=local\s%2B\%24_

So are there any objections to doing s/local $_/local *_/g for the non-
dual-lifed code in the core?

Cheers,
-Jan

[p5pRT]

From @Leont

On Fri, Feb 25, 2011 at 1​:22 AM, Jan Dubois <jand@​activestate.com> wrote​:

That doesn't really fix the problem though.

I meant that combined with the now-reverted patch. It doesn't fix the
issue by itself. I think it would only break when someone aliases $_
to something that ties LOCALIZE. The user still has enough rope to
hang themselves by, but at least it's less rope than currently. Am I
missing something?

And given that local() has to maintain the magic of various punctuation
variables anyways, I think there is not much point in fixing it just
for tied variables.

Those don't really have to deal with aliasing, so I don't see how
that's a problem.

Leon

[p5pRT]

From @jandubois

On Thu, 24 Feb 2011, Leon Timmermans wrote​:

On Fri, Feb 25, 2011 at 1​:22 AM, Jan Dubois <jand@​activestate.com> wrote​:

That doesn't really fix the problem though.

I meant that combined with the now-reverted patch. It doesn't fix the
issue by itself. I think it would only break when someone aliases $_
to something that ties LOCALIZE. The user still has enough rope to
hang themselves by, but at least it's less rope than currently. Am I
missing something?

Yes, you reduce the chance for the bug to surface, but you don't fix it
completely. It still means that a module author cannot use

  local $_;

to free $_ up to use in whatever way they need it. It may still remain
tied to some other code. The only way to use $_ safely in a function
will be by aliasing it to a regular value first, with either for() or
map(), or by allocating a new glob via

  local *_;

But perlsub.pod already documents that it is never safe to call
local($_) and assume you can now do whatever you want with $_.
That's really unfortunate.

And given that local() has to maintain the magic of various punctuation
variables anyways, I think there is not much point in fixing it just
for tied variables.

Those don't really have to deal with aliasing, so I don't see how
that's a problem.

You can alias $_ to any of the other punctuation variables, at which
point $_ will carry the same magic as those variables. And local($_)
will not strip that magic either​:

$ perl -wE 'sub foo { local $_ = "bar"; say } foo() for $!'
Argument "bar" isn't numeric in scalar assignment at -e line 1.

Unfortunately you can't write "local *_ = 'foo'" either, you have
to write​:

$ perl -wE 'sub foo { local *_; $_ = "bar"; say } foo() for $!'
bar

I'm just saying that if we can't strip the other kinds of magic from
$_ with local(), then it doesn't really matter if we strip tie()
magic or not, as local($_) isn't safe to use anyways.

Cheers,
-Jan

PS​: I've looked at the "local $_" in Exporter.pm again (which is the
one that triggered this bug report), and as far as I can tell it is
completely redundant anyways, as all assignments to $_ in import()
happen via foreach(), which localizes $_ implicitly...

[p5pRT]

From @jandubois

On Thu, 24 Feb 2011, Jan Dubois wrote​:

It still means that a module author cannot use

local $\_;

to free $_ up to use in whatever way they need it.

One more (possibly wild) idea​: Could we add a special rule to
local() that it would strip *all* magic if its argument is an
alias (a loop iteration variable), but otherwise keep the magic
and just reset the value?

This would allow local($_) to be useful again without breaking
File​::chdir(). The problem is that currently we don't seem to
mark aliases, so we would need an SV flag, and I don't think
there is anything left that we could overload.

An alternative would be to only treat "local $_" specially.
This could be done at compile time, so wouldn't need any
SV flags at runtime.

The motivation is of course to make local($_) reliable.
Currently it is somewhat useless, unless you know for sure
that $_ isn't aliased to something magical. But typically
you just want to save $_ because you have no idea what it
contains and just don't want to touch it in case your caller
expects it to be unchanged on return.

Cheers,
-Jan

[p5pRT]

From @iabyn

On Thu, Feb 24, 2011 at 12​:58​:29AM -0800, Christian Walde via RT wrote​:

The original reason why this was reverted to bug behavior as far as i can
see was that the localizing of tied arrays/hashes broke. In this case we
don't really care about that though and instead care only about the
special case of localizing the scalar $_. Now, i'm probably being naive,
but would it be possible to reduce this to special-casing only "local
$scalar" and/or "local $_"?

I'm not sure if what you say above is correct (because its a bit
ambiguous).

What happened is that there were two fixes. The first made
  local @​a and local %h
not keep @​a and %h tied. This fix is still in place (and the old behaviour
made no sense).

The second fix tried to to the same for
  local $s
and was reverted after it broke File​::chdir (and I suspected would break
lots of other things too).

Similarly​: Can File​::chdir be fixed at all?

No, Because it's documented usage pattern relies on local keeping its
magical tiedness. (Unless it becomes an XS module and starts using special
magic rather than tieing.)

--
Spock (or Data) is fired from his high-ranking position for not being able
to understand the most basic nuances of about one in three sentences that
anyone says to him.
  -- Things That Never Happen in "Star Trek" #19

[p5pRT]

From @iabyn

On Thu, Feb 24, 2011 at 04​:22​:46PM -0800, Jan Dubois wrote​:

So are there any objections to doing s/local $_/local *_/g for the non-
dual-lifed code in the core?

If we're doing that, then we really ought to be changing it to 'my $_'
instead for most cases. After all, that was one of the reasons for
introducing lexical $_ !

--
Overhead, without any fuss, the stars were going out.
  -- Arthur C Clarke