Change 29950 by [EMAIL PROTECTED] on 2007/01/24 15:42:09
Integrate:
[ 27801]
Subject: [PATCH] doop.c: (Coverity) found a bug but not quite what
Coverity thought it did (try valgrind on the new bop.t without the doop.c patch)
From: [EMAIL PROTECTED] (Jarkko Hietaniemi)
Date: Thu, 13 Apr 2006 19:20:46 +0300 (EEST)
Message-Id: <[EMAIL PROTECTED]>
[ 27856]
The danger of piping an mbox to patch is that it contains more than
one message. So both:
Subject: [PATCH] doop.c: one more code path where memory could leak
(Coverity)
From: [EMAIL PROTECTED] (Jarkko Hietaniemi)
Message-Id: <[EMAIL PROTECTED]>
Date: Sun, 16 Apr 2006 11:19:25 +0300 (EEST)
and
Subject: [PATCH] doop.c: one more code path where memory could leak
(Coverity)
From: [EMAIL PROTECTED] (Jarkko Hietaniemi)
Message-Id: <[EMAIL PROTECTED]>
Date: Sun, 16 Apr 2006 11:19:25 +0300 (EEST)
[ 27857]
Revert 27856.
[ 27859]
Subject: [PATCH] dooop.c: the strong asserts in Sv* macros could cause
memory leakage -- move the macro calls earlier (Coverity CID 84)
From: [EMAIL PROTECTED] (Jarkko Hietaniemi)
Message-Id: <[EMAIL PROTECTED]>
Date: Mon, 17 Apr 2006 10:19:37 +0300 (EEST)
[ 27883]
Coverity still thinks that there is a route through do_vop that can
leak resources. I believe that it's spotted that you can skip all the
cases in the switch. Plug that hole.
Affected files ...
... //depot/maint-5.8/perl/doop.c#44 integrate
... //depot/maint-5.8/perl/pod/perlapi.pod#86 integrate
... //depot/maint-5.8/perl/sv.c#308 integrate
... //depot/maint-5.8/perl/t/op/bop.t#6 integrate
Differences ...
==== //depot/maint-5.8/perl/doop.c#44 (text) ====
Index: perl/doop.c
--- perl/doop.c#43~29802~ 2007-01-13 16:36:51.000000000 -0800
+++ perl/doop.c 2007-01-24 07:42:09.000000000 -0800
@@ -1200,6 +1200,8 @@
len = leftlen < rightlen ? leftlen : rightlen;
lensave = len;
+ SvCUR_set(sv, len);
+ (void)SvPOK_only(sv);
if ((left_utf || right_utf) && (sv == left || sv == right)) {
needlen = optype == OP_BIT_AND ? len : leftlen + rightlen;
Newxz(dc, needlen + 1, char);
@@ -1220,11 +1222,10 @@
(void)sv_usepvn(sv, dc, needlen);
dc = SvPVX(sv); /* sv_usepvn() calls Renew() */
}
- SvCUR_set(sv, len);
- (void)SvPOK_only(sv);
if (left_utf || right_utf) {
UV duc, luc, ruc;
- char * const dcsave = dc;
+ char *dcorig = dc;
+ char *dcsave = NULL;
STRLEN lulen = leftlen;
STRLEN rulen = rightlen;
STRLEN ulen;
@@ -1242,8 +1243,8 @@
dc = (char*)uvchr_to_utf8((U8*)dc, duc);
}
if (sv == left || sv == right)
- (void)sv_usepvn(sv, dcsave, needlen);
- SvCUR_set(sv, dc - dcsave);
+ (void)sv_usepvn(sv, dcorig, needlen);
+ SvCUR_set(sv, dc - dcorig);
break;
case OP_BIT_XOR:
while (lulen && rulen) {
@@ -1269,16 +1270,26 @@
dc = (char*)uvchr_to_utf8((U8*)dc, duc);
}
mop_up_utf:
+ if (rulen)
+ dcsave = savepvn(rc, rulen);
+ else if (lulen)
+ dcsave = savepvn(lc, lulen);
if (sv == left || sv == right)
- (void)sv_usepvn(sv, dcsave, needlen);
- SvCUR_set(sv, dc - dcsave);
+ (void)sv_usepvn(sv, dcorig, needlen); /* Uses Renew(). */
+ SvCUR_set(sv, dc - dcorig);
if (rulen)
- sv_catpvn(sv, rc, rulen);
+ sv_catpvn(sv, dcsave, rulen);
else if (lulen)
- sv_catpvn(sv, lc, lulen);
+ sv_catpvn(sv, dcsave, lulen);
else
*SvEND(sv) = '\0';
+ Safefree(dcsave);
break;
+ default:
+ if (sv == left || sv == right)
+ Safefree(dcorig);
+ Perl_croak(aTHX_ "panic: do_vop called for op %u (%s)", optype,
+ PL_op_name[optype]);
}
SvUTF8_on(sv);
goto finish;
==== //depot/maint-5.8/perl/pod/perlapi.pod#86 (text+w) ====
Index: perl/pod/perlapi.pod
--- perl/pod/perlapi.pod#85~29928~ 2007-01-22 15:12:43.000000000 -0800
+++ perl/pod/perlapi.pod 2007-01-24 07:42:09.000000000 -0800
@@ -5568,12 +5568,14 @@
=item sv_usepvn
X<sv_usepvn>
-Tells an SV to use C<ptr> to find its string value. Normally the string is
-stored inside the SV but sv_usepvn allows the SV to use an outside string.
-The C<ptr> should point to memory that was allocated by C<malloc>. The
-string length, C<len>, must be supplied. This function will realloc the
-memory pointed to by C<ptr>, so that pointer should not be freed or used by
-the programmer after giving it to sv_usepvn. Does not handle 'set' magic.
+Tells an SV to use C<ptr> to find its string value. Normally the
+string is stored inside the SV but sv_usepvn allows the SV to use an
+outside string. The C<ptr> should point to memory that was allocated
+by C<malloc>. The string length, C<len>, must be supplied. This
+function will realloc (i.e. move) the memory pointed to by C<ptr>,
+so that pointer should not be freed or used by the programmer after
+giving it to sv_usepvn, and neither should any pointers from "behind"
+that pointer (e.g. ptr + 1) be used. Does not handle 'set' magic.
See C<sv_usepvn_mg>.
void sv_usepvn(SV* sv, char* ptr, STRLEN len)
==== //depot/maint-5.8/perl/sv.c#308 (text) ====
Index: perl/sv.c
--- perl/sv.c#307~29949~ 2007-01-24 07:28:38.000000000 -0800
+++ perl/sv.c 2007-01-24 07:42:09.000000000 -0800
@@ -3614,12 +3614,14 @@
/*
=for apidoc sv_usepvn
-Tells an SV to use C<ptr> to find its string value. Normally the string is
-stored inside the SV but sv_usepvn allows the SV to use an outside string.
-The C<ptr> should point to memory that was allocated by C<malloc>. The
-string length, C<len>, must be supplied. This function will realloc the
-memory pointed to by C<ptr>, so that pointer should not be freed or used by
-the programmer after giving it to sv_usepvn. Does not handle 'set' magic.
+Tells an SV to use C<ptr> to find its string value. Normally the
+string is stored inside the SV but sv_usepvn allows the SV to use an
+outside string. The C<ptr> should point to memory that was allocated
+by C<malloc>. The string length, C<len>, must be supplied. This
+function will realloc (i.e. move) the memory pointed to by C<ptr>,
+so that pointer should not be freed or used by the programmer after
+giving it to sv_usepvn, and neither should any pointers from "behind"
+that pointer (e.g. ptr + 1) be used. Does not handle 'set' magic.
See C<sv_usepvn_mg>.
=cut
==== //depot/maint-5.8/perl/t/op/bop.t#6 (xtext) ====
Index: perl/t/op/bop.t
--- perl/t/op/bop.t#5~27391~ 2006-03-06 10:13:50.000000000 -0800
+++ perl/t/op/bop.t 2007-01-24 07:42:09.000000000 -0800
@@ -15,7 +15,7 @@
# If you find tests are failing, please try adding names to tests to track
# down where the failure is, and supply your new names as a patch.
# (Just-in-time test naming)
-plan tests => 148;
+plan tests => 160;
# numerics
ok ((0xdead & 0xbeef) == 0x9ead);
@@ -343,3 +343,75 @@
is(~~$y, "c");
is(fetches($y), 1);
is(stores($y), 0);
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $c = $a | $b;
+ is($c, chr(0x1FF) x 0xFF . chr(0x101) x 2);
+
+ $c = $b | $a;
+ is($c, chr(0x1FF) x 0xFF . chr(0x101) x 2);
+
+ $c = $a & $b;
+ is($c, chr(0x001) x 0x0FF);
+
+ $c = $b & $a;
+ is($c, chr(0x001) x 0x0FF);
+
+ $c = $a ^ $b;
+ is($c, chr(0x1FE) x 0x0FF . chr(0x101) x 2);
+
+ $c = $b ^ $a;
+ is($c, chr(0x1FE) x 0x0FF . chr(0x101) x 2);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $a |= $b;
+ is($a, chr(0x1FF) x 0xFF . chr(0x101) x 2);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $b |= $a;
+ is($b, chr(0x1FF) x 0xFF . chr(0x101) x 2);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $a &= $b;
+ is($a, chr(0x001) x 0x0FF);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $b &= $a;
+ is($b, chr(0x001) x 0x0FF);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $a ^= $b;
+ is($a, chr(0x1FE) x 0x0FF . chr(0x101) x 2);
+}
+
+{
+ $a = chr(0x101) x 0x101;
+ $b = chr(0x0FF) x 0x0FF;
+
+ $b ^= $a;
+ is($b, chr(0x1FE) x 0x0FF . chr(0x101) x 2);
+}
+
End of Patch.
