Front page | perl.perl5.porters |
Postings from January 2006
aelemfast optimization opclass problem (c.f. [perl #38279])
Thread Next
From:
Stephen McCamant
Date:
January 23, 2006 22:23
Subject:
aelemfast optimization opclass problem (c.f. [perl #38279])
Message ID:
17365.51166.604020.571992@conquest.OCF.Berkeley.EDU
The error message reported in bug #38279 doesn't come up on my Linux
box, but I was able to reproduce it on a FreeBSD machine I have
limited access to, and the example also causes Valgrind errors on
Linux. In tracking down one such error from a simplified test case, I
discovered an interesting bug. This message is about that bug; I'm not
sure if it's the cause of the *BSD failure, but I've added the tag to
the subject just in case.
A simplified test case looks like:
% valgrind perl -MO=C -e 'package Foo; sub new{} sub f {my @x; $x[0]}' \
>/dev/null
Here's the relevant report:
==30961== Invalid read of size 4
==30961== at 0x402A2B8: XS_B__SVOP_sv (B.c:1945)
==30961== by 0x80DED52: Perl_pp_entersub (pp_hot.c:2803)
==30961== by 0x47F5ECD: my_runops (C.xs:40)
==30961== by 0x8064122: S_call_body (perl.c:2211)
==30961== by 0x80639BF: Perl_call_sv (perl.c:2112)
==30961== by 0x8063751: Perl_call_method (perl.c:2045)
==30961== by 0x4024A35: walkoptree (B.xs:403)
==30961== by 0x4024A97: walkoptree (B.xs:408)
==30961== by 0x4024A97: walkoptree (B.xs:408)
==30961== by 0x4024A97: walkoptree (B.xs:408)
==30961== by 0x402605D: XS_B_walkoptree (B.xs:610)
==30961== by 0x80DED52: Perl_pp_entersub (pp_hot.c:2803)
==30961== Address 0x47238C4 is 0 bytes after a block of size 20 alloc'd
==30961== at 0x401B882: malloc (vg_replace_malloc.c:149)
==30961== by 0x80BF15D: Perl_safesysmalloc (util.c:73)
==30961== by 0x8094361: Perl_newOP (op.c:2282)
==30961== by 0x80812CE: S_pending_ident (toke.c:5553)
==30961== by 0x8072C6F: Perl_yylex (toke.c:2327)
==30961== by 0x808CA48: Perl_yyparse (perly.c:410)
==30961== by 0x8062CC6: S_parse_body (perl.c:1733)
==30961== by 0x8061C6C: perl_parse (perl.c:1202)
==30961== by 0x805E767: main (perlmain.c:97)
From the first line of the first backtrace and the third of the
second, it's clear that the problem is some that B::C code is trying
to read the op_sv field of an OP that was allocated as a BASEOP. The
troublesome OP is the aelemfast from the "$x[0]". The aelemfast
optimization morphs the array OP in an array index expression into an
OP_AELEMFAST that keeps the small index in the op_private field.
Originally, it was just for package variables:
6 <2> aelem vK/2 ->7
4 <1> rv2av sKR/1 ->5
3 <$> gv(*x) s ->4
5 <$> const(IV 42) s ->6
VV becomes VV
- <1> ex-aelem vK/2 ->7
- <1> ex-rv2av sKR/1 ->-
3 <$> aelemfast(*x) s/42 ->7
- <0> ex-const s ->-
Since change #22357, though, the optimization has also applied to
lexical arrays:
7 <2> aelem vK/2 ->8
5 <0> padav[@x:1,2] sR ->6
6 <$> const(IV 42) s ->7
VV becomes VV
- <1> ex-aelem vK/2 ->8
5 <0> aelemfast[@x:1,2] sR*/42 ->8
- <0> ex-const s ->-
Since an OP_PADAV is a BASEOP (not, as one might initially expect, a
PADOP), aelemfast OPs created this way are shorter than those that
come from OP_GVs. In normal execution this turns out to be safe
because pp_aelemfast knows not to try to use the missing field in the
lexical case (distinguishable by the OPf_SPECIAL flag). However, not
all of the compiler backends are so savvy: they rely on the class of
the OP, as guessed by B.xs:cc_opclass() to figure out what to dump.
The fix, then, is to make cc_opclass() smarter. Since we seem to be
close to using up the number of op class codes that can fit in 4 bits,
and I'm lazy, it seemed best to just use a test in the code.
In the following patch against bleadperl, I've also added a regression
test in a new file ext/B/t/optree_misc.t. Though somewhat cumbersome
(it looks like the automatic test-writing capability of OptreeCheck.pm
isn't working as well as it once did), this was less work that
creating ext/B/t/c.t, which is what would really be best.
-- Stephen
--- perl-current/ext/B/B.xs 2005-11-20 07:41:43.000000000 -0500
+++ perl@26919/ext/B/B.xs 2006-01-22 21:37:39.000000000 -0500
@@ -112,9 +112,20 @@ cc_opclass(pTHX_ const OP *o)
if (o->op_type == OP_SASSIGN)
return ((o->op_private & OPpASSIGN_BACKWARDS) ? OPc_UNOP : OPc_BINOP);
+ if (o->op_type == OP_AELEMFAST) {
+ if (o->op_flags & OPf_SPECIAL)
+ return OPc_BASEOP;
+ else
+#ifdef USE_ITHREADS
+ return OPc_PADOP;
+#else
+ return OPc_SVOP;
+#endif
+ }
+
#ifdef USE_ITHREADS
if (o->op_type == OP_GV || o->op_type == OP_GVSV ||
- o->op_type == OP_AELEMFAST || o->op_type == OP_RCATLINE)
+ o->op_type == OP_RCATLINE)
return OPc_PADOP;
#endif
--- perl-current/ext/B/t/optree_misc.t 1969-12-31 19:00:00.000000000 -0500
+++ perl@26919/ext/B/t/optree_misc.t 2006-01-23 22:41:52.707249668 -0500
@@ -0,0 +1,72 @@
+#!perl
+
+BEGIN {
+ if ($ENV{PERL_CORE}){
+ chdir('t') if -d 't';
+ @INC = ('.', '../lib', '../ext/B/t');
+ } else {
+ unshift @INC, 't';
+ push @INC, "../../t";
+ }
+ require Config;
+ if (($Config::Config{'extensions'} !~ /\bB\b/) ){
+ print "1..0 # Skip -- Perl configured without B module\n";
+ exit 0;
+ }
+}
+use OptreeCheck;
+use Config;
+plan tests => 1;
+
+SKIP: {
+skip "no perlio in this build", 1 unless $Config::Config{useperlio};
+
+# The regression this is testing is that the first aelemfast, derived
+# from a lexical array, is supposed to be a BASEOP "<0>", while the
+# second, from a global, is an SVOP "<$>" or a PADOP "<#>" depending
+# on threading. In buggy versions, both showed up as SVOPs/PADOPs. See
+# B.xs:cc_opclass() for the relevant code.
+
+checkOptree ( name => 'OP_AELEMFAST opclass',
+ code => sub { my @x; our @y; $x[0] + $y[0]},
+ expect => <<'EOT_EOT', expect_nt => <<'EONT_EONT');
+# a <1> leavesub[1 ref] K/REFC,1 ->(end)
+# - <@> lineseq KP ->a
+# 1 <;> nextstate(main 634 optree_misc.t:25) v ->2
+# 2 <0> padav[@x:634,636] vM/LVINTRO ->3
+# 3 <;> nextstate(main 635 optree_misc.t:25) v ->4
+# 5 <1> rv2av[t4] vK/OURINTR,1 ->6
+# 4 <#> gv[*y] s ->5
+# 6 <;> nextstate(main 636 optree_misc.t:25) v ->7
+# 9 <2> add[t6] sK/2 ->a
+# - <1> ex-aelem sK/2 ->8
+# 7 <0> aelemfast[@x:634,636] sR* ->8
+# - <0> ex-const s ->-
+# - <1> ex-aelem sK/2 ->9
+# - <1> ex-rv2av sKR/1 ->-
+# 8 <#> aelemfast[*y] s ->9
+# - <0> ex-const s ->-
+EOT_EOT
+# a <1> leavesub[1 ref] K/REFC,1 ->(end)
+# - <@> lineseq KP ->a
+# 1 <;> nextstate(main 634 optree_misc.t:27) v ->2
+# 2 <0> padav[@x:634,636] vM/LVINTRO ->3
+# 3 <;> nextstate(main 635 optree_misc.t:27) v ->4
+# 5 <1> rv2av[t3] vK/OURINTR,1 ->6
+# 4 <$> gv(*y) s ->5
+# 6 <;> nextstate(main 636 optree_misc.t:27) v ->7
+# 9 <2> add[t4] sK/2 ->a
+# - <1> ex-aelem sK/2 ->8
+# 7 <0> aelemfast[@x:634,636] sR* ->8
+# - <0> ex-const s ->-
+# - <1> ex-aelem sK/2 ->9
+# - <1> ex-rv2av sKR/1 ->-
+# 8 <$> aelemfast(*y) s ->9
+# - <0> ex-const s ->-
+EONT_EONT
+
+
+} #skip
+
+__END__
+
Thread Next
-
aelemfast optimization opclass problem (c.f. [perl #38279])
by Stephen McCamant