P6opaque can end up up serialising uninitialised memory.
P6opaque's compose() needs to zero the freshly allocated
repr_data->unbox_slots.
Not all the slots are used, but serialize_repr_data() will serialise
all of them, and hence write out garbage to disk. This doesn't cause
crashes, because the unused slots are never accessed, but it does
conceal other problems. valgrind reports an awful lot of warnings,
concealing real problems, and the generated bytecode files will not be
byte-for-byte identical, preventing easy consistency checking.
I don't think that there are that many places where this happens (but I
haven't rigged the build to count them yet), so I don't think that it's
worth adding to the C structures to remember how many slots are used, and
updating the serialisation to serialise this. Particularly as the new
variable length integers store 0 as 1 byte, it's likely that the overhead of
serialising the count will be more than the saving from not serialising a
few zeros.
Patch attached.
With this, core setting just produces 1 error from valgrind:
==11990== Memcheck, a memory error detector
==11990== Copyright (C) 2002-2012, and GNU GPL'd, by Julian Seward et al.
==11990== Using Valgrind-3.8.1 and LibVEX; rerun with -h for copyright info
==11990== Command: /home/nicholas/Sandpit/moar3/bin/moar
--libpath=/home/nicholas/Sandpit/moar3/languages/nqp/lib perl6.moarvm
--setting=NULL --optimize=3 --target=mbc --stagestats
--output=CORE.setting.moarvm src/gen/m-CORE.setting
==11990==
Stage start : 0.000
Stage parse : 1700.903
Stage syntaxcheck: 0.000
Stage ast : 0.001
Stage optimize : 110.726
Stage mast : 634.316
==11990== Syscall param write(buf) points to uninitialised byte(s)
==11990== at 0x53FB650: __write_nocancel (in /lib64/libc-2.12.so)
==11990== by 0x5391D52: _IO_file_write@@GLIBC_2.2.5 (in /lib64/libc-2.12.so)
==11990== by 0x5391C19: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.12.so)
==11990== by 0x5387CCC: fwrite (in /lib64/libc-2.12.so)
==11990== by 0x4F55D8C: MVM_mast_to_file (driver.c:76)
==11990== by 0x4F0E87E: MVM_interp_run (interp.c:1263)
==11990== by 0x4F6434D: MVM_vm_run_file (moar.c:176)
==11990== by 0x400D3F: main (main.c:146)
==11990== Address 0x4a4cecc5 is 605,317 bytes inside a block of size 9,189,191
alloc'd
==11990== at 0x4C279EE: malloc (vg_replace_malloc.c:270)
==11990== by 0x4F552A5: form_bytecode_output (compiler.c:1152)
==11990== by 0x4F5591F: MVM_mast_compile (compiler.c:1315)
==11990== by 0x4F55D40: MVM_mast_to_file (driver.c:76)
==11990== by 0x4F0E87E: MVM_interp_run (interp.c:1263)
==11990== by 0x4F6434D: MVM_vm_run_file (moar.c:176)
==11990== by 0x400D3F: main (main.c:146)
==11990==
Stage mbc : 30.383
==11990==
==11990== HEAP SUMMARY:
==11990== in use at exit: 475,163,082 bytes in 2,210,555 blocks
==11990== total heap usage: 40,818,237 allocs, 38,607,682 frees,
12,409,867,903 bytes allocated
==11990==
==11990== LEAK SUMMARY:
==11990== definitely lost: 92,339,905 bytes in 1,564,719 blocks
==11990== indirectly lost: 53,682,608 bytes in 619,375 blocks
==11990== possibly lost: 12,354,562 bytes in 1,106 blocks
==11990== still reachable: 316,786,007 bytes in 25,355 blocks
==11990== suppressed: 0 bytes in 0 blocks
==11990== Rerun with --leak-check=full to see details of leaked memory
==11990==
==11990== For counts of detected and suppressed errors, rerun with: -v
==11990== Use --track-origins=yes to see where uninitialised values come from
==11990== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 9 from 9)
I'm not sure how to find that one.
Nicholas Clark
>From 9717c39e5e74cfe145e2210342e63f183478fa14 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Mon, 27 Jan 2014 22:20:54 +0100
Subject: [PATCH] In P6opaque's compose(), zero the freshly allocated repr_data->unbox_slots.
Not all the slots are used, but serialize_repr_data() will serialise all of
them, and hence write out garbage to disk. This doesn't cause crashes, because
the unuused slots are never accessed, but it does conceal other problems.
valgrind reports an awful lot of warnings, concealing real problems, and the
generated bytecode files will not be byte-for-byte identical, preventing easy
consistency checking.
---
src/6model/reprs/P6opaque.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/6model/reprs/P6opaque.c b/src/6model/reprs/P6opaque.c
index e439ccb..4edbcf1 100644
--- a/src/6model/reprs/P6opaque.c
+++ b/src/6model/reprs/P6opaque.c
@@ -769,7 +769,7 @@ static void compose(MVMThreadContext *tc, MVMSTable *st, MVMObject *info_hash) {
/* Also list in the by-repr unbox list. */
if (repr_data->unbox_slots == NULL)
- repr_data->unbox_slots = (MVMP6opaqueBoxedTypeMap *)malloc(total_attrs * sizeof(MVMP6opaqueBoxedTypeMap));
+ repr_data->unbox_slots = (MVMP6opaqueBoxedTypeMap *)calloc(total_attrs, sizeof(MVMP6opaqueBoxedTypeMap));
repr_data->unbox_slots[cur_unbox_slot].repr_id = REPR(type)->ID;
repr_data->unbox_slots[cur_unbox_slot].slot = cur_slot;
cur_unbox_slot++;
--
1.7.1
