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

"sub foo :unique" segfaults #7043

Closed
p5pRT opened this issue Jan 18, 2004 · 15 comments
Closed

"sub foo :unique" segfaults #7043

p5pRT opened this issue Jan 18, 2004 · 15 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 18, 2004

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

Searchable as RT24940$

@p5pRT
Copy link
Author

p5pRT commented Jan 18, 2004

From grazz@pobox.com

This is a bug report for perl from grazz@​pobox.com,
generated with the help of perlbug 1.34 running under perl v5.8.3.


  % perl -e 'sub foo :unique'
  Bus error

It seems to blow up here

  xsutils.c​:102​:
  --------------
  if (strEQ(name, "unique")) {
  if (negated)
  GvUNIQUE_off(CvGV((CV*)sv));
  else
  GvUNIQUE_on(CvGV((CV*)sv));
  continue;
  }

Because the attributes are applied before CvGV(cv) is set.



Flags​:
  category=core
  severity=low


Site configuration information for perl v5.8.3​:

Configured by grazz at Sat Jan 17 14​:19​:04 EST 2004.

Summary of my perl5 (revision 5.0 version 8 subversion 3) configuration​:
  Platform​:
  osname=darwin, osvers=6.6, archname=darwin
  uname='darwin faure.grazzini.net 6.6 darwin kernel version 6.6​: thu
may 1 21​:48​:54 pdt 2003; root​:xnuxnu-344.34.obj~1release_ppc power
macintosh powerpc '
  config_args='-DDEBUGGING -de'
  hint=recommended, useposix=true, d_sigaction=define
  usethreads=undef use5005threads=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 ='-pipe -fno-common -DPERL_DARWIN -no-cpp-precomp
-fno-strict-aliasing',
  optimize='-Os',
  cppflags='-no-cpp-precomp -pipe -fno-common -DPERL_DARWIN
-no-cpp-precomp -fno-strict-aliasing'
  ccversion='', gccversion='3.1 20020420 (prerelease)',
gccosandvers=''
  intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8
  ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -flat_namespace -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib
  libs=-lm -lc
  perllibs=-lm -lc
  libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false,
libperl=libperl.a
  gnulibc_version=''
  Dynamic Linking​:
  dlsrc=dl_dyld.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
  cccdlflags=' ', lddlflags=' -flat_namespace -bundle -undefined
suppress -L/usr/local/lib'

Locally applied patches​:


@​INC for perl v5.8.3​:
  /sw/lib/perl5/darwin
  /sw/lib/perl5
  /usr/local/lib/perl5/5.8.3/darwin
  /usr/local/lib/perl5/5.8.3
  /usr/local/lib/perl5/site_perl/5.8.3/darwin
  /usr/local/lib/perl5/site_perl/5.8.3
  /usr/local/lib/perl5/site_perl/5.8.2/darwin
  /usr/local/lib/perl5/site_perl/5.8.2
  /usr/local/lib/perl5/site_perl
  .


Environment for perl v5.8.3​:
  DYLD_LIBRARY_PATH (unset)
  HOME=/Users/grazz
  LANG=C
  LANGUAGE (unset)
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
 
PATH=/sw/bin​:/sw/sbin​:/usr/bin​:/bin​:/usr/sbin​:/sbin​:/Users/grazz​:/usr/
X11R6/bin
  PERL5LIB=/sw/lib/perl5
  PERL_BADLANG (unset)
  SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Jan 19, 2004

From @rgs

Steve Grazzini (via RT) wrote​:

 % perl \-e 'sub foo :unique'
 Bus error

Does :unique make any sense at all with subroutines anyway?

@p5pRT
Copy link
Author

p5pRT commented Jan 19, 2004

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

@p5pRT
Copy link
Author

p5pRT commented Jan 19, 2004

From @lizmat

At 10​:31 +0100 1/19/04, Rafael Garcia-Suarez wrote​:

Steve Grazzini (via RT) wrote​:

  % perl \-e 'sub foo :unique'
 Bus error

Does :unique make any sense at all with subroutines anyway?

Can't find any documentation in /pod about it. Only about using
"​:unique" with our.

Liz

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @rgs

Elizabeth Mattijsen wrote​:

At 10​:31 +0100 1/19/04, Rafael Garcia-Suarez wrote​:

Steve Grazzini (via RT) wrote​:

  % perl \-e 'sub foo :unique'
 Bus error

Does :unique make any sense at all with subroutines anyway?

Can't find any documentation in /pod about it. Only about using
"​:unique" with our.

But the internals use a CVf_UNIQUE attribute for code references.

I don't really understand the implications of the patch below, but
it solves the segfault. Comments?

--- xsutils.c (revision 3095)
+++ xsutils.c (working copy)
@​@​ -111,9 +111,9 @​@​ modify_SV_attributes(pTHX_ SV *sv, SV **
  case 'u'​:
  if (strEQ(name, "unique")) {
  if (negated)
- GvUNIQUE_off(CvGV((CV*)sv));
+ CvUNIQUE_off((CV*)sv);
  else
- GvUNIQUE_on(CvGV((CV*)sv));
+ CvUNIQUE_on((CV*)sv);
  continue;
  }
  break;

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @iabyn

On Tue, Jan 20, 2004 at 09​:07​:55PM +0100, Rafael Garcia-Suarez wrote​:

Elizabeth Mattijsen wrote​:

At 10​:31 +0100 1/19/04, Rafael Garcia-Suarez wrote​:

Steve Grazzini (via RT) wrote​:

  % perl \-e 'sub foo :unique'
 Bus error

Does :unique make any sense at all with subroutines anyway?

Can't find any documentation in /pod about it. Only about using
"​:unique" with our.

But the internals use a CVf_UNIQUE attribute for code references.

I don't really understand the implications of the patch below, but
it solves the segfault. Comments?

--- xsutils.c (revision 3095)
+++ xsutils.c (working copy)
@​@​ -111,9 +111,9 @​@​ modify_SV_attributes(pTHX_ SV *sv, SV **
case 'u'​:
if (strEQ(name, "unique")) {
if (negated)
- GvUNIQUE_off(CvGV((CV*)sv));
+ CvUNIQUE_off((CV*)sv);
else
- GvUNIQUE_on(CvGV((CV*)sv));
+ CvUNIQUE_on((CV*)sv);
continue;
}
break;

CVf_UNIQUE and GVf_UNIQUE are two separate concepts. The :unique
atrribute sets the GVf_UNIQUE flag on the GV associated with the
variable. Note that C<our $a : unique> makes not only $a unique, but @​a,
%a etc too.

On the the hand, CVf_UNIQUE is simply a flag on the CV saying that it
will only ever be called once. Eg PL_main_cv and the CVs associated with
require/eval.

I presume that the real cause of the bug is that CVs are attached to the
GV at the end of compilation rather than at the beginning, so at the time
atrributes are processed, GvGV is null (or refers to the old CV if we are
redefining it).

Ages ago I was working on a patch for this (attach the PL_compcv to the GV
at the start of compilation), which would solve lots of problems, eg the
'prototype too late' warning in foo($) { ... foo(1) ... } This involved
(roughly speaking) splitting newATTRSUB() into two halves, one called at
the start and one at the, of compilation. But it was hard, and I put it to
one side...

I think in the short term, :unique on a sub should just give an error.

--
O Unicef Clearasil!
Gibberish and Drivel!
  - "Bored of the Rings"

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @rgs

Dave Mitchell wrote​:

CVf_UNIQUE and GVf_UNIQUE are two separate concepts. The :unique
atrribute sets the GVf_UNIQUE flag on the GV associated with the
variable. Note that C<our $a : unique> makes not only $a unique, but @​a,
%a etc too.

Indeed -- the name confused me, as Arthur pointed out on IRC.

On the the hand, CVf_UNIQUE is simply a flag on the CV saying that it
will only ever be called once. Eg PL_main_cv and the CVs associated with
require/eval.

The documentation about it is rather terse.

I presume that the real cause of the bug is that CVs are attached to the
GV at the end of compilation rather than at the beginning, so at the time
atrributes are processed, GvGV is null (or refers to the old CV if we are
redefining it).

Ages ago I was working on a patch for this (attach the PL_compcv to the GV
at the start of compilation), which would solve lots of problems, eg the
'prototype too late' warning in foo($) { ... foo(1) ... } This involved
(roughly speaking) splitting newATTRSUB() into two halves, one called at
the start and one at the, of compilation. But it was hard, and I put it to
one side...

I think in the short term, :unique on a sub should just give an error.

I agree. But would your patch solve all problems with :unique subs ?
Notably unique pads, unique outside cv...

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @rgs

Rafael Garcia-Suarez wrote​:

Dave Mitchell wrote​:

I think in the short term, :unique on a sub should just give an error.

I agree. But would your patch solve all problems with :unique subs ?
Notably unique pads, unique outside cv...

Hmm, that probably a reason for forbidding "my $x​:unique" too.
(currently it looks :unique up in user attributes, or better said,
it wastes time.)

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @iabyn

On Tue, Jan 20, 2004 at 09​:59​:19PM +0100, Rafael Garcia-Suarez wrote​:

Ages ago I was working on a patch for this (attach the PL_compcv to the GV
at the start of compilation), which would solve lots of problems, eg the
'prototype too late' warning in foo($) { ... foo(1) ... } This involved
(roughly speaking) splitting newATTRSUB() into two halves, one called at
the start and one at the, of compilation. But it was hard, and I put it to
one side...

I think in the short term, :unique on a sub should just give an error.

I agree. But would your patch solve all problems with :unique subs ?
Notably unique pads, unique outside cv...

Ah yes, I wasn't thinking far enough ahead. It would solve the coredump,
but it's still a nonsense concept to mark a sub as :unique.

I think therefore a C<sub foo​:unique> should generate a warning that it
has no effect, and then be ignored.

--
"Foul and greedy Dwarf - you have eaten the last candle."
  -- "Hoardes of the Things", BBC Radio.

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @lizmat

At 20​:51 +0000 1/20/04, Dave Mitchell wrote​:

On Tue, Jan 20, 2004 at 09​:07​:55PM +0100, Rafael Garcia-Suarez wrote​:

Elizabeth Mattijsen wrote​:

At 10​:31 +0100 1/19/04, Rafael Garcia-Suarez wrote​:

Steve Grazzini (via RT) wrote​:

  % perl \-e 'sub foo :unique'
 Bus error

Does :unique make any sense at all with subroutines anyway?

Can't find any documentation in /pod about it. Only about using
"​:unique" with our.

But the internals use a CVf_UNIQUE attribute for code references.

I don't really understand the implications of the patch below, but
it solves the segfault. Comments?

--- xsutils.c (revision 3095)
+++ xsutils.c (working copy)
@​@​ -111,9 +111,9 @​@​ modify_SV_attributes(pTHX_ SV *sv, SV **
case 'u'​:
if (strEQ(name, "unique")) {
if (negated)
- GvUNIQUE_off(CvGV((CV*)sv));
+ CvUNIQUE_off((CV*)sv);
else
- GvUNIQUE_on(CvGV((CV*)sv));
+ CvUNIQUE_on((CV*)sv);
continue;
}
break;

CVf_UNIQUE and GVf_UNIQUE are two separate concepts. The :unique
atrribute sets the GVf_UNIQUE flag on the GV associated with the
variable. Note that C<our $a : unique> makes not only $a unique, but @​a,
%a etc too.

Argh... is that true? ARGH, it IS!

use threads;
our $foo : unique;
threads->new( sub { @​foo = (1,2,3) } )->join;
__END__
thread failed to start​: Modification of a read-only value attempted
at file line 3.

I think in the short term, :unique on a sub should just give an error.

And in the longer term, I think "​:unique" should die altogether...
It's a poor man's attempt at COW that breaks too easily (as was found
with bugs introduced by me in 5.8.2, fixed in 5.8.3 fortunately).

Liz

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @iabyn

On Tue, Jan 20, 2004 at 09​:59​:19PM +0100, Rafael Garcia-Suarez wrote​:

On the the hand, CVf_UNIQUE is simply a flag on the CV saying that it
will only ever be called once. Eg PL_main_cv and the CVs associated with
require/eval.

The documentation about it is rather terse.

now corrected​:

--
A walk of a thousand miles begins with a single step...
then continues for another 1,999,999 or so.

Change 22185 by davem@​davem-percy on 2004/01/20 21​:01​:08

  Document CVf_UNIQUE flag better

Affected files ...

... //depot/perl/cv.h#46 edit

Differences ...

==== //depot/perl/cv.h#46 (text) ====

@​@​ -75,7 +75,10 @​@​
#define CVf_CLONED 0x0002 /* a clone of one of those */
#define CVf_ANON 0x0004 /* CvGV() can't be trusted */
#define CVf_OLDSTYLE 0x0008
-#define CVf_UNIQUE 0x0010 /* can't be cloned */
+#define CVf_UNIQUE 0x0010 /* sub is only called once (eg PL_main_cv,
+ * require, eval). Not to be confused
+ * with the GVf_UNIQUE flag associated
+ * with the :unique attribute */
#define CVf_NODEBUG 0x0020 /* no DB​::sub indirection for this CV
  (esp. useful for special XSUBs) */
#define CVf_METHOD 0x0040 /* CV is explicitly marked as a method */

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @iabyn

On Tue, Jan 20, 2004 at 10​:06​:09PM +0100, Rafael Garcia-Suarez wrote​:

Rafael Garcia-Suarez wrote​:
Hmm, that probably a reason for forbidding "my $x​:unique" too.
(currently it looks :unique up in user attributes, or better said,
it wastes time.)

Yuck.
Do you want to fixup all this or shall I?

--
The optimist believes that he lives in the best of all possible worlds.
As does the pessimist.

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @rgs

Dave Mitchell wrote​:

On Tue, Jan 20, 2004 at 10​:06​:09PM +0100, Rafael Garcia-Suarez wrote​:

Rafael Garcia-Suarez wrote​:
Hmm, that probably a reason for forbidding "my $x​:unique" too.
(currently it looks :unique up in user attributes, or better said,
it wastes time.)

Yuck.
Do you want to fixup all this or shall I?

You can do it :) (As I'm going to London in two days I'll have less
time to hack things, not counting the weekly summary, so you're most
welcome.)

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

From @iabyn

On Tue, Jan 20, 2004 at 10​:32​:24PM +0100, Rafael Garcia-Suarez wrote​:

Dave Mitchell wrote​:

On Tue, Jan 20, 2004 at 10​:06​:09PM +0100, Rafael Garcia-Suarez wrote​:

Rafael Garcia-Suarez wrote​:
Hmm, that probably a reason for forbidding "my $x​:unique" too.
(currently it looks :unique up in user attributes, or better said,
it wastes time.)

Yuck.
Do you want to fixup all this or shall I?

You can do it :) (As I'm going to London in two days I'll have less
time to hack things, not counting the weekly summary, so you're most
welcome.)

Well, say hello to the city of my childhood for me :-)

I've made my/sub : unique fatal compile-time errors as patch #22187. I've
also added the following warning to L<perlfunc/our> as patch #22188​:

+Warning​: the current implementation of this attribute operates on the
+typeglob associated with the variable; this means that C<our $x : unique>
+also has the effect of C<our @​x : unique; our %x : unique>. This may be
+subject to change.

--
Little fly, thy summer's play my thoughtless hand
has terminated with extreme prejudice.
  (with apologies to William Blake)

@p5pRT
Copy link
Author

p5pRT commented Jan 20, 2004

@iabyn - Status changed from 'open' 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