Re: [xml] Possible 64-bit issues in xml 2.9.2

[Daniel Veillard <veillard redhat com>]

On Tue, Nov 10, 2015 at 05:34:45PM -0800, Bruce Dawson wrote:
> Sure. I'll do that.

  okay, thanks

> On old architectures it would be easy enough to use a typedef to 'create'
> intptr_t, so any compatibility problems that such a patch introduces will
> be easily fixed.

  the problem is to know when that type is not defined, will require some
configure.ac magic I'm afraid

Daniel

> On Tue, Nov 10, 2015 at 5:32 PM, Daniel Veillard <veillard redhat com>
> wrote:
>
> > On Tue, Nov 10, 2015 at 01:04:51PM -0800, Bruce Dawson wrote:
> > > Resending now that I've joined the mailing list...
> > >
> > > While building 64-bit Chromium with VC++ 2015 Update 1 I noticed a
> > > significant number of pointer truncation warnings in libxml, especially
> > in
> > > xpath.c. A typical warning is:
> > >
> > > warning C4311: 'type cast': pointer truncation from 'xmlChar *' to 'long'
> > >
> > > which triggers on the last two lines of this block:
> > >
> > > case XML_ELEMENT_NODE:
> > >     if (node2->type == XML_ELEMENT_NODE) {
> > > if ((0 > (long) node1->content) && /* TODO: Would a != 0 suffice here? */
> > >     (0 > (long) node2->content) &&
> > >
> > > The intent is not entirely clear but if these are supposed to be NULL
> > > checks then they could easily give the wrong result. In the VC++ world
> > > 'long' is always a 32-bit type, so converting a pointer to a long both
> > > throws away the top 32 bits and also makes it a signed type. That means
> > > that pointers to the first 2 GB of address space will behave as expected,
> > > and pointers beyond that will have a 50% chance of behaving incorrectly!
> > >
> > > Another example is these two conversions. The code is apparently trying
> > to
> > > sort by content address, but on 64-bit Windows builds it will just be
> > > sorting by the bottom 32 bits of the content address.
> > > l1 = -((long) node1->content);
> > > l2 = -((long) node2->content);
> > > if (l1 < l2)
> > >    return(1);
> > > if (l1 > l2)
> > >    return(-1);
> > >     }
> > >
> > > This is not a problem with gcc or clang because their 64-bit builds have
> > > 64-bit long, but the C/C++ standard do not mandate and that is not the
> > case
> > > with VC++.
> > >
> > > I think that the correct type would be ptrdiff_t, or size_t, or intptr_t,
> > > or uintptr_t. I've attached the full set of warnings in case that is of
> > any
> > > assistance. Even though some of these warnings do not indicate actual
> > bugs
> > > it would still be best to use the correct type in order to avoid
> > confusion
> > > and warnings.
> >
> >   Sounds like intptr_t would be the proper type to use for those, I just
> > wonder what kind of old arches will break if they don't have it.
> >
> > Can you come up with a patch which just change the casts and possibly the
> > target
> > types ?
> >
> > Daniel
> >
> > --
> > Daniel Veillard      | Open Source and Standards, Red Hat
> > veillard redhat com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
> > http://veillard.com/ | virtualization library  http://libvirt.org/
>
>
>
> -- 
> Bruce Dawson

-- 
Daniel Veillard      | Open Source and Standards, Red Hat
veillard redhat com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | virtualization library  http://libvirt.org/