----- 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