Tcl Source Code

View Ticket
Login
Ticket UUID: 1cc44617e2b4ed0a29f75762d45fe46388260f74
Title: Mechanism with 64 bit support in tcl.h does not work outside of core
Type: Bug Version: 8.6/8.7
Submitter: anonymous Created on: 2017-03-23 19:04:49
Subsystem: 69. Other Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Important
Status: Open Last Modified: 2017-03-24 17:33:14
Resolution: Works For Me Closed By: nobody
    Closed on:
Description:

For 64 bit support in tcl.h the following constants will be used: TCL_WIDE_INT_TYPE (on 32 bit arch) or TCL_WIDE_INT_IS_LONG (on 64 bit arch).

With Mac the following (stripped) code seems to work:

#ifdef __APPLE__
#   ifdef __LP64__
#       undef TCL_WIDE_INT_TYPE
#	define TCL_WIDE_INT_IS_LONG 1
#    else /* !__LP64__ */
#       define TCL_WIDE_INT_TYPE long long
#       undef TCL_WIDE_INT_IS_LONG
#    endif /* __LP64__ */
#endif /* __APPLE__ */
For Windows/Linux/UNIX it cannot work properly, see this annotated code snippet:
#if !defined(TCL_WIDE_INT_TYPE)&&!defined(TCL_WIDE_INT_IS_LONG)*
/*****************************************************************
 This is the case when compiling outside of Tcl core, because none
 of these constants is pre-defined. Mac is excluded here, because
 handled above.
*******************************************************************/
#   if defined(_WIN32)
/******************************************************************
 This branch will be reached when compiling under Windows, even
 when compiling with any other compiler than MSVC.
 Now TCL_WIDE_INT_TYPE will be defined even on 64 bit architectures,
 but due to documentation on 64 bit architectures the constant
 TCL_WIDE_INT_IS_LONG should be defined instead.
 Note that on 64 bit machines the constant _WIN32 is also
 pre-defined.
 Also note that the definition of TCL_WIDE_INT_TYPE is probably
 incorrect when compiling with an other compiler than MSVC.
*******************************************************************/
#      define TCL_WIDE_INT_TYPE __int64
#      ifdef __BORLANDC__
#         define TCL_LL_MODIFIER  "L"
#      else /* __BORLANDC__ */
#         define TCL_LL_MODIFIER  "I64"
#      endif /* __BORLANDC__ */
#   elif defined(__GNUC__)
/******************************************************************
 This is the case when compiling with gcc under Linux/UNIX.
 Now TCL_WIDE_INT_TYPE will be defined, whether 32 bit or 64 bit
 architecture, but due to documentation on 64 bit architectures
 the constant TCL_WIDE_INT_IS_LONG should be defined instead.
 And the definition of TCL_LL_MODIFIER is incorrect on 64 bit
 architectures, should be "l" instead.
*******************************************************************/
#      define TCL_WIDE_INT_TYPE long long
#      define TCL_LL_MODIFIER "ll"
#   else /* ! _WIN32 && ! __GNUC__ */
#      ...
#   endif /* _WIN32 */
#endif /* !TCL_WIDE_INT_TYPE & !TCL_WIDE_INT_IS_LONG */
A technical note: for me it seems to be more logical to change the order for __GCC__ and _WIN32, because the __GCC__ branch works in any case, whether Windows or not:
#if defined(__GCC__)
#   ...
#elif defined(_WIN32)
#   ...
#endif
At this place I suggest to use the opportunity for modernizing the definitions of 64 bit support based on C99, see this example:
#if HAVE_STDINT_H
#   include <stdint.h>
#elif defined(_MSC_VER) && _MSC_VER < 1600
/* work around for the support of ancient compilers */
#   include "tkWinStdInt.h"
#else
/* this is not expected with compilers from this century, except MSVC (handled above) */
#   error "C99 support is required - can't include stdint.h"
#endif

typedef int64_t  Tcl_WideInt;
typedef uint64_t Tcl_WideUInt;

#if HAVE_STDINT_H
#   if (UINTPTR_MAX == 0xffffffffu)
#	define TCL_IS_32_BIT_ARCH
#   elif (UINTPTR_MAX >= 0xffffffffffffffffu)
        /* We handle >64 bit also as 64 bit architectures. */
#	define TCL_IS_64_BIT_ARCH
#   else
#	error "unsupported architecture" /* can never happen */
#   endif
#elif defined(_WIN64) /* ancient compiler support */
#   define TCL_IS_64_BIT_ARCH
#elif defined(_WIN32) /* ancient compiler support */
#   define TCL_IS_32_BIT_ARCH
#else
#   error "cannot detect architecture" /* can never happen */
#endif /* HAVE_STDINT_H */

#if TCL_IS_64_BIT_ARCH
#   define Tcl_WideAsLong(val)    ((long)(val))
    ...
#else  /* if TCL_IS_32_BIT_ARCH */
#   define Tcl_WideAsLong(val)    ((long)((Tcl_WideInt)(val)))
    ...
#endif /* TCL_IS_64_BIT_ARCH */

#if defined(_MSC_VER) /* only MSVC (Windows) */
#   define TCL_LL_MODIFIER    "I64"
#elif defined(TCL_IS_64_BIT_ARCH)
#   define TCL_LL_MODIFIER    "l"
#else /* if TCL_IS_32_BIT_ARCH */
#   define TCL_LL_MODIFIER    "ll"
#endif /* _WIN32 */

...

For file tkWinStdInt.h see repository of Tk project revised_text. Note that every compiler of these days is supplying the header files <stdint.h> and <inttypes.h> (belongs to C99). The ancient compiler support (old MSVC versions) is only for ActiveState (they don't have enough money for a newer compiler?).

Please note that the Tk developers would benefit with the modern definitions, because in this project <stdint.h> will be used, and also the differentiation between 32 and 64 bit architectures is important for Tk.

User Comments: anonymous added on 2017-03-24 17:33:14:

Yes, I trapped into a pitfall because of this comment in tcl.h:

 * TCL_WIDE_INT_IS_LONG - if wide ints are really longs (i.e. we're on a real
 *	64-bit system.)
This comment "we're on a real 64-bit system" is misleading. That's why I've used this constant for testing whether it's a 64 bit machine. But probably I didn't understand the term "real" here.

BTW: And this shows that a modernization is more clear, because in C99 you can simply use:

typedef uint64_t Tcl_WideInt;
#define TCL_LL_MODIFIER __PRI64_PREFIX

But of course here the name TCL_LL_MODIFIER is not useful anymore, should be TCL_WIDE_INT_MODIFIER.


jan.nijtmans added on 2017-03-24 16:03:01:
Well, I think you misunderstand the intention of TCL_WIDE_INT_IS_LONG. On Win64, sizeof(long) == 4 and sizeof(void *) == 8. So, although pointers are 64-bit, long is 32-bit. So it is correct that on Win32/Win64, TCL_WIDE_INT_IS_LONG is NOT
defined. That's as expected! TCL_WIDE_INT_IS_LONG doesn't tell anything about the platform being 32-bit or 64-bit. It just tells whether "long" is 32-bit or 64-bit. Not really what you expect, isn't it?

In your analysis, you expect TCL_WIDE_INT_IS_LONG to be set on Win64. Well, that's incorrect, and that's where your analysis is wrong. Sorry ;-)

anonymous added on 2017-03-24 15:02:56:

I don't have a 64 bit architecture, so I cannot test. On my 32 bit machine indeed the mechanism works. But I refer to verbal statements:

  1. I asked Francois Vogel - he is compiling under Windows on a 64 bit architecture - whether the constant TCL_WIDE_INT_IS_LONG is defined, and he tested with:

    #ifdef TCL_WIDE_INT_IS_LONG
    # error here
    #endif
    
    His response was that the compiler accepted, with other words, TCL_WIDE_INT_IS_LONG is not defined, although he has a 64 bit machine.

  2. See [cd374e7d] and [f8055298], the author says "TCL_WIDE_INT_IS_LONG doesn't behave as expected on WIN64". I conclude that TCL_WIDE_INT_IS_LONG wasn't set on his 64 bit machine.

What's wrong with my analysis of tcl.h, due to my analysis it cannot work. Certainly it works if TCL_WIDE_INT_IS_LONG or TCL_WIDE_INT_TYPE is already properly defined by call of compiler -
gcc -DTCL_WIDE_INT_IS_LONG=1
- is it possible that in your test environment the makefile contains this definition?. Outside of Tcl core, when TCL_WIDE_INT_IS_LONG is not predefined, the mechanism cannot work.

If you remove -DTCL_WIDE_INT_IS_LONG=1, what happens now?

If it works although you have removed -DTCL_WIDE_INT_IS_LONG=1, in which line of tcl.h the correct setup (TCL_WIDE_INT_IS_LONG=1) will be done?


jan.nijtmans added on 2017-03-24 13:35:48:

So, if it (as you claim) doesn't work outside of the core, the best thing to do is create a test-case. Committed here: dd4d4f822dd767de This test-case works fine on any platform I know.

So, what's wrong with this test-case? Why does it work despite you claiming that it doesn't work?

Sure, the 64-bit support could be improved, the current macro organization is not really 'transparant'. But - still - I fail to see the bug here.

In my view, TCL_LL_MODIFIER should be "I64" on Win32/Win64 (any compiler, including gcc), "l" on LP64 systems, "ll" on any other system. TCL_WIDE_INT_TYPE should be __int64 on Win32/Win64 (any compiler, including gcc), long on LP64 systems, and long long on any other system. What am I missing?