[xml] Possible 64-bit issues in xml 2.9.2

[Bruce Dawson <brucedawson google com>]

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.

--

Bruce Dawson

Attachment: libxmlwarnings.txt
Description: Text document