develooper Front page | perl.perl5.porters | Postings from April 2013

Re: [perl #54044] perl -wle '%::=();//' crash

Thread Previous | Thread Next
From:
Nicholas Clark
Date:
April 10, 2013 11:13
Subject:
Re: [perl #54044] perl -wle '%::=();//' crash
Message ID:
20130410111337.GY3729@plum.flirble.org
On Mon, Apr 08, 2013 at 02:27:04PM -0700, Reini Urban via RT wrote:
> On Mon Apr 08 11:36:23 2013, rurban wrote:
> > Oops, previous patch had a last minute typo in it (sv vs hv)
> > 
> > Use this one instead.
> > I'm smoking it now with my cpan subset to catch possible 
> > %{"$pkg\::"}=... cases with empty $pkg.
> 
> Smoked my cpan successfully with this revised patch.

Mmm, that's an interesting failure case that I'd not thought of.


> +++ b/t/op/stash.t
> @@ -7,12 +7,12 @@ BEGIN {
>  
>  BEGIN { require "./test.pl"; }
>  
> -plan( tests => 58 );
> +plan( tests => 60 );
>  
> -# Used to segfault (bug #15479)
> +# Used to segfault (bug #15479 and #54044)
>  fresh_perl_like(
>      '%:: = ""',
> -    qr/Odd number of elements in hash assignment at - line 1\./,
> +    qr/^Attempt to clear the %main:: symbol table at - line 1\./m,
>      { switches => [ '-w' ] },
>      'delete $::{STDERR} and print a warning',
>  );
> @@ -60,14 +60,14 @@ package main;
>      local $ENV{PERL_DESTRUCT_LEVEL} = 2;
>      fresh_perl_is(
>  		  'package A; sub a { // }; %::=""',
> -		  '',
> +		  'Attempt to clear the %main:: symbol table at - line 1.',
>  		  '',
>  		  );
>      # Variant of the above which creates an object that persists until global
>      # destruction.
>      fresh_perl_is(
>  		  'use Exporter; package A; sub a { // }; %::=""',
> -		  '',
> +		  'Attempt to clear the %main:: symbol table at - line 1.',
>  		  '',
>  		  );
>  }

I don't think that these three should be changed like that. Clearing %::
was the intent of the tests. Triggering an error or SEGV as the side effect
of something no longer being in %:: was the intent. I think that a better
change would be this:

diff --git a/t/op/stash.t b/t/op/stash.t
index fd5450e..2681d47 100644
--- a/t/op/stash.t
+++ b/t/op/stash.t
@@ -11,7 +11,7 @@ plan( tests => 58 );
 
 # Used to segfault (bug #15479)
 fresh_perl_like(
-    '%:: = ""',
+    'delete $::{STDERR}; my %a = ""',
     qr/Odd number of elements in hash assignment at - line 1\./,
     { switches => [ '-w' ] },
     'delete $::{STDERR} and print a warning',
@@ -59,14 +59,15 @@ package main;
 {
     local $ENV{PERL_DESTRUCT_LEVEL} = 2;
     fresh_perl_is(
-		  'package A; sub a { // }; %::=""',
+		  'package A::B; sub a { // }; %A::=""',
 		  '',
 		  '',
 		  );
     # Variant of the above which creates an object that persists until global
-    # destruction.
+    # destruction, and triggers an assertion failure prior to change
+    # a420522db95b7762
     fresh_perl_is(
-		  'use Exporter; package A; sub a { // }; %::=""',
+		  'use Exporter; package A; sub a { // }; delete $::{$_} for keys %::',
 		  '',
 		  '',
 		  );


as those all maintain the original intent of the tests.

Verified. In that I built the 3 revisions where those 3 were added and
checked that my suggested revised test case also triggers the bug that each
was testing. Also, as those 3 don't change behaviour with Reini's patch
added, I'd be tempted to apply them as a commit before Reini's patch, so that
it's clear that the revised versions pass independently of his change.

I sort of assume that "code freeze" applies equally to the code that is tests,
so I'm not doing it now.

Nicholas Clark

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About