On Mon, Jan 27, 2014 at 09:43:49PM +0000, Nicholas Clark wrote:

> With this, core setting just produces 1 error from valgrind:

> I'm not sure how to find that one.

Writing everything plausible to /dev/null helped a lot, as it caused valgrind
to produce an earlier backtrace:

diff --cc src/mast/compiler.c
index 4f795b3,4f795b3..ced014d
--- a/src/mast/compiler.c
+++ b/src/mast/compiler.c
@@@ -12,6 -12,6 +12,14 @@@
  #define MOARVM_IN_COMPILER
  #endif
  
++static void *
++smurf(void *d, void *s, size_t n) {
++    int fd = open("/dev/null", O_RDONLY);
++    write(fd, s, n);
++    close(fd);
++    return memcpy(d, s, n);
++}
++
  /* Some constants. */
  #define HEADER_SIZE                 92
  #define BYTECODE_VERSION            2
@@@ -199,7 -199,7 +207,7 @@@ static void memcpy_endian(char *dest, v
      for (i = 0; i < size; i++)
          dest[size - i - 1] = srcbytes[i];
  #else
--    memcpy(dest, src, size);
++    smurf(dest, src, size);
  #endif
  }
  
@@@ -1103,9 -1103,9 +1111,9 @@@ char * form_string_heap(VM, WriterStat
  
          /* Write string. */
  #ifdef PARROT_OPS_BUILD
--        memcpy(heap + heap_size, utf8->strstart, bytelen);
++        smurf(heap + heap_size, utf8->strstart, bytelen);
  #else
--        memcpy(heap + heap_size, utf8, bytelen);
++        smurf(heap + heap_size, utf8, bytelen);
          free(utf8);
  #endif
          heap_size += bytelen;
@@@ -1158,38 -1158,38 +1166,38 @@@ char * form_bytecode_output(VM, WriterS
      memset(output, 0, size);
  
      /* Generate start of header. */
--    memcpy(output, "MOARVM\r\n", 8);
++    smurf(output, "MOARVM\r\n", 8);
      write_int32(output, 8, BYTECODE_VERSION);
      pos += HEADER_SIZE;
  
      /* Add SC dependencies section and its header entries. */
      write_int32(output, SCDEP_HEADER_OFFSET, pos);
      write_int32(output, SCDEP_HEADER_OFFSET + 4, ELEMS(vm, 
ws->cu->sc_handles));
--    memcpy(output + pos, ws->scdep_seg, ws->scdep_bytes);
++    smurf(output + pos, ws->scdep_seg, ws->scdep_bytes);
      pos += ws->scdep_bytes;
  
      /* Add extension ops section and its header entries. */
      write_int32(output, EXTOP_HEADER_OFFSET, pos);
      write_int32(output, EXTOP_HEADER_OFFSET + 4, ws->num_extops);
--    memcpy(output + pos, ws->extops_seg, ws->extops_bytes);
++    smurf(output + pos, ws->extops_seg, ws->extops_bytes);
      pos += ws->extops_bytes;
  
      /* Add frames section and its header entries. */
      write_int32(output, FRAME_HEADER_OFFSET, pos);
      write_int32(output, FRAME_HEADER_OFFSET + 4, ws->num_frames);
--    memcpy(output + pos, ws->frame_seg, ws->frame_pos);
++    smurf(output + pos, ws->frame_seg, ws->frame_pos);
      pos += ws->frame_pos;
  
      /* Add callsites section and its header entries. */
      write_int32(output, CALLSITE_HEADER_OFFSET, pos);
      write_int32(output, CALLSITE_HEADER_OFFSET + 4, ws->num_callsites);
--    memcpy(output + pos, ws->callsite_seg, ws->callsite_pos);
++    smurf(output + pos, ws->callsite_seg, ws->callsite_pos);
      pos += ws->callsite_pos;
  
      /* Add strings heap section and its header entries. */
      write_int32(output, STRING_HEADER_OFFSET, pos);
      write_int32(output, STRING_HEADER_OFFSET + 4, ELEMS(vm, ws->strings));
--    memcpy(output + pos, string_heap, string_heap_size);
++    smurf(output + pos, string_heap, string_heap_size);
      pos += string_heap_size;
      if (string_heap) {
          free(string_heap);
@@@ -1200,7 -1200,7 +1208,7 @@@
      if (vm->serialized) {
          write_int32(output, SCDATA_HEADER_OFFSET, pos);
          write_int32(output, SCDATA_HEADER_OFFSET + 4, vm->serialized_size);
--        memcpy(output + pos, vm->serialized, vm->serialized_size);
++        smurf(output + pos, vm->serialized, vm->serialized_size);
          pos += vm->serialized_size;
          free(vm->serialized);
          vm->serialized = NULL;
@@@ -1210,13 -1210,13 +1218,13 @@@
      /* Add bytecode section and its header entries (offset, length). */
      write_int32(output, BYTECODE_HEADER_OFFSET, pos);
      write_int32(output, BYTECODE_HEADER_OFFSET + 4, ws->bytecode_pos);
--    memcpy(output + pos, ws->bytecode_seg, ws->bytecode_pos);
++    smurf(output + pos, ws->bytecode_seg, ws->bytecode_pos);
      pos += ws->bytecode_pos;
  
      /* Add annotation section and its header entries (offset, length). */
      write_int32(output, ANNOTATION_HEADER_OFFSET, pos);
      write_int32(output, ANNOTATION_HEADER_OFFSET + 4, ws->annotation_pos);
--    memcpy(output + pos, ws->annotation_seg, ws->annotation_pos);
++    smurf(output + pos, ws->annotation_seg, ws->annotation_pos);
      pos += ws->annotation_pos;
  
      /* Add HLL and special frame indexes. */


And the answer is "the padding bytes"

Nicholas Clark
>From e6c003b1a881394417c52d374263b80f51ac0c78 Mon Sep 17 00:00:00 2001
From: Nicholas Clark <n...@ccl4.org>
Date: Tue, 28 Jan 2014 12:06:00 +0100
Subject: [PATCH] In form_string_heap(), ensure that the padding bytes are initialised.

Whilst we never read this memory, it's useful to ensure it is initialised,
otherwise valgrind (and similar tools) will rightly complain that we're
writing garbage to disk.

With this change, compiling the setting runs without error under valgrind.
---
 src/mast/compiler.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/mast/compiler.c b/src/mast/compiler.c
index 2ce9ac2..4f795b3 100644
--- a/src/mast/compiler.c
+++ b/src/mast/compiler.c
@@ -1110,8 +1110,13 @@ char * form_string_heap(VM, WriterState *ws, unsigned int *string_heap_size) {
 #endif
         heap_size += bytelen;
 
-        /* Add alignment. */
-        heap_size += align;
+        /* Add alignment. Whilst we never read this memory, it's useful to
+           ensure it is initialised, otherwise valgrind (and similar tools)
+           will rightly complain that we're writing garbage to disk. */
+        if (align) {
+            memset(heap + heap_size, 0, align);
+            heap_size += align;
+        }
     }
 
     *string_heap_size = heap_size;
-- 
1.7.1

Reply via email to