add 1 to SvGROW under COW (and fix svleak.t)
authorDavid Mitchell <davem@iabyn.com>
Sat, 8 Jun 2013 17:24:51 +0000 (18:24 +0100)
committerDavid Mitchell <davem@iabyn.com>
Sat, 8 Jun 2013 17:24:51 +0000 (18:24 +0100)
the new COW scheme uses SvPVX(sv)[SvLEN(sv)-1] (if spare)
to store the COW reference count. If this byte isn't spare, the string is
copied rather than COWed.  So, when built under the new COW, allocate one
more byte than asked for in sv_grow(), to make it likely this byte is
always spare: and thus make more strings COW-able. If the new size is a
big power of two, don't bother: we assume the caller wanted a nice 2^N
sized block and will be annoyed at getting 2^N+1 or more.

Note that sv_grow() already rounds up. For example: formerly, strings of
length 1..15 were allocated 16 bytes, 16..31 allocated 32 bytes, etc
(usually allowing +1 for trailing \0).

Now its 1..14 => 16, 15..30 => 32 etc.

As a side-effect, this should fix a bizarre occasionally failing svleak.t
test under COW. This test,

    '"too many errors" from constant overloading returning undef'

basically twice evals some code that is expected to fail. If over the
course of the second eval the total number of allocated SVs grows, then
the test fails. This particular eval uses overload::constant, and that
module has this code:

    "$_[1]" !~ /(^|=)CODE\(0x[0-9a-f]+\)$/

i.e. match against a stringified reference. Now, if it stringifies to a
15-char string (e.g. "CODE(0x1234567)"), then SvCUR is 15 and SvLEN is 16,
and there isn't a spare byte (after allowing for \0) for the COW ref
count, so the string can't be COWed. Now, if during the first eval,
addresses are in the range 0x1000000..0xfffffff, then COW doesn't kick in.
If during the second eval they fall outside this range, COW kicks in, and
a new SV (which is a COW copy of the string) is attached to the regex, for
capture info. This new SV won't be released until the regex is released,
and so appears to be a leak. If COW kicks in during the first eval then
this isn't an issue, as it will just be reused during the second eval.

This was exceedingly difficult to reliably reproduce, as it was heavily
affected by kernel address randomisation (on linux); plus part of the test
file used rand(); plus of course the hash randomisation and perturbation
feature in 5.18.

I eventually cracked it by: tweaking the hash randomisation; hacking the
perl seed code to use an ENV var rather than /dev/urandom; hacking the
perl startup to display the initial system brk(), then when it failed,
hacked it again to set that particular brk().

sv.c

diff --git a/sv.c b/sv.c
index c45d4b1..8d1c015 100644 (file)
--- a/sv.c
+++ b/sv.c
@@ -1507,6 +1507,18 @@ Perl_sv_grow(pTHX_ SV *const sv, STRLEN newlen)
        s = SvPVX_mutable(sv);
     }
 
+#ifdef PERL_NEW_COPY_ON_WRITE
+    /* the new COW scheme uses SvPVX(sv)[SvLEN(sv)-1] (if spare)
+     * to store the COW count. So in general, allocate one more byte than
+     * asked for, to make it likely this byte is always spare: and thus
+     * make more strings COW-able.
+     * If the new size is a big power of two, don't bother: we assume the
+     * caller wanted a nice 2^N sized block and will be annoyed at getting
+     * 2^N+1 */
+    if (newlen & 0xff)
+        newlen++;
+#endif
+
     if (newlen > SvLEN(sv)) {          /* need more room? */
        STRLEN minlen = SvCUR(sv);
        minlen += (minlen >> PERL_STRLEN_EXPAND_SHIFT) + 10;