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 <nsivov(a)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
>
>
