----- Original Message ----- From: "chromatic" <[EMAIL PROTECTED]>
To: <[EMAIL PROTECTED]>
Sent: Saturday, March 15, 2008 8:58 PM
Subject: Re: [svn:parrot] r26390 - trunk/src


On Saturday 15 March 2008 05:48:09 [EMAIL PROTECTED] wrote:

New Revision: 26390

Modified:
   trunk/src/inter_call.c

Log:
Prevent overrun of array. Found using valgrind while chasing down tcl test
failures on linux x86-64.

Modified: trunk/src/inter_call.c
===========================================================================
=== --- trunk/src/inter_call.c (original)
+++ trunk/src/inter_call.c Sat Mar 15 05:48:08 2008
@@ -1191,6 +1191,9 @@
                 idx = st->dest.u.op.pc[i];
                 store_arg(st, idx);

+                /* Don't walk off the end of the array */
+                if (i+1 >= st->dest.n)
+                    continue;
arg_sig = st->dest.sig = SIG_ITEM(st->dest.u.op.signature,
i+1); if (arg_sig & PARROT_ARG_OPT_FLAG) {
                     i++;

That explains some weirdness I saw, but I wonder if it's papering over
something else.

I couldn't explain *why* we were getting apparently invalid bytecode here, but
something I did made it go away for me.

-- c


I thought the same thing, but decided some sort of defensive check was required anyway, and I got lost trying to track it further. It served my purpose at the time, which was to resolve a very fragile bug that moved or disappeared while trying to pin it down. If you want to track the root cause, just replace the new lines by PARROT_ASSERT(i+1 < st->dest.n), which seems like a reasonable defense to leave there.

Incidentally, I found the following useful to stop valgrind complaining about uninitialized values causes by walking the stack:

Index: include/parrot/parrot.h
===================================================================
--- include/parrot/parrot.h     (revision 26389)
+++ include/parrot/parrot.h     (working copy)
@@ -108,6 +108,12 @@
#  include <limits.h>
#endif /* PARROT_HAS_HEADER_LIMITS */

+#ifdef VALGRIND
+#  include "valgrind/valgrind.h"
+#  include "valgrind/memcheck.h"
+#endif
+
#define NUM_REGISTERS 32
#define PARROT_MAGIC 0x13155a1

Index: src/gc/dod.c
===================================================================
--- src/gc/dod.c        (revision 26389)
+++ src/gc/dod.c        (working copy)
@@ -883,6 +883,10 @@
        lo_var_ptr           = tmp_ptr;
    }

+#  ifdef VALGRIND
+    VALGRIND_MAKE_READABLE(hi_var_ptr, lo_var_ptr - hi_var_ptr);
+#  endif
+
    /* Get the expected prefix */
    prefix = mask & buffer_min;

--
Peter Gibbs


Reply via email to