nextDeleted should be common to all node structures #14
Description
If tDOM is threaded, when a node is deleted from a shared document, it is added to the deletedNode list of that document and its nextDeleted member is set to NULL (commit 8667ce5, generic/dom.c, ll. 2582 ff.):
TDomThreaded (
if (shared) {
if (doc->deletedNodes) {
doc->deletedNodes->nextDeleted = node;
} else {
doc->deletedNodes = node;
}
node->nodeFlags |= IS_DELETED;
node->nextDeleted = NULL;
}
)
However, only the domNode type contains a nextDeleted member and, furthermore, it appears towards the end of the structure; thus in the case of nodes of other types, assigning NULL to nextDeleted will result in an invalid write and may corrupt the node itself or any structure which follows it in memory:
Here's an example:
$ cat tdomtest.tcl
#!/bin/sh
#\
exec tclsh8.5 "$0" "$@"
package require tdom
set xml {
<root>
<child>
<attr1>one</attr1>
<attr2>two</attr2>
<attr3>three</attr3>
</child>
</root>
}
dom parse $xml doc
dom attachDocument $doc doc_
$doc documentElement root
foreach node [$root selectNodes {//attr1 | //attr2}] {
[$node firstChild] delete
}
$ valgrind --leak-check=full --log-file=vg.log tclsh8.5 tdomtest.tcl
$ head -60 vg.log
==27872== Memcheck, a memory error detector
==27872== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==27872== Using Valgrind-3.6.0 and LibVEX; rerun with -h for copyright info
==27872== Command: tclsh8.5 tdomtest.tcl
==27872== Parent PID: 14431
==27872==
==27872== Invalid write of size 8
==27872== at 0xC24C89F: domDeleteNode (dom.c:2582)
==27872== by 0xC295F32: tcldom_NodeObjCmd (tcldom.c:4474)
==27872== by 0x4E63A62: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4EAD0FC: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4EB47A8: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E650F5: TclEvalObjEx (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E6DA49: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E63A62: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E6452E: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4ED0E60: Tcl_FSEvalFileEx (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4ED6E22: Tcl_Main (in /usr/lib64/libtcl8.5.so)
==27872== by 0x40083F: main (in /usr/bin/tclsh8.5)
==27872== Address 0xc03d500 is 8 bytes after a block of size 56 alloc'd
==27872== at 0x4C26FDE: malloc (vg_replace_malloc.c:236)
==27872== by 0xC249E59: DispatchPCDATA (dom.c:1474)
==27872== by 0xC249B9E: endElement (dom.c:1370)
==27872== by 0xC238C08: doContent (xmlparse.c:2449)
==27872== by 0xC2376E6: contentProcessor (xmlparse.c:2022)
==27872== by 0xC23CC49: doProlog (xmlparse.c:3905)
==27872== by 0xC23C01D: prologProcessor (xmlparse.c:3635)
==27872== by 0xC23BA8D: prologInitProcessor (xmlparse.c:3452)
==27872== by 0xC23678B: XML_Parse (xmlparse.c:1483)
==27872== by 0xC24B7ED: domReadDocument (dom.c:2069)
==27872== by 0xC29A6D0: tcldom_parse (tcldom.c:5585)
==27872== by 0xC29AF59: tcldom_DomObjCmd (tcldom.c:5749)
==27872==
==27872== Invalid write of size 8
==27872== at 0xC24C875: domDeleteNode (dom.c:2582)
==27872== by 0xC295F32: tcldom_NodeObjCmd (tcldom.c:4474)
==27872== by 0x4E63A62: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4EAD0FC: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4EB47A8: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E650F5: TclEvalObjEx (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E6DA49: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E63A62: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4E6452E: ??? (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4ED0E60: Tcl_FSEvalFileEx (in /usr/lib64/libtcl8.5.so)
==27872== by 0x4ED6E22: Tcl_Main (in /usr/lib64/libtcl8.5.so)
==27872== by 0x40083F: main (in /usr/bin/tclsh8.5)
==27872== Address 0xc03d500 is 8 bytes after a block of size 56 alloc'd
==27872== at 0x4C26FDE: malloc (vg_replace_malloc.c:236)
==27872== by 0xC249E59: DispatchPCDATA (dom.c:1474)
==27872== by 0xC249B9E: endElement (dom.c:1370)
==27872== by 0xC238C08: doContent (xmlparse.c:2449)
==27872== by 0xC2376E6: contentProcessor (xmlparse.c:2022)
==27872== by 0xC23CC49: doProlog (xmlparse.c:3905)
==27872== by 0xC23C01D: prologProcessor (xmlparse.c:3635)
==27872== by 0xC23BA8D: prologInitProcessor (xmlparse.c:3452)
==27872== by 0xC23678B: XML_Parse (xmlparse.c:1483)
==27872== by 0xC24B7ED: domReadDocument (dom.c:2069)
==27872== by 0xC29A6D0: tcldom_parse (tcldom.c:5585)
==27872== by 0xC29AF59: tcldom_DomObjCmd (tcldom.c:5749)
==27872==
It appears to me that the fix is to move nextDeleted to the end of the common members, and add it to the structures from which it is missing:
diff --git a/generic/dom.h b/generic/dom.h
index ed17efd..ac2ac62 100644
--- a/generic/dom.h
+++ b/generic/dom.h
@@ -598,13 +598,13 @@ typedef struct domNode {
struct domNode *parentNode;
struct domNode *previousSibling;
struct domNode *nextSibling;
+#ifdef TCL_THREADS
+ struct domNode *nextDeleted;
+#endif
domString nodeName; /* now the element node specific fields */
struct domNode *firstChild;
struct domNode *lastChild;
-#ifdef TCL_THREADS
- struct domNode *nextDeleted;
-#endif
struct domAttrNode *firstAttr;
} domNode;
@@ -637,6 +637,9 @@ typedef struct domTextNode {
struct domNode *parentNode;
struct domNode *previousSibling;
struct domNode *nextSibling;
+#ifdef TCL_THREADS
+ struct domNode *nextDeleted;
+#endif
domString nodeValue; /* now the text node specific fields */
int valueLength;
@@ -659,6 +662,9 @@ typedef struct domProcessingInstructionNode {
struct domNode *parentNode;
struct domNode *previousSibling;
struct domNode *nextSibling;
+#ifdef TCL_THREADS
+ struct domNode *nextDeleted;
+#endif
domString targetValue; /* now the pi specific fields */
int targetLength;
@@ -683,6 +689,9 @@ typedef struct domAttrNode {
int valueLength;
struct domNode *parentNode;
struct domAttrNode *nextSibling;
+#ifdef TCL_THREADS
+ struct domNode *nextDeleted;
+#endif
} domAttrNode;