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

Noam Postavsky <[email protected]>
Newsgroups gmane.comp.gnome.lib.xslt
Message-ID <[email protected]>
Daniel Veillard <[email protected]> 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

_______________________________________________
xslt mailing list, project page http://xmlsoft.org/XSLT/
[email protected]
http://mail.gnome.org/mailman/listinfo/xslt
01xmlmem-refactor.patch (application/octet-stream, 9.6 KB)
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);
02xmlmem-addfooter.patch (application/octet-stream, 3.3 KB)
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;
     }
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.