On Thu, Feb 16, 2012 at 01:55:44AM +0300, Nikolay Sivov wrote: > The problem is that vsnprintf() was called multiple times with same > va_list. Ti fix that it was necessary to get rid of some tracing > bits like macro-defined callback calls and a single function for all > kinds of error types. > > As far as I understand this problem it leads to a stack corruption > when va_list is used multiple time without va_start/va_end around > it, so it's critical to fix.
If I remember correctly, you can even process a va_list only once on some platforms. If you need to process it multiple times, you need to create a copy with va_copy() first. Ciao, Marcus > From d75433c8ff34e125c7db1fb665bdb9271991b9f5 Mon Sep 17 00:00:00 2001 > From: Nikolay Sivov <nsi...@codeweavers.com> > Date: Thu, 16 Feb 2012 01:06:16 +0300 > Subject: [PATCH 1/1] Fix varargs handling in libxml2 error callback > implementation > > --- > dlls/msxml3/domdoc.c | 50 +++++------------------- > dlls/msxml3/main.c | 49 +++++++++++++++++------- > dlls/msxml3/msxml_private.h | 18 ++------- > dlls/msxml3/schema.c | 90 > ++++++++++++++----------------------------- > dlls/msxml3/selection.c | 4 +- > 5 files changed, 80 insertions(+), 131 deletions(-) > > diff --git a/dlls/msxml3/domdoc.c b/dlls/msxml3/domdoc.c > index 64122ce..9bcecdd 100644 > --- a/dlls/msxml3/domdoc.c > +++ b/dlls/msxml3/domdoc.c > @@ -425,23 +425,7 @@ static void sax_characters(void *ctx, const xmlChar *ch, > int len) > xmlSAX2Characters(ctxt, ch, len); > } > > -static void LIBXML2_LOG_CALLBACK sax_error(void* ctx, char const* msg, ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_ERR(doparse, msg, ap); > - va_end(ap); > -} > - > -static void LIBXML2_LOG_CALLBACK sax_warning(void* ctx, char const* msg, ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_WARN(doparse, msg, ap); > - va_end(ap); > -} > - > -static void sax_serror(void* ctx, xmlErrorPtr err) > +static void libxml2_sax_structerror(void* ctx, xmlErrorPtr err) > { > LIBXML2_CALLBACK_SERROR(doparse, err); > } > @@ -472,9 +456,9 @@ static xmlDocPtr doparse(domdoc* This, char const* ptr, > int len, xmlCharEncoding > sax_characters, /* ignorableWhitespace */ > xmlSAX2ProcessingInstruction, /* processingInstruction */ > xmlSAX2Comment, /* comment */ > - sax_warning, /* warning */ > - sax_error, /* error */ > - sax_error, /* fatalError */ > + libxml2_warn_callback, /* warning */ > + libxml2_error_callback, /* error */ > + libxml2_error_callback, /* fatalError */ > xmlSAX2GetParameterEntity, /* getParameterEntity */ > xmlSAX2CDataBlock, /* cdataBlock */ > xmlSAX2ExternalSubset, /* externalSubset */ > @@ -482,10 +466,12 @@ static xmlDocPtr doparse(domdoc* This, char const* ptr, > int len, xmlCharEncoding > NULL, /* _private */ > xmlSAX2StartElementNs, /* startElementNs */ > xmlSAX2EndElementNs, /* endElementNs */ > - sax_serror /* serror */ > + libxml2_sax_structerror /* serror */ > }; > xmlInitParser(); > > + TRACE("(%p)->(%p %d %d)\n", This, ptr, len, encoding); > + > pctx = xmlCreateMemoryParserCtxt(ptr, len); > if (!pctx) > { > @@ -2634,22 +2620,6 @@ static inline BOOL is_wellformed(xmlDocPtr doc) > #endif > } > > -static void LIBXML2_LOG_CALLBACK validate_error(void* ctx, char const* msg, > ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_ERR(domdoc_validateNode, msg, ap); > - va_end(ap); > -} > - > -static void LIBXML2_LOG_CALLBACK validate_warning(void* ctx, char const* > msg, ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_WARN(domdoc_validateNode, msg, ap); > - va_end(ap); > -} > - > static HRESULT WINAPI domdoc_validateNode( > IXMLDOMDocument3* iface, > IXMLDOMNode* node, > @@ -2695,11 +2665,11 @@ static HRESULT WINAPI domdoc_validateNode( > if (get_doc(This)->intSubset || get_doc(This)->extSubset) > { > xmlValidCtxtPtr vctx = xmlNewValidCtxt(); > - vctx->error = validate_error; > - vctx->warning = validate_warning; > + vctx->error = libxml2_error_callback; > + vctx->warning = libxml2_warn_callback; > ++validated; > > - if (!((node == (IXMLDOMNode*)iface)? > + if (!((node == (IXMLDOMNode*)iface) ? > xmlValidateDocument(vctx, get_doc(This)) : > xmlValidateElement(vctx, get_doc(This), > get_node_obj(node)->node))) > { > diff --git a/dlls/msxml3/main.c b/dlls/msxml3/main.c > index 464ef90..4f9b020 100644 > --- a/dlls/msxml3/main.c > +++ b/dlls/msxml3/main.c > @@ -60,28 +60,47 @@ HINSTANCE MSXML_hInstance = NULL; > > #ifdef HAVE_LIBXML2 > > -void wineXmlCallbackLog(char const* caller, xmlErrorLevel lvl, char const* > msg, va_list ap) > +#define LIBXML2_LOG_CALLBACK __WINE_PRINTF_ATTR(2,3) > + > +void LIBXML2_LOG_CALLBACK libxml2_error_callback(void *ctxt, const char > *msg, ...) > { > - char* buf = NULL; > int len = 32, needed; > - enum __wine_debug_class dbcl = __WINE_DBCL_ERR; > - switch (lvl) > + char* buf = NULL; > + va_list ap; > + > + do > { > - case XML_ERR_NONE: > - dbcl = __WINE_DBCL_TRACE; > - break; > - case XML_ERR_WARNING: > - dbcl = __WINE_DBCL_WARN; > - break; > - default: > - break; > + heap_free(buf); > + buf = heap_alloc(len); > + va_start(ap, msg); > + needed = vsnprintf(buf, len, msg, ap); > + va_end(ap); > + if (needed == -1) > + len *= 2; > + else if (needed >= len) > + len = needed + 1; > + else > + needed = 0; > } > + while (needed); > + > + wine_dbg_log(__WINE_DBCL_ERR, &__wine_dbch_msxml, "libxml2", "%s", buf); > + heap_free(buf); > +} > + > +void LIBXML2_LOG_CALLBACK libxml2_warn_callback(void *ctxt, const char *msg, > ...) > +{ > + int len = 32, needed; > + char* buf = NULL; > + va_list ap; > > do > { > heap_free(buf); > buf = heap_alloc(len); > + va_start(ap, msg); > needed = vsnprintf(buf, len, msg, ap); > + va_end(ap); > if (needed == -1) > len *= 2; > else if (needed >= len) > @@ -91,11 +110,13 @@ void wineXmlCallbackLog(char const* caller, > xmlErrorLevel lvl, char const* msg, > } > while (needed); > > - wine_dbg_log(dbcl, &__wine_dbch_msxml, caller, "%s", buf); > + wine_dbg_log(__WINE_DBCL_WARN, &__wine_dbch_msxml, "libxml2", "%s", buf); > heap_free(buf); > } > > -void wineXmlCallbackError(char const* caller, xmlErrorPtr err) > +#undef LIBXML2_LOG_CALLBACK > + > +void libxml2_structured_error(const char* caller, xmlErrorPtr err) > { > enum __wine_debug_class dbcl; > > diff --git a/dlls/msxml3/msxml_private.h b/dlls/msxml3/msxml_private.h > index e51501b..66c3b89 100644 > --- a/dlls/msxml3/msxml_private.h > +++ b/dlls/msxml3/msxml_private.h > @@ -289,21 +289,11 @@ extern xmlNodePtr xmldoc_unlink_xmldecl(xmlDocPtr doc) > DECLSPEC_HIDDEN; > > extern HRESULT XMLElement_create( IUnknown *pUnkOuter, xmlNodePtr node, > LPVOID *ppObj, BOOL own ) DECLSPEC_HIDDEN; > > -extern void wineXmlCallbackLog(char const* caller, xmlErrorLevel lvl, char > const* msg, va_list ap) DECLSPEC_HIDDEN; > -extern void wineXmlCallbackError(char const* caller, xmlErrorPtr err) > DECLSPEC_HIDDEN; > +extern void libxml2_structured_error(const char* caller, xmlErrorPtr err) > DECLSPEC_HIDDEN; > +extern void libxml2_error_callback(void *ctxt, const char *msg, ...) > DECLSPEC_HIDDEN; > +extern void libxml2_warn_callback(void *ctxt, const char *msg, ...) > DECLSPEC_HIDDEN; > > -#define LIBXML2_LOG_CALLBACK __WINE_PRINTF_ATTR(2,3) > - > -#define LIBXML2_CALLBACK_TRACE(caller, msg, ap) \ > - wineXmlCallbackLog(#caller, XML_ERR_NONE, msg, ap) > - > -#define LIBXML2_CALLBACK_WARN(caller, msg, ap) \ > - wineXmlCallbackLog(#caller, XML_ERR_WARNING, msg, ap) > - > -#define LIBXML2_CALLBACK_ERR(caller, msg, ap) \ > - wineXmlCallbackLog(#caller, XML_ERR_ERROR, msg, ap) > - > -#define LIBXML2_CALLBACK_SERROR(caller, err) wineXmlCallbackError(#caller, > err) > +#define LIBXML2_CALLBACK_SERROR(caller, err) > libxml2_structured_error(#caller, err) > > extern BOOL is_preserving_whitespace(xmlNodePtr node) DECLSPEC_HIDDEN; > extern BOOL is_xpathmode(const xmlDocPtr doc) DECLSPEC_HIDDEN; > diff --git a/dlls/msxml3/schema.c b/dlls/msxml3/schema.c > index 253f901..c06bb69 100644 > --- a/dlls/msxml3/schema.c > +++ b/dlls/msxml3/schema.c > @@ -230,85 +230,53 @@ static const BYTE hash_assoc_values[] = > 116, 116, 116, 116, 116, 116 > }; > > -static void LIBXML2_LOG_CALLBACK parser_error(void* ctx, char const* msg, > ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_ERR(Schema_parse, msg, ap); > - va_end(ap); > -} > - > -static void LIBXML2_LOG_CALLBACK parser_warning(void* ctx, char const* msg, > ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_WARN(Schema_parse, msg, ap); > - va_end(ap); > -} > - > #ifdef HAVE_XMLSCHEMASSETPARSERSTRUCTUREDERRORS > -static void parser_serror(void* ctx, xmlErrorPtr err) > +static void libxml2_schema_structerror(void* ctx, xmlErrorPtr err) > { > - LIBXML2_CALLBACK_SERROR(Schema_parse, err); > + LIBXML2_CALLBACK_SERROR(schema_parse, err); > } > #endif > > -static inline xmlSchemaPtr Schema_parse(xmlSchemaParserCtxtPtr spctx) > +static inline xmlSchemaPtr schema_parse(xmlSchemaParserCtxtPtr ctxt) > { > - TRACE("(%p)\n", spctx); > + TRACE("(%p)\n", ctxt); > > - xmlSchemaSetParserErrors(spctx, parser_error, parser_warning, NULL); > + xmlSchemaSetParserErrors(ctxt, libxml2_error_callback, > libxml2_warn_callback, NULL); > #ifdef HAVE_XMLSCHEMASSETPARSERSTRUCTUREDERRORS > - xmlSchemaSetParserStructuredErrors(spctx, parser_serror, NULL); > + xmlSchemaSetParserStructuredErrors(ctxt, libxml2_schema_structerror, > NULL); > #endif > > - return xmlSchemaParse(spctx); > -} > - > -static void LIBXML2_LOG_CALLBACK validate_error(void* ctx, char const* msg, > ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_ERR(Schema_validate_tree, msg, ap); > - va_end(ap); > -} > - > -static void LIBXML2_LOG_CALLBACK validate_warning(void* ctx, char const* > msg, ...) > -{ > - va_list ap; > - va_start(ap, msg); > - LIBXML2_CALLBACK_WARN(Schema_validate_tree, msg, ap); > - va_end(ap); > + return xmlSchemaParse(ctxt); > } > > #ifdef HAVE_XMLSCHEMASSETVALIDSTRUCTUREDERRORS > -static void validate_serror(void* ctx, xmlErrorPtr err) > +static void libxml2_validate_structerror(void* ctx, xmlErrorPtr err) > { > - LIBXML2_CALLBACK_SERROR(Schema_validate_tree, err); > + LIBXML2_CALLBACK_SERROR(schema_validate_tree, err); > } > #endif > > -static inline HRESULT Schema_validate_tree(xmlSchemaPtr schema, xmlNodePtr > tree) > +static inline HRESULT schema_validate_tree(xmlSchemaPtr schema, xmlNodePtr > tree) > { > - xmlSchemaValidCtxtPtr svctx; > + xmlSchemaValidCtxtPtr ctxt; > int err; > > TRACE("(%p, %p)\n", schema, tree); > /* TODO: if validateOnLoad property is false, > * we probably need to validate the schema here. */ > - svctx = xmlSchemaNewValidCtxt(schema); > - xmlSchemaSetValidErrors(svctx, validate_error, validate_warning, NULL); > + ctxt = xmlSchemaNewValidCtxt(schema); > + xmlSchemaSetValidErrors(ctxt, libxml2_error_callback, > libxml2_warn_callback, NULL); > #ifdef HAVE_XMLSCHEMASSETVALIDSTRUCTUREDERRORS > - xmlSchemaSetValidStructuredErrors(svctx, validate_serror, NULL); > + xmlSchemaSetValidStructuredErrors(ctxt, libxml2_validate_structerror, > NULL); > #endif > > if (tree->type == XML_DOCUMENT_NODE) > - err = xmlSchemaValidateDoc(svctx, (xmlDocPtr)tree); > + err = xmlSchemaValidateDoc(ctxt, (xmlDocPtr)tree); > else > - err = xmlSchemaValidateOneElement(svctx, tree); > + err = xmlSchemaValidateOneElement(ctxt, tree); > > - xmlSchemaFreeValidCtxt(svctx); > - return err? S_FALSE : S_OK; > + xmlSchemaFreeValidCtxt(ctxt); > + return err ? S_FALSE : S_OK; > } > > static DWORD dt_hash(xmlChar const* str, int len /* calculated if -1 */) > @@ -599,11 +567,11 @@ HRESULT dt_validate(XDR_DT dt, xmlChar const* content) > > if (!datatypes_schema) > { > - xmlSchemaParserCtxtPtr spctx; > + xmlSchemaParserCtxtPtr ctxt; > assert(datatypes_src != NULL); > - spctx = xmlSchemaNewMemParserCtxt((char const*)datatypes_src, > datatypes_len); > - datatypes_schema = Schema_parse(spctx); > - xmlSchemaFreeParserCtxt(spctx); > + ctxt = xmlSchemaNewMemParserCtxt((char const*)datatypes_src, > datatypes_len); > + datatypes_schema = schema_parse(ctxt); > + xmlSchemaFreeParserCtxt(ctxt); > } > > switch (dt) > @@ -656,7 +624,7 @@ HRESULT dt_validate(XDR_DT dt, xmlChar const* content) > xmlSetNs(node, ns); > xmlDocSetRootElement(tmp_doc, node); > > - hr = Schema_validate_tree(datatypes_schema, > (xmlNodePtr)tmp_doc); > + hr = schema_validate_tree(datatypes_schema, > (xmlNodePtr)tmp_doc); > xmlFreeDoc(tmp_doc); > } > else > @@ -844,7 +812,7 @@ static BOOL link_datatypes(xmlDocPtr schema) > static cache_entry* cache_entry_from_xsd_doc(xmlDocPtr doc, xmlChar const* > nsURI, MSXML_VERSION v) > { > cache_entry* entry = heap_alloc(sizeof(cache_entry)); > - xmlSchemaParserCtxtPtr spctx; > + xmlSchemaParserCtxtPtr ctxt; > xmlDocPtr new_doc = xmlCopyDoc(doc, 1); > > link_datatypes(new_doc); > @@ -853,9 +821,9 @@ static cache_entry* cache_entry_from_xsd_doc(xmlDocPtr > doc, xmlChar const* nsURI > * do we need to do something special here? */ > entry->type = CacheEntryType_XSD; > entry->ref = 0; > - spctx = xmlSchemaNewDocParserCtxt(new_doc); > + ctxt = xmlSchemaNewDocParserCtxt(new_doc); > > - if ((entry->schema = Schema_parse(spctx))) > + if ((entry->schema = schema_parse(ctxt))) > { > xmldoc_init(entry->schema->doc, v); > entry->doc = entry->schema->doc; > @@ -868,7 +836,7 @@ static cache_entry* cache_entry_from_xsd_doc(xmlDocPtr > doc, xmlChar const* nsURI > heap_free(entry); > entry = NULL; > } > - xmlSchemaFreeParserCtxt(spctx); > + xmlSchemaFreeParserCtxt(ctxt); > return entry; > } > > @@ -884,7 +852,7 @@ static cache_entry* cache_entry_from_xdr_doc(xmlDocPtr > doc, xmlChar const* nsURI > entry->ref = 0; > spctx = xmlSchemaNewDocParserCtxt(xsd_doc); > > - if ((entry->schema = Schema_parse(spctx))) > + if ((entry->schema = schema_parse(spctx))) > { > entry->doc = new_doc; > xmldoc_init(entry->schema->doc, version); > @@ -1443,7 +1411,7 @@ HRESULT > SchemaCache_validate_tree(IXMLDOMSchemaCollection2* iface, xmlNodePtr tr > /* TODO: if the ns is not in the cache, and it's a URL, > * do we try to load from that? */ > if (schema) > - return Schema_validate_tree(schema, tree); > + return schema_validate_tree(schema, tree); > else > WARN("no schema found for xmlns=%s\n", get_node_nsURI(tree)); > > diff --git a/dlls/msxml3/selection.c b/dlls/msxml3/selection.c > index ba173f9..61e7f24 100644 > --- a/dlls/msxml3/selection.c > +++ b/dlls/msxml3/selection.c > @@ -738,7 +738,7 @@ static void XSLPattern_OP_IGEq(xmlXPathParserContextPtr > pctx, int nargs) > xmlFree(arg2); > } > > -static void query_serror(void* ctx, xmlErrorPtr err) > +static void libxml2_selection_structerror(void* ctx, xmlErrorPtr err) > { > LIBXML2_CALLBACK_SERROR(domselection_create, err); > } > @@ -767,7 +767,7 @@ HRESULT create_selection(xmlNodePtr node, xmlChar* query, > IXMLDOMNodeList **out) > init_dispex(&This->dispex, (IUnknown*)&This->IXMLDOMSelection_iface, > &domselection_dispex); > xmldoc_add_ref(This->node->doc); > > - ctxt->error = query_serror; > + ctxt->error = libxml2_selection_structerror; > ctxt->node = node; > registerNamespaces(ctxt); > > -- > 1.5.6.5 > >