Skip to content

Commit b9ad13a

Browse files
nwc10obra
authored andcommitted
Fix for non-regexps being upgraded to SVt_REGEXP
$ ./perl -lwe '$a = ${qr//}; $a = 2; print re::is_regexp(\$a)' 1 It is possible for arbitrary SVs (eg PAD entries) to be upgraded to SVt_REGEXP. (This is new with first class regexps) Whilst the example above does not SEGV, it will be possible to write code that will cause SEGVs (or worse) at the point when the scalar is freed, because the code in sv_clear() assumes that all scalars of type SVt_REGEXP *are* regexps, and passes them to pregfree2(), which assumes that pointers within are valid.
1 parent 3141af4 commit b9ad13a

File tree

4 files changed

+65
-5
lines changed

4 files changed

+65
-5
lines changed

ext/Devel-Peek/t/Peek.t

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ do_test(15,
326326
RV = $ADDR
327327
SV = REGEXP\\($ADDR\\) at $ADDR
328328
REFCNT = 1
329-
FLAGS = \\(OBJECT,POK,pPOK\\)
329+
FLAGS = \\(OBJECT,POK,FAKE,pPOK\\)
330330
IV = 0
331331
PV = $ADDR "\\(\\?-xism:tic\\)"
332332
CUR = 12

regcomp.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9457,6 +9457,7 @@ Perl_reg_temp_copy (pTHX_ REGEXP *ret_x, REGEXP *rx)
94579457
memcpy(&(ret->xpv_cur), &(r->xpv_cur),
94589458
sizeof(regexp) - STRUCT_OFFSET(regexp, xpv_cur));
94599459
SvLEN_set(ret_x, 0);
9460+
SvSTASH_set(ret_x, NULL);
94609461
Newx(ret->offs, npar, regexp_paren_pair);
94619462
Copy(r->offs, ret->offs, npar, regexp_paren_pair);
94629463
if (r->substrs) {

sv.c

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1372,6 +1372,10 @@ Perl_sv_upgrade(pTHX_ register SV *const sv, svtype new_type)
13721372
break;
13731373

13741374

1375+
case SVt_REGEXP:
1376+
/* This ensures that SvTHINKFIRST(sv) is true, and hence that
1377+
sv_force_normal_flags(sv) is called. */
1378+
SvFAKE_on(sv);
13751379
case SVt_PVIV:
13761380
/* XXX Is this still needed? Was it ever needed? Surely as there is
13771381
no route from NV to PVIV, NOK can never be true */
@@ -1382,7 +1386,6 @@ Perl_sv_upgrade(pTHX_ register SV *const sv, svtype new_type)
13821386
case SVt_PVGV:
13831387
case SVt_PVCV:
13841388
case SVt_PVLV:
1385-
case SVt_REGEXP:
13861389
case SVt_PVMG:
13871390
case SVt_PVNV:
13881391
case SVt_PV:
@@ -4615,6 +4618,45 @@ Perl_sv_force_normal_flags(pTHX_ register SV *const sv, const U32 flags)
46154618
sv_unref_flags(sv, flags);
46164619
else if (SvFAKE(sv) && SvTYPE(sv) == SVt_PVGV)
46174620
sv_unglob(sv);
4621+
else if (SvFAKE(sv) && SvTYPE(sv) == SVt_REGEXP) {
4622+
/* Need to downgrade the REGEXP to a simple(r) scalar. This is analagous
4623+
to sv_unglob. We only need it here, so inline it. */
4624+
const svtype new_type = SvMAGIC(sv) || SvSTASH(sv) ? SVt_PVMG : SVt_PV;
4625+
SV *const temp = newSV_type(new_type);
4626+
void *const temp_p = SvANY(sv);
4627+
4628+
if (new_type == SVt_PVMG) {
4629+
SvMAGIC_set(temp, SvMAGIC(sv));
4630+
SvMAGIC_set(sv, NULL);
4631+
SvSTASH_set(temp, SvSTASH(sv));
4632+
SvSTASH_set(sv, NULL);
4633+
}
4634+
SvCUR_set(temp, SvCUR(sv));
4635+
/* Remember that SvPVX is in the head, not the body. */
4636+
if (SvLEN(temp)) {
4637+
SvLEN_set(temp, SvLEN(sv));
4638+
/* This signals "buffer is owned by someone else" in sv_clear,
4639+
which is the least effort way to stop it freeing the buffer.
4640+
*/
4641+
SvLEN_set(sv, SvLEN(sv)+1);
4642+
} else {
4643+
/* Their buffer is already owned by someone else. */
4644+
SvPVX(sv) = savepvn(SvPVX(sv), SvCUR(sv));
4645+
SvLEN_set(temp, SvCUR(sv)+1);
4646+
}
4647+
4648+
/* Now swap the rest of the bodies. */
4649+
4650+
SvFLAGS(sv) &= ~(SVf_FAKE|SVTYPEMASK);
4651+
SvFLAGS(sv) |= new_type;
4652+
SvANY(sv) = SvANY(temp);
4653+
4654+
SvFLAGS(temp) &= ~(SVTYPEMASK);
4655+
SvFLAGS(temp) |= SVt_REGEXP|SVf_FAKE;
4656+
SvANY(temp) = temp_p;
4657+
4658+
SvREFCNT_dec(temp);
4659+
}
46184660
}
46194661

46204662
/*

t/op/qr.t

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use strict;
44

55
require './test.pl';
66

7-
plan(tests => 12);
7+
plan(tests => 18);
88

99
sub r {
1010
return qr/Good/;
@@ -37,5 +37,22 @@ isnt($c + 0, $d + 0, 'Not the same object');
3737
$$d = 'Bad';
3838

3939
like("$c", qr/Good/);
40-
like("$d", qr/Bad/);
41-
like("$d1", qr/Bad/);
40+
is($$d, 'Bad');
41+
is($$d1, 'Bad');
42+
43+
# Assignment to an implicitly blessed Regexp object retains the class
44+
# (No different from direct value assignment to any other blessed SV
45+
46+
isa_ok($d, 'Regexp');
47+
like("$d", qr/\ARegexp=SCALAR\(0x[0-9a-f]+\)\z/);
48+
49+
# As does an explicitly blessed Regexp object.
50+
51+
my $e = bless qr/Faux Pie/, 'Stew';
52+
53+
isa_ok($e, 'Stew');
54+
$$e = 'Fake!';
55+
56+
is($$e, 'Fake!');
57+
isa_ok($e, 'Stew');
58+
like("$e", qr/\Stew=SCALAR\(0x[0-9a-f]+\)\z/);

0 commit comments

Comments
 (0)