The MAD leakage fixed

MAD leaks shared hash keys if PERL_DESTRUCT_LEVEL is set

The minimal test case I can find is:

BEGIN {
    $^H{whatever} = 1;
}

if (1) {}
else {
    1
}
__END__

Basically, the problem is that the ops that would have been freed during
dead code elimination aren't at the time, and the implementation of use
pragmata means that the COP holds a shared hash key.

It looks like the ops in the dead branch are never freed. The confusing part
is that valgrind reports that they're still reachable.

The troublesome code is in newCONDOP:

	if (SvTRUE(((SVOP*)first)->op_sv)) {
#ifdef PERL_MAD
	    if (PL_madskills) {
		trueop = newUNOP(OP_NULL, 0, trueop);
		op_getmad(first,trueop,'C');
		op_getmad(falseop,trueop,'e');
	    }
	    /* FIXME for MAD - should there be an ELSE here?  */
#else
	    op_free(first);
	    op_free(falseop);
#endif
	    return trueop;
	}

and that FIXME seems to be correctly identify the problem - we're returning
the non-dead branch, without hanging the other ops from it.

I applied the appended change, which feels like the right change to make,
and certainly stops the warnings.

Nicholas Clark

Change 30605 by nicholas@nicholas-saigo on 2007/03/16 19:29:04

	Under -Dmad, free the ops representing the dead code when a branch
	conditional value is actually a constant. Previously this was leaking
	ops, and more visibly shared hash keys.

Affected files ...

... //depot/perl/op.c#896 edit

Differences ...

==== //depot/perl/op.c#896 (text) ====

@@ -4246,12 +4246,12 @@
 		trueop = newUNOP(OP_NULL, 0, trueop);
 		op_getmad(first,trueop,'C');
 		op_getmad(falseop,trueop,'e');
+	    } else
+#endif
+	    {
+		op_free(first);
+		op_free(falseop);
 	    }
-	    /* FIXME for MAD - should there be an ELSE here?  */
-#else
-	    op_free(first);
-	    op_free(falseop);
-#endif
 	    return trueop;
 	}
 	else {
@@ -4260,12 +4260,12 @@
 		falseop = newUNOP(OP_NULL, 0, falseop);
 		op_getmad(first,falseop,'C');
 		op_getmad(trueop,falseop,'t');
+	    } else
+#endif
+	    {
+		op_free(first);
+		op_free(trueop);
 	    }
-	    /* FIXME for MAD - should there be an ELSE here?  */
-#else
-	    op_free(first);
-	    op_free(trueop);
-#endif
 	    return falseop;
 	}
     }

• The MAD leakage fixed by Nicholas Clark