GNOME.org
 

Re: [xslt] CDATA text nodes can cause ctxt->lasttsize to be incorrect, bug?

Daniel Veillard <veillard redhat com> writes:

>   I take patches :-) That code is so old ... 

Okay, patches attached. While I have your attention, could you take a
look at the patch I submitted for the relaxng streaming validation bug:
https://bugzilla.gnome.org/show_bug.cgi?id=512454#c2

>   Valgrind makes this a bit redundant now, except for speed and portability

It turns that this particular problem doesn't show up under valgrind
because its realloc() returns a new block of memory even if the new size
is smaller than the old size.

thank, Noam


diff --git a/xmlmemory.c b/xmlmemory.c
index 433abb8..1874ba3 100644
--- a/xmlmemory.c
+++ b/xmlmemory.c
@@ -121,12 +121,11 @@ static void *xmlMemTraceBlockAt = NULL;
 static MEMHDR *memlist = NULL;
 #endif
 
-static void debugmem_tag_error(void *addr);
+static void debugmem_error(const char *error, void *addr);
 #ifdef MEM_LIST
 static void  debugmem_list_add(MEMHDR *);
 static void debugmem_list_delete(MEMHDR *);
 #endif
-#define Mem_Tag_Err(a) debugmem_tag_error(a);
 
 #ifndef TEST_POINT
 #define TEST_POINT
@@ -147,45 +146,22 @@ xmlMallocBreakpoint(void) {
 }
 
 /**
- * xmlMallocLoc:
- * @size:  an int specifying the size in byte to allocate.
- * @file:  the file name or NULL
- * @line:  the line number
- *
- * a malloc() equivalent, with logging of the allocation info.
+ * xmlMemFillNode:
  *
- * Returns a pointer to the allocated area or NULL in case of lack of memory.
+ * fill the MEMHDR fields appropriately
  */
-
-void *
-xmlMallocLoc(size_t size, const char * file, int line)
+static void
+xmlMemFillNode(MEMHDR *p, int type, int number, int size,
+    const char *file, int line)
 {
-    MEMHDR *p;
-    void *ret;
-
-    if (!xmlMemInitialized) xmlInitMemory();
-#ifdef DEBUG_MEMORY
-    xmlGenericError(xmlGenericErrorContext,
-	    "Malloc(%d)\n",size);
-#endif
-
-    TEST_POINT
-
-    p = (MEMHDR *) malloc(RESERVE_SIZE+size);
-
-    if (!p) {
-	xmlGenericError(xmlGenericErrorContext,
-		"xmlMallocLoc : Out of free space\n");
-	xmlMemoryDump();
-	return(NULL);
-    }
     p->mh_tag = MEMTAG;
+    p->mh_type = type;
+    p->mh_number = number;
     p->mh_size = size;
-    p->mh_type = MALLOC_TYPE;
     p->mh_file = file;
     p->mh_line = line;
     xmlMutexLock(xmlMemMutex);
-    p->mh_number = ++block;
+    if (!number) p->mh_number = ++block;
     debugMemSize += size;
     debugMemBlocks++;
     if (debugMemSize > debugMaxMemSize) debugMaxMemSize = debugMemSize;
@@ -193,40 +169,50 @@ xmlMallocLoc(size_t size, const char * file, int line)
     debugmem_list_add(p);
 #endif
     xmlMutexUnlock(xmlMemMutex);
+}
 
-#ifdef DEBUG_MEMORY
-    xmlGenericError(xmlGenericErrorContext,
-	    "Malloc(%d) Ok\n",size);
-#endif
-
-    if (xmlMemStopAtBlock == p->mh_number) xmlMallocBreakpoint();
-
-    ret = HDR_2_CLIENT(p);
-
-    if (xmlMemTraceBlockAt == ret) {
-	xmlGenericError(xmlGenericErrorContext,
-			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt, size);
-	xmlMallocBreakpoint();
+/**
+ * xmlMemNodeCheck:
+ *
+ * check node for memory corruption
+ *
+ * @returns 1 if node is OK
+ */
+static int
+xmlMemNodeCheck(MEMHDR *p)
+{
+#define MEM_ERROR(error) debugmem_error(error, p); return 0
+    if (p->mh_tag != MEMTAG) { MEM_ERROR("tag"); }
+
+    switch (p->mh_type) {
+    case MALLOC_TYPE:
+    case REALLOC_TYPE:
+    case STRDUP_TYPE:
+    case MALLOC_ATOMIC_TYPE:
+    case REALLOC_ATOMIC_TYPE:
+        break;
+    default: MEM_ERROR("type");
     }
 
-    TEST_POINT
+    if (p->mh_number > block) { MEM_ERROR("block number"); }
+#undef MEM_ERROR
 
-    return(ret);
+    return 1;
 }
-
 /**
- * xmlMallocAtomicLoc:
+ * xmlMallocLocType:
  * @size:  an int specifying the size in byte to allocate.
  * @file:  the file name or NULL
  * @line:  the line number
+ * @type:  the mh_type
  *
  * a malloc() equivalent, with logging of the allocation info.
  *
  * Returns a pointer to the allocated area or NULL in case of lack of memory.
  */
 
-void *
-xmlMallocAtomicLoc(size_t size, const char * file, int line)
+static void *
+xmlMallocLocType(size_t size, const char * file, int line, int type)
 {
     MEMHDR *p;
     void *ret;
@@ -247,20 +233,7 @@ xmlMallocAtomicLoc(size_t size, const char * file, int line)
 	xmlMemoryDump();
 	return(NULL);
     }
-    p->mh_tag = MEMTAG;
-    p->mh_size = size;
-    p->mh_type = MALLOC_ATOMIC_TYPE;
-    p->mh_file = file;
-    p->mh_line = line;
-    xmlMutexLock(xmlMemMutex);
-    p->mh_number = ++block;
-    debugMemSize += size;
-    debugMemBlocks++;
-    if (debugMemSize > debugMaxMemSize) debugMaxMemSize = debugMemSize;
-#ifdef MEM_LIST
-    debugmem_list_add(p);
-#endif
-    xmlMutexUnlock(xmlMemMutex);
+    xmlMemFillNode(p, type, 0, size, file, line);
 
 #ifdef DEBUG_MEMORY
     xmlGenericError(xmlGenericErrorContext,
@@ -271,16 +244,57 @@ xmlMallocAtomicLoc(size_t size, const char * file, int line)
 
     ret = HDR_2_CLIENT(p);
 
+    TEST_POINT
+
+    return(ret);
+}
+
+/**
+ * xmlMallocLoc:
+ * @size:  an int specifying the size in byte to allocate.
+ * @file:  the file name or NULL
+ * @line:  the line number
+ *
+ * a malloc() equivalent, with logging of the allocation info.
+ *
+ * Returns a pointer to the allocated area or NULL in case of lack of memory.
+ */
+
+void *
+xmlMallocLoc(size_t size, const char * file, int line)
+{
+    void *ret = xmlMallocLocType(size, file, line, MALLOC_TYPE);
     if (xmlMemTraceBlockAt == ret) {
 	xmlGenericError(xmlGenericErrorContext,
-			"%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt, size);
+            "%p : Malloc(%ld) Ok\n", xmlMemTraceBlockAt, size);
 	xmlMallocBreakpoint();
     }
+    return ret;
+}
 
-    TEST_POINT
+/**
+ * xmlMallocAtomicLoc:
+ * @size:  an int specifying the size in byte to allocate.
+ * @file:  the file name or NULL
+ * @line:  the line number
+ *
+ * a malloc() equivalent, with logging of the allocation info.
+ *
+ * Returns a pointer to the allocated area or NULL in case of lack of memory.
+ */
 
-    return(ret);
+void *
+xmlMallocAtomicLoc(size_t size, const char * file, int line)
+{
+    void *ret = xmlMallocLocType(size, file, line, MALLOC_ATOMIC_TYPE);
+    if (xmlMemTraceBlockAt == ret) {
+	xmlGenericError(xmlGenericErrorContext,
+            "%p : MallocAtomic(%ld) Ok\n", xmlMemTraceBlockAt, size);
+	xmlMallocBreakpoint();
+    }
+    return ret;
 }
+
 /**
  * xmlMemMalloc:
  * @size:  an int specifying the size in byte to allocate.
@@ -297,6 +311,27 @@ xmlMemMalloc(size_t size)
 }
 
 /**
+ * xmlMemSetNodeDealloc:
+ *
+ * set the fields of @p to indicated it has been deallocated
+ *
+ * @returns mh_size of @p
+ */
+static int
+xmlMemSetNodeDealloc(MEMHDR *p)
+{
+    p->mh_tag = ~MEMTAG;
+    xmlMutexLock(xmlMemMutex);
+    debugMemSize -= p->mh_size;
+    debugMemBlocks--;
+#ifdef MEM_LIST
+    debugmem_list_delete(p);
+#endif
+    xmlMutexUnlock(xmlMemMutex);
+
+    return p->mh_size;
+}
+/**
  * xmlReallocLoc:
  * @ptr:  the initial memory block pointer
  * @size:  an int specifying the size in byte to allocate.
@@ -326,21 +361,11 @@ xmlReallocLoc(void *ptr,size_t size, const char * file, int line)
     p = CLIENT_2_HDR(ptr);
     number = p->mh_number;
     if (xmlMemStopAtBlock == number) xmlMallocBreakpoint();
-    if (p->mh_tag != MEMTAG) {
-       Mem_Tag_Err(p);
-	 goto error;
-    }
-    p->mh_tag = ~MEMTAG;
-    xmlMutexLock(xmlMemMutex);
-    debugMemSize -= p->mh_size;
-    debugMemBlocks--;
+    if (!xmlMemNodeCheck(p)) goto error;
 #ifdef DEBUG_MEMORY
-    oldsize = p->mh_size;
+    oldsize =
 #endif
-#ifdef MEM_LIST
-    debugmem_list_delete(p);
-#endif
-    xmlMutexUnlock(xmlMemMutex);
+        xmlMemSetNodeDealloc(p);
 
     p = (MEMHDR *) realloc(p,RESERVE_SIZE+size);
     if (!p) {
@@ -352,20 +377,7 @@ xmlReallocLoc(void *ptr,size_t size, const char * file, int line)
 			xmlMemTraceBlockAt, p->mh_size, size);
 	xmlMallocBreakpoint();
     }
-    p->mh_tag = MEMTAG;
-    p->mh_number = number;
-    p->mh_type = REALLOC_TYPE;
-    p->mh_size = size;
-    p->mh_file = file;
-    p->mh_line = line;
-    xmlMutexLock(xmlMemMutex);
-    debugMemSize += size;
-    debugMemBlocks++;
-    if (debugMemSize > debugMaxMemSize) debugMaxMemSize = debugMemSize;
-#ifdef MEM_LIST
-    debugmem_list_add(p);
-#endif
-    xmlMutexUnlock(xmlMemMutex);
+    xmlMemFillNode(p, REALLOC_TYPE, number, size, file, line);
 
     TEST_POINT
 
@@ -429,23 +441,13 @@ xmlMemFree(void *ptr)
     target = (char *) ptr;
 
     p = CLIENT_2_HDR(ptr);
-    if (p->mh_tag != MEMTAG) {
-        Mem_Tag_Err(p);
-        goto error;
-    }
+    if (!xmlMemNodeCheck(p)) goto error;
     if (xmlMemStopAtBlock == p->mh_number) xmlMallocBreakpoint();
-    p->mh_tag = ~MEMTAG;
     memset(target, -1, p->mh_size);
-    xmlMutexLock(xmlMemMutex);
-    debugMemSize -= p->mh_size;
-    debugMemBlocks--;
 #ifdef DEBUG_MEMORY
-    size = p->mh_size;
-#endif
-#ifdef MEM_LIST
-    debugmem_list_delete(p);
+    size =
 #endif
-    xmlMutexUnlock(xmlMemMutex);
+        xmlMemSetNodeDealloc(p);
 
     free(p);
 
@@ -481,33 +483,11 @@ xmlMemStrdupLoc(const char *str, const char *file, int line)
 {
     char *s;
     size_t size = strlen(str) + 1;
-    MEMHDR *p;
 
     if (!xmlMemInitialized) xmlInitMemory();
     TEST_POINT
 
-    p = (MEMHDR *) malloc(RESERVE_SIZE+size);
-    if (!p) {
-      goto error;
-    }
-    p->mh_tag = MEMTAG;
-    p->mh_size = size;
-    p->mh_type = STRDUP_TYPE;
-    p->mh_file = file;
-    p->mh_line = line;
-    xmlMutexLock(xmlMemMutex);
-    p->mh_number = ++block;
-    debugMemSize += size;
-    debugMemBlocks++;
-    if (debugMemSize > debugMaxMemSize) debugMaxMemSize = debugMemSize;
-#ifdef MEM_LIST
-    debugmem_list_add(p);
-#endif
-    xmlMutexUnlock(xmlMemMutex);
-
-    s = (char *) HDR_2_CLIENT(p);
-
-    if (xmlMemStopAtBlock == p->mh_number) xmlMallocBreakpoint();
+    s = xmlMallocLocType(size, file, line, STRDUP_TYPE);
 
     if (s != NULL)
       strcpy(s,str);
@@ -811,15 +791,15 @@ static void debugmem_list_delete(MEMHDR *p)
 #endif
 
 /*
- * debugmem_tag_error:
+ * debugmem_error:
  *
  * internal error function.
  */
 
-static void debugmem_tag_error(void *p)
+static void debugmem_error(const char *error, void *p)
 {
      xmlGenericError(xmlGenericErrorContext,
-	     "Memory tag error occurs :%p \n\t bye\n", p);
+         "Memory %s error occurs :%p \n\t bye\n", error, p);
 #ifdef MEM_LIST
      if (stderr)
      xmlMemDisplay(stderr);

diff --git a/xmlmemory.c b/xmlmemory.c
index 1874ba3..221f5d8 100644
--- a/xmlmemory.c
+++ b/xmlmemory.c
@@ -79,6 +79,7 @@ void xmlMallocBreakpoint(void);
  */
 
 #define MEMTAG 0x5aa5
+#define MEM_BYTETAG 0x49
 
 #define MALLOC_TYPE 1
 #define REALLOC_TYPE 2
@@ -99,19 +100,27 @@ typedef struct memnod {
    unsigned int   mh_line;
 }  MEMHDR;
 
+typedef struct {
+    unsigned int mh_tag;
+    unsigned int mh_type;
+    size_t       mh_size;
+} MEMFTR;
 
 #ifdef SUN4
 #define ALIGN_SIZE  16
 #else
 #define ALIGN_SIZE  sizeof(double)
 #endif
-#define HDR_SIZE    sizeof(MEMHDR)
-#define RESERVE_SIZE (((HDR_SIZE + (ALIGN_SIZE-1)) \
-		      / ALIGN_SIZE ) * ALIGN_SIZE)
 
+#define ALIGNED(size) (((size + (ALIGN_SIZE-1)) / ALIGN_SIZE ) * ALIGN_SIZE)
+#define HDR_SIZE ALIGNED(sizeof(MEMHDR))
+#define FTR_SIZE ALIGNED(sizeof(MEMFTR))
+#define RESERVE_SIZE (HDR_SIZE + FTR_SIZE)
 
-#define CLIENT_2_HDR(a) ((MEMHDR *) (((char *) (a)) - RESERVE_SIZE))
-#define HDR_2_CLIENT(a)    ((void *) (((char *) (a)) + RESERVE_SIZE))
+#define CLIENT_2_HDR(a) ((MEMHDR *) (((char *) (a)) - HDR_SIZE))
+#define HDR_2_CLIENT(a)   ((void *) (((char *) (a)) + HDR_SIZE))
+#define HDR_2_FOOTER(a) \
+    ((MEMFTR*) (((char *) (a)) + HDR_SIZE + ALIGNED(a->mh_size)))
 
 
 static unsigned int block=0;
@@ -154,6 +163,9 @@ static void
 xmlMemFillNode(MEMHDR *p, int type, int number, int size,
     const char *file, int line)
 {
+    char *target;
+    MEMFTR *footer;
+
     p->mh_tag = MEMTAG;
     p->mh_type = type;
     p->mh_number = number;
@@ -169,6 +181,15 @@ xmlMemFillNode(MEMHDR *p, int type, int number, int size,
     debugmem_list_add(p);
 #endif
     xmlMutexUnlock(xmlMemMutex);
+
+    target = HDR_2_CLIENT(p);
+    memset(target + size, MEM_BYTETAG, ALIGNED(size) - size);
+
+    footer = HDR_2_FOOTER(p);
+
+    footer->mh_tag = MEMTAG;
+    footer->mh_size = size;
+    footer->mh_type = type;
 }
 
 /**
@@ -181,7 +202,11 @@ xmlMemFillNode(MEMHDR *p, int type, int number, int size,
 static int
 xmlMemNodeCheck(MEMHDR *p)
 {
-#define MEM_ERROR(error) debugmem_error(error, p); return 0
+#   define MEM_ERROR(error) debugmem_error(error, p); return 0
+    char *target;
+    MEMFTR *ftr;
+    unsigned i;
+
     if (p->mh_tag != MEMTAG) { MEM_ERROR("tag"); }
 
     switch (p->mh_type) {
@@ -195,6 +220,18 @@ xmlMemNodeCheck(MEMHDR *p)
     }
 
     if (p->mh_number > block) { MEM_ERROR("block number"); }
+
+    target = HDR_2_CLIENT(p);
+    for (i = 0; i < ALIGNED(p->mh_size) - p->mh_size; i++) {
+        if (target[p->mh_size + i] != MEM_BYTETAG) {
+            MEM_ERROR("padding byte tag");
+        }
+    }
+    ftr = HDR_2_FOOTER(p);
+    if (ftr->mh_tag != MEMTAG) { MEM_ERROR("footer tag"); }
+    if (ftr->mh_type != p->mh_type) { MEM_ERROR("footer type"); }
+    if (ftr->mh_size != p->mh_size) { MEM_ERROR("footer size"); }
+
 #undef MEM_ERROR
 
     return 1;
@@ -225,7 +262,7 @@ xmlMallocLocType(size_t size, const char * file, int line, int type)
 
     TEST_POINT
 
-    p = (MEMHDR *) malloc(RESERVE_SIZE+size);
+    p = (MEMHDR *) malloc(RESERVE_SIZE+ALIGNED(size));
 
     if (!p) {
 	xmlGenericError(xmlGenericErrorContext,
@@ -367,7 +404,7 @@ xmlReallocLoc(void *ptr,size_t size, const char * file, int line)
 #endif
         xmlMemSetNodeDealloc(p);
 
-    p = (MEMHDR *) realloc(p,RESERVE_SIZE+size);
+    p = (MEMHDR *) realloc(p,RESERVE_SIZE+ALIGNED(size));
     if (!p) {
 	 goto error;
     }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]