MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0006051CMakeCMakepublic2007-11-14 12:222007-11-16 15:40
ReporterSean McBride 
Assigned To 
PrioritynormalSeverityblockReproducibilityalways
StatusclosedResolutionfixed 
PlatformMac OS XOSMac OS XOS Version10.5
Product Version 
Target VersionFixed in Version 
Summary0006051: CMake cannot build itself as 64 bit on a 64 bit Mac OS X 10.5 machine, incorrect va_list copying performed
DescriptionCMake (from today's CVS) it unable to build itself with "CMAKE_OSX_ARCHITECTURES" as 'x86_64', it says:

/Users/sean/kitware/CMake/Utilities/cmxmlrpc/xmlrpc_data.c: In function ‘xmlrpc_build_value_va’:
/Users/sean/kitware/CMake/Utilities/cmxmlrpc/xmlrpc_data.c:938: error: incompatible types in assignment
/Users/sean/kitware/CMake/Utilities/cmxmlrpc/xmlrpc_data.c: In function ‘xmlrpc_parse_value_va’:
/Users/sean/kitware/CMake/Utilities/cmxmlrpc/xmlrpc_data.c:1282: error: incompatible types in assignment
make[2]: *** [Utilities/cmxmlrpc/CMakeFiles/cmXMLRPC.dir/xmlrpc_data.o] Error 1
make[1]: *** [Utilities/cmxmlrpc/CMakeFiles/cmXMLRPC.dir/all] Error 2
make: *** [all] Error 2

This looks like a legitimate error. The 'va_list' type is opaque and the VA_LIST_COPY macro makes invalid assumptions. Currently, it reads:

/* Borrowed from Python 1.5.2.
** Python copies its va_list objects before using them in certain
** tricky fashions. We don't why Python does this, but since we're
** abusing our va_list objects in a similar fashion, we'll copy them
** too. */
#if VA_LIST_IS_ARRAY
#define VA_LIST_COPY(dest,src) memcpy((dest), (src), sizeof(va_list))
#else
#define VA_LIST_COPY(dest,src) ((dest) = (src))
#endif

I changed it to:

#define VA_LIST_COPY(dest,src) va_copy((dest), (src))

This fixes the problem, and seems like the best solution. However, va_copy() was added in C99, and maybe you need to support older compilers too? I suggest reviewing the need for VA_LIST_COPY at all, especially in light of the comment just above it. :)
Steps To Reproduce
Additional Information
TagsNo tags attached.
Relationships
Attached Filestxt CMakeVAListPatch.txt (822) 2007-11-14 15:12
https://public.kitware.com/Bug/file/1229/CMakeVAListPatch.txt
Issue History
Date ModifiedUsernameFieldChange
2007-11-14 12:22Sean McBrideNew Issue
2007-11-14 13:58Bill HoffmanNote Added: 0009673
2007-11-14 15:12Sean McBrideFile Added: CMakeVAListPatch.txt
2007-11-14 15:12Sean McBrideNote Added: 0009675
2007-11-15 07:47Bill HoffmanNote Added: 0009680
2007-11-15 11:47Sean McBrideNote Added: 0009687
2007-11-15 14:21Bill HoffmanNote Added: 0009688
2007-11-15 14:59Sean McBrideNote Added: 0009689
2007-11-16 14:07Bill HoffmanNote Added: 0009695
2007-11-16 14:50Sean McBrideNote Added: 0009696
2007-11-16 15:40Bill HoffmanStatusnew => closed
2007-11-16 15:40Bill HoffmanResolutionopen => fixed
2007-11-16 15:40Bill HoffmanCategoryCMakeSetup => CMake

Notes
(0009673)
Bill Hoffman   
2007-11-14 13:58   
Can you try this: (basically check if va_copy is a define or not for you.)
I am almost certain that this would fail on a variety of compilers if we checked in your change as is.


#if VA_LIST_IS_ARRAY
#define VA_LIST_COPY(dest,src) memcpy((dest), (src), sizeof(va_list))
#else
#ifdef va_copy
#define VA_LIST_COPY(dest,src) va_copy((dest), (src))
#else
#define VA_LIST_COPY(dest,src) ((dest) = (src))
#endif
#endif

I changed it to:

#define VA_LIST_COPY(dest,src) va_copy((dest), (src))
(0009675)
Sean McBride   
2007-11-14 15:12   
After some googling, I found the portable way to detect a C99-compliant compiler. It is:

#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L

But __STDC_VERSION__ may not be defined when compiling C++ so I also check for __APPLE__ explicitly. My new suggestion is:

#if (defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L) || defined(__APPLE__)
  #define VA_LIST_COPY(dest,src) va_copy((dest), (src))
#else
  #if VA_LIST_IS_ARRAY
    #define VA_LIST_COPY(dest,src) memcpy((dest), (src), sizeof(va_list))
  #else
    #define VA_LIST_COPY(dest,src) ((dest) = (src))
  #endif
#endif

Patch attached.
(0009680)
Bill Hoffman   
2007-11-15 07:47   
Do you think you could test if #ifdef va_copy works? I am thinking other c99 platforms might have the same issue, and I don't want a mac only fix. The alternative is to write a try_compile. But if va_copy is already a #define, then I should be OK. From the google searches I saw, it seemed to be.
(0009687)
Sean McBride   
2007-11-15 11:47   
I agree other platforms could have this issue.

I tried the following, and it too works:

#if defined(va_copy)
  #define VA_LIST_COPY(dest,src) va_copy((dest), (src))
#else
  #if VA_LIST_IS_ARRAY
    #define VA_LIST_COPY(dest,src) memcpy((dest), (src), sizeof(va_list))
  #else
    #define VA_LIST_COPY(dest,src) ((dest) = (src))
  #endif
#endif

I'm not sure which solution I prefer (I dislike them all, as this whole thing is a big #if mess by definition). I'm not a language lawyer, but I doubt va_copy is required to be implemented as a macro. As there is already a TRY_COMPILE for VA_LIST_IS_ARRAY maybe first checking for a va_copy() API would be better... I dunno.
(0009688)
Bill Hoffman   
2007-11-15 14:21   
Can you try the following program and let me know if it compiles by hand :

#include <stdio.h>
#include <stdarg.h>
#include <wchar.h>

int main() { va_list list1, list2; va_copy(list1, list2); ; return 0; }

If so, I can do this as a try-compile.
(0009689)
Sean McBride   
2007-11-15 14:59   
sean$ gcc -Wall -Wextra -arch ppc64 foo.c
sean$ gcc -Wall -Wextra -arch ppc foo.c
sean$ g++ -Wall -Wextra -arch x86_64 foo.c

No errors/warnings.
(0009695)
Bill Hoffman   
2007-11-16 14:07   
Can you try a cvs update and see if the try compile works:

$ cvs commit -m "ENH: add a try compile for va_copy"
cvs commit: Examining .
cvs commit: Examining CMake
/cvsroot/CMake/CMake/Utilities/cmxmlrpc/CMakeLists.txt,v <-- CMakeLists.txt
new revision: 1.18; previous revision: 1.17
/cvsroot/CMake/CMake/Utilities/cmxmlrpc/xmlrpc_config.h.in,v <-- xmlrpc_config.h.in
new revision: 1.8; previous revision: 1.7
/cvsroot/CMake/CMake/Utilities/cmxmlrpc/xmlrpc_data.c,v <-- xmlrpc_data.c
new revision: 1.6; previous revision: 1.5
(0009696)
Sean McBride   
2007-11-16 14:50   
I had to blast the bin directory, but it works. Thanks!