Re: msxml3: Fix varargs handling in libxml2 error callback implementation
On Thu, Feb 16, 2012 at 03:15:59AM +0300, Nikolay Sivov wrote: > >If I remember correctly, you can even process a va_list only once > >on some platforms. > We use it that way in couple of places, so it seems to work and I can't > find a proper description or part of a standard that says it's not portable. It definitely isn't portable. The fact that it works sometimes is irrelevant. > See winegcc/wrc for > --- > char* strmake(const char* fmt, ...) > --- > as an example. > > That probably means vsnprintf() and similar calls were added as part > of C99 as well, so their presence implies working va_copy() is available. vsnprintf() was added to a lot of system much earlier than va_copy(). Last time I looked I couldn't find a va_copy() for the Microsoft C compiler we were using at the time. In my case it wasn't a printf, but a function that took a list of pointers followed by a NULL - so I copied them into an on-stack array. David -- David Laight: da...@l8s.co.uk
Re: msxml3: Fix varargs handling in libxml2 error callback implementation
Nikolay Sivov writes: > 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. Actually there's no need to call vsnprintf multiple times. The debug buffer has a limited size anyway, so we might as well use a limited stack buffer here. -- Alexandre Julliard julli...@winehq.org
Re: msxml3: Fix varargs handling in libxml2 error callback implementation
On 2/16/2012 01:28, Marcus Meissner wrote: 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. We use it that way in couple of places, so it seems to work and I can't find a proper description or part of a standard that says it's not portable. See winegcc/wrc for --- char* strmake(const char* fmt, ...) --- as an example. That probably means vsnprintf() and similar calls were added as part of C99 as well, so their presence implies working va_copy() is available. Anyway calling it many times with same va_list is broken. If you need to process it multiple times, you need to create a copy with va_copy() first. Yes, but that's a part of C99. Ciao, Marcus
Re: msxml3: Fix varargs handling in libxml2 error callback implementation
On Wed, Feb 15, 2012 at 11:28:37PM +0100, Marcus Meissner wrote: > 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 Correct - on architectures that don't pass all arguments on the stack a va_list is a complex data item that can only be processed once. The Microsoft ABI for amd64 reserves stack space for the arguments passed in registers so that the processing of integer/ptr args is easy. For all Unix OS amd64 passed the first 6 (IIRC) integer/ptr args in normal registers, and the first few FP args in FP regs (regardless of the order of the parameters), the va_list data has to remember which register args have been processed. This is all somewhat tricky! and makes support for printf's argument order selection stuff extremely difficult to write! David -- David Laight: da...@l8s.co.uk
Re: msxml3: Fix varargs handling in libxml2 error callback implementation
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
> 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/msxml
