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

%stash:: = () anonymises CVs #10826

Closed
p5pRT opened this issue Nov 14, 2010 · 6 comments
Closed

%stash:: = () anonymises CVs #10826

p5pRT opened this issue Nov 14, 2010 · 6 comments

Comments

@p5pRT
Copy link
Collaborator

@p5pRT p5pRT commented Nov 14, 2010

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

Searchable as RT79208$

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 14, 2010

From @cpansprout

Currently, a stash loses its name temporarily during hv_clear. As a result, when a subroutine whose GV is in that stash has a reference elsewhere, its GV pointer is replaced with a pointer to an __ANON__ GV whose stash pointer points to the __ANON__ package.

In order to fix another bug, I tried changing hv_clear such that the name is always visible, but there are tests for that anonymisation that fail as a result (in t/op/stash.t)​:

  $sub = do {
  package two;
  \&{"two"};
  };
  %two​:: = ();
  $gv = b($sub)->GV;

  isa_ok( $gv, "B​::GV", "cleared stash leaves CV with valid GV");
  is( b($sub)->CvFLAGS & $CVf_ANON, $CVf_ANON, "...and CVf_ANON set");
  is( eval { $gv->NAME }, "__ANON__", "...and an __ANON__ name");
  is( eval { $gv->STASH->NAME }, "__ANON__", "...and an __ANON__ stash");

That last is() does not seem right to me. May I change it?

This is getting in the way of about three bug fixes.

Also, there are to-do tests to check that either stashes lose their names after %stash​::=() or that anonymous subs with ->GV->STASH pointing to %stash​:: will have it redirected to %__ANON__​::. I think those should change, too.


Flags​:
  category=core
  severity=low


Site configuration information for perl 5.13.6​:

Configured by sprout at Thu Nov 11 04​:29​:30 PST 2010.

Summary of my perl5 (revision 5 version 13 subversion 6) configuration​:
  Commit id​: fc6b470
  Platform​:
  osname=darwin, osvers=10.4.0, archname=darwin-2level
  uname='darwin pint.local 10.4.0 darwin kernel version 10.4.0​: fri apr 23 18​:28​:53 pdt 2010; root​:xnu-1504.7.4~1release_i386 i386 '
  config_args='-de -Dusedevel -Doptimize=-Os -Dusemymalloc=y -Duseshrplib=true -Duselargefiles=yes -Duseposix=true -Dhint-recommended =Duseperlio=yes -Duse64bitint=yes'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=undef, use64bitall=undef, uselongdouble=undef
  usemymalloc=y, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include',
  optimize='-Os',
  cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='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 =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-ldbm -ldl -lm -lutil -lc
  perllibs=-ldl -lm -lutil -lc
  libc=/usr/lib/libc.dylib, 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 -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.13.6​:
  /usr/local/lib/perl5/site_perl/5.13.6/darwin-2level
  /usr/local/lib/perl5/site_perl/5.13.6
  /usr/local/lib/perl5/5.13.6/darwin-2level
  /usr/local/lib/perl5/5.13.6
  /usr/local/lib/perl5/site_perl
  .


Environment for perl 5.13.6​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/sprout
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2010

From @demerphq

On 14 November 2010 22​:43, Father Chrysostomos
<perlbug-followup@​perl.org> wrote​:

# New Ticket Created by  Father Chrysostomos
# Please include the string​:  [perl #79208]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=79208 >

Currently, a stash loses its name temporarily during hv_clear. As a result, when a subroutine whose GV is in that stash has a reference elsewhere, its GV pointer is replaced with a pointer to an __ANON__ GV whose stash pointer points to the __ANON__ package.

In order to fix another bug, I tried changing hv_clear such that the name is always visible, but there are tests for that anonymisation that fail as a result (in t/op/stash.t)​:

   $sub = do {
       package two;
       \&{"two"};
   };
   %two​:: = ();
   $gv = b($sub)->GV;

   isa_ok( $gv, "B​::GV", "cleared stash leaves CV with valid GV");
   is( b($sub)->CvFLAGS & $CVf_ANON, $CVf_ANON, "...and CVf_ANON set");
   is( eval { $gv->NAME }, "__ANON__", "...and an __ANON__ name");
   is( eval { $gv->STASH->NAME }, "__ANON__", "...and an __ANON__ stash");

That last is() does not seem right to me. May I change it?

I think you can, but this has to be prominent in the changes file.

The advantage of having named anonymous subs IMO massively outweighs
the damage that might be done to code that depends on them being
called __ANON__, code which most likely become redudndant when they no
longer are.

IOW, i think the only code that cares about this name is code that
probably exists only to deal with the fact that it doesnt have a real
name.

Yves

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2010

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

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2010

From @iabyn

On Sun, Nov 14, 2010 at 01​:43​:05PM -0800, Father Chrysostomos wrote​:

Currently, a stash loses its name temporarily during hv_clear. As a result, when a subroutine whose GV is in that stash has a reference elsewhere, its GV pointer is replaced with a pointer to an __ANON__ GV whose stash pointer points to the __ANON__ package.

In order to fix another bug, I tried changing hv_clear such that the name is always visible, but there are tests for that anonymisation that fail as a result (in t/op/stash.t)​:

$sub = do \{
    package two;
    \\&\{"two"\};
\};
%two&#8203;:: = \(\);
$gv = b\($sub\)\->GV;

isa\_ok\( $gv\, "B&#8203;::GV"\, "cleared stash leaves CV with valid GV"\);
is\( b\($sub\)\->CvFLAGS & $CVf\_ANON\, $CVf\_ANON\, "\.\.\.and CVf\_ANON set"\);
is\( eval \{ $gv\->NAME \}\, "\_\_ANON\_\_"\, "\.\.\.and an \_\_ANON\_\_ name"\);
is\( eval \{ $gv\->STASH\->NAME \}\, "\_\_ANON\_\_"\, "\.\.\.and an \_\_ANON\_\_ stash"\);

That last is() does not seem right to me. May I change it?

This is getting in the way of about three bug fixes.

Also, there are to-do tests to check that either stashes lose their names after %stash​::=() or that anonymous subs with ->GV->STASH pointing to %stash​:: will have it redirected to %__ANON__​::. I think those should change, too.

I think those tests were more concerned with checking that the STASH
points to something valid (rather than freed memory etc) than with what
the particular value was, and I agree with you that the test is wrong and
that the behaviour should be changed. In particular, I think that

  %two​:: = ();

should be semantically equivalent to

  delete $two​::{$_} for keys %two​::;

and indeed if you substitute that in stash.t, the test fails.

There does however need to be a clear distinction between clearing
and undeffing (e.g. '%stash​:: = ()' verses 'undef %stash​::'); in the latter
case I think its right that the stash loses its name. At one point
I was planning to give S_hfreeentries() an extra arg to indicate whether it
was called from hv_clear() or hv_undef().

--
"You may not work around any technical limitations in the software"
  -- Windows Vista license

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2010

From @cpansprout

Fixed in 2d0d1ec.

@p5pRT
Copy link
Collaborator Author

@p5pRT p5pRT commented Nov 15, 2010

@cpansprout - 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
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.