Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.99k stars 559 forks source link

Magical change in perl >= 5.21.7? #14449

Closed p5pRT closed 9 years ago

p5pRT commented 9 years ago

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

Searchable as RT123683$

p5pRT commented 9 years ago

From @steve-m-hay

Testing modperl (svn trunk\, not the current CPAN release) with httpd-2.4.10 and recent 5.21.x perls I find that things look ok up to and including 5.21.6\, but the mod_perl-enabled server fails to start with 5.21.7 and 5.21.8.

This function in modperl_util.c is where things go wrong​:

U16 *modperl_code_attrs(pTHX_ CV *cv) {   MAGIC *mg;

  if (!SvMAGICAL(cv)) {   sv_magic((SV*)cv\, (SV *)NULL\, PERL_MAGIC_ext\, NULL\, -1);   }

  mg = mg_find((SV*)cv\, PERL_MAGIC_ext);   return &(mg->mg_private); }

The cv passed in comes from cv = get_cv(name\, FALSE) where name is "main​::add_my_version".

SvMAGICAL(cv) is true so sv_magic() is not called\, and then mg_find() returns NULL and things all go wrong from there.

The "main​::add_my_version" in question is this in modperl_extra.pl​:

sub test_add_version_component {   Apache2​::ServerUtil->server->push_handlers(   PerlPostConfigHandler => \&add_my_version);

  sub add_my_version {   my ($conf_pool\, $log_pool\, $temp_pool\, $s) = @​_;   $s->add_version_component("world domination series/2.0");   return Apache2​::Const​::OK;   } }

If I move sub add_my_version {} outside of sub test_add_version_component {} like this​:

sub test_add_version_component {   Apache2​::ServerUtil->server->push_handlers(   PerlPostConfigHandler => \&add_my_version); }

sub add_my_version {   my ($conf_pool\, $log_pool\, $temp_pool\, $s) = @​_;   $s->add_version_component("world domination series/2.0");   return Apache2​::Const​::OK; }

then SvMAGICAL(cv) is now false so sv_magic() is called\, and then mg_find() returns non-NULL and all is well.

Is this an expected change in behaviour between 5.21.6 and 5.21.7 that mod_perl needs to keep up with\, or a bug in 5.21.7?

(I haven't tried the latest bleadperl yet\, but I don't see anything since 5.21.8 that looks like it would affect this.)

p5pRT commented 9 years ago

From @steve-m-hay

The change in behaviour can be seen with the attached XS module. Under 5.21.6 "make test" outputs "ok". Under 5.21.7 it outputs "not ok - SvMAGICAL returned TRUE".

p5pRT commented 9 years ago

From @steve-m-hay

Foo-1.00.tar.gz

p5pRT commented 9 years ago

From [Unknown Contact. See original ticket]

The change in behaviour can be seen with the attached XS module. Under 5.21.6 "make test" outputs "ok". Under 5.21.7 it outputs "not ok - SvMAGICAL returned TRUE".

p5pRT commented 9 years ago

From @Leont

On Tue\, Jan 27\, 2015 at 3​:15 PM\, Steve Hay \perlbug\-followup@​perl\.org wrote​:

Testing modperl (svn trunk\, not the current CPAN release) with httpd-2.4.10 and recent 5.21.x perls I find that things look ok up to and including 5.21.6\, but the mod_perl-enabled server fails to start with 5.21.7 and 5.21.8.

This function in modperl_util.c is where things go wrong​:

U16 *modperl_code_attrs(pTHX_ CV *cv) { MAGIC *mg;

if \(\!SvMAGICAL\(cv\)\) \{
   sv\_magic\(\(SV\*\)cv\, \(SV \*\)NULL\, PERL\_MAGIC\_ext\, NULL\, \-1\);
\}

mg = mg\_find\(\(SV\*\)cv\, PERL\_MAGIC\_ext\);
return &\(mg\->mg\_private\);

}

The cv passed in comes from cv = get_cv(name\, FALSE) where name is "main​::add_my_version".

SvMAGICAL(cv) is true so sv_magic() is not called\, and then mg_find() returns NULL and things all go wrong from there.

The code is naive at best. It is assuming there is no magic on the cv from any other type. If there is non-ext magic SvMAGICAL will be true but mg_find(PERL_MAGIC_ext) will return NULL. I would suggest something like this (untested)​:

U16 *modperl_code_attrs(pTHX_ CV *cv) {   MAGIC *mg;

  if (!(SvMAGICAL(cv) && mg = mg_find((SV*)cv\, PERL_MAGIC_ext))) {   mg = sv_magicext((SV*)cv\, (SV *)NULL\, PERL_MAGIC_ext\, NULL\, NULL\, -1);   }

  return &(mg->mg_private); }

Actually\, you may prefer to use mg_findext and an explicit vtable (even if it's empty)\, but that's another discussion.

Leon

p5pRT commented 9 years ago

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

p5pRT commented 9 years ago

From @tonycoz

On Tue Jan 27 14​:52​:38 2015\, LeonT wrote​:

The code is naive at best. It is assuming there is no magic on the cv from any other type. If there is non-ext magic SvMAGICAL will be true but mg_find(PERL_MAGIC_ext) will return NULL. I would suggest something like this (untested)​:

I had something pretty similar to this written up when I saw your post\, but I do wonder...

PERL_DL_NONLAZY=1 "/home/tony/perl/blead/bin/perl5.21.8" "-Iblib/lib" "-Iblib/arch" test.pl SV = PVCV(0x1d027d0) at 0x1cfa658   REFCNT = 1   FLAGS = (RMG\,DYNFILE)   MAGIC = 0x1cd9400   MG_VIRTUAL = &PL_vtbl_backref   MG_TYPE = PERL_MAGIC_backref(\<)   MG_OBJ = 0x1cd1640   COMP_STASH = 0x1cd14f0 "Foo"   START = 0x1cd9320 ===> 1   ROOT = 0x1cd92e0   GVGV​::GV = 0x1d410b8 "Foo" :​: "sub2"   FILE = "blib/lib/Foo.pm"   DEPTH = 0   FLAGS = 0x1000   OUTSIDE_SEQ = 454   PADLIST = 0x1cfd8f0   OUTSIDE = 0x1cd1568 (sub1) not ok - SvMAGICAL returned TRUE

why the reference started getting backref magic.

Tony

p5pRT commented 9 years ago

From @tonycoz

On Tue Jan 27 15​:14​:41 2015\, tonyc wrote​:

why the reference started getting backref magic.

a70f21d0d169a526a6bafd2465e01e1ca8d16234 is why.

Tony

p5pRT commented 9 years ago

From @steve-m-hay

Thank you both for your helpful replies.I should have realized what the cause of the problem there was\, but I wouldn't have known whether that was an intentional change in perl or not.

The suggested fix (plus some extra parens) did the trick​: https://svn.apache.org/viewvc?view=revision&revision=1655200

p5pRT commented 9 years ago

@steve-m-hay - Status changed from 'open' to 'resolved'