[Python-Dev] Invalid memory read in PyObject_Free

[Neal Norwitz]

On Fri, Jul 04, 2003 at 03:53:44PM -0400, Tim Peters wrote:
> [Guido]
> > I haven't heard of platforms where turning off pymalloc is required --
> > unless we hear about those, I expect that for 2.4, pymalloc may no
> > longer be optional.  (The reason: maintaining two versions of the same
> > code is a pain, and usually the version that's not selected by default
> > is severely broken after a few releases.)
> 
> We never build without WITH_PYMALLOC defined anymore, so under the "if it's
> not tested, it's broken" theory, it's already broken <0.5 wink>.  OTOH,
> there are really only two substantive WITH_PYMALLOC #ifdefs in the codebase,
> and one of them just surrounds the bulk of the code in obmalloc.c.  So as
> untested features go, I bet this one is less problematic than
> WITHOUT_COMPLEX (which is tested in many more places!).

Depends on your definition of we. :-)

I never got around to submitting the attached patch which
makes it easier and safer to use memory testing tools such
as Purify and Valgrind.  The suppression then only needs to
be applied to ADDRESS_IN_RANGE.

I haven't noticed a problem with pymalloc on Linux, Solaris, Tru64,
AIX, HP-UX, FreeBSD.  So far there haven't seemed to be any problems
with pymalloc.

Somtimes, there are benefits to turning off pymalloc from time to time
in order to diagnose memory in use and some other memory related
issues.  Usually, pymalloc is a big win.

Neal
--
Index: Objects/obmalloc.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Objects/obmalloc.c,v
retrieving revision 2.51
diff -w -u -r2.51 obmalloc.c
--- Objects/obmalloc.c  17 Jun 2003 15:48:11 -0000      2.51
+++ Objects/obmalloc.c  5 Jul 2003 15:53:55 -0000
@@ -534,8 +534,31 @@
  * so the (I) < narenas must be false, saving us from trying to index into
  * a NULL arenas.
  */
-#define ADDRESS_IN_RANGE(P, I) \
-       ((I) < narenas && (uptr)(P) - arenas[I] < (uptr)ARENA_SIZE)
+#define ADDRESS_IN_RANGE(P, POOL)      \
+       ((POOL)->arenaindex < narenas &&                \
+        (uptr)(P) - arenas[(POOL)->arenaindex] < (uptr)ARENA_SIZE)
+
+/* This is only useful when running memory debuggers such as
+ * Purify or Valgrind.  Uncomment to use.
+ */
+#define USING_MEMORY_DEBUGGER
+
+#ifdef USING_MEMORY_DEBUGGER
+
+/* ADDRESS_IN_RANGE may access uninitialized memory by design
+ * This leads to thousands of spurious warnings when using
+ * Purify or Valgrind.  By making a function, we can easily
+ * suppress the uninitialized memory reads in this one function.
+ * So we won't ignore real errors elsewhere.
+ *
+ * Disable the macro and use a function.
+ */
+
+#undef ADDRESS_IN_RANGE
+
+/* Don't make static, to ensure this isn't inlined. */
+int ADDRESS_IN_RANGE(void *P, poolp pool);
+#endif
  
 /*==========================================================================*/
  
@@ -708,7 +731,7 @@
                return;
  
        pool = POOL_ADDR(p);
-       if (ADDRESS_IN_RANGE(p, pool->arenaindex)) {
+       if (ADDRESS_IN_RANGE(p, pool)) {
                /* We allocated this address. */
                LOCK();
                /*
@@ -791,7 +814,7 @@
                return PyObject_Malloc(nbytes);
  
        pool = POOL_ADDR(p);
-       if (ADDRESS_IN_RANGE(p, pool->arenaindex)) {
+       if (ADDRESS_IN_RANGE(p, pool)) {
                /* We're in charge of this block */
                size = INDEX2SIZE(pool->szidx);
                if (nbytes <= size) {
@@ -1373,3 +1396,13 @@
 }
  
 #endif /* PYMALLOC_DEBUG */
+
+#ifdef USING_MEMORY_DEBUGGER
+/* Make this function last so gcc won't inline it
+   since the definition is after the reference. */
+int ADDRESS_IN_RANGE(void *P, poolp pool)
+{
+       return ((pool->arenaindex) < narenas &&
+               (uptr)(P) - arenas[pool->arenaindex] < (uptr)ARENA_SIZE);
+}
+#endif