Tcl Source Code

View Ticket
Login
Ticket UUID: 2651823
Title: Tcl_StatBuf definition problem with VS2005 and Win XP x64
Type: Bug Version: obsolete: 8.5.6
Submitter: lubatti Created on: 2009-03-01 13:49:10
Subsystem: 52. Portability Support Assigned To: patthoyts
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2011-06-28 00:20:14
Resolution: Fixed Closed By: sf-robot
    Closed on: 2011-06-27 17:20:14
Description:
TCL version 8.5.6

I found a problem when trying to build TCL version 8.5.6 using Visual Studio 2005 SP1 on a AMD64 machine with Windows XP Professional x64 edition.
I'm not able to compile, since when  I give the command:

nmake -nologo -b -f makefile.vc MACHINE=AMD64 release

I get the following error (see file error.log):

NMAKE : fatal error U1077: '"D:\Program Files (x86)\Microsoft Visual Studio 8\VC\BIN\amd64\cl.EXE"' : return code '0x2'

during compilation I noticed the following unrecoverable error (see file run.log):

..\generic\tclCmdAH.c(818) : error C2079: 'buf' uses undefined struct '_stati64'
..\generic\tclEncoding.c(3522) : error C2079: 'stat' uses undefined struct '_stati64'
..\generic\tclFCmd.c(113) : error C2079: 'statBuf' uses undefined struct '_stati64'
..\generic\tclFCmd.c(229) : error C2079: 'statBuf' uses undefined struct '_stati64'
..\generic\tclFCmd.c(365) : error C2079: 'statBuf' uses undefined struct '_stati64'
..\generic\tclFCmd.c(489) : error C2079: 'sourceStatBuf' uses undefined struct '_stati64'
..\generic\tclFCmd.c(489) : error C2079: 'targetStatBuf' uses undefined struct '_stati64'
..\generic\tclFileName.c(2525) : error C2027: use of undefined type '_stati64'
        d:\fausto\tcl_tk_8.5.6\tcl8.5.6\generic\tcl.h(372) : see declaration of '_stati64'
..\generic\tclIOUtil.c(68) : error C2079: 'buf' uses undefined struct '_stati64'
..\generic\tclIOUtil.c(1748) : error C2079: 'statBuf' uses undefined struct '_stati64'
..\generic\tclIOUtil.c(2886) : error C2079: 'buf' uses undefined struct '_stati64'
..\generic\tclIOUtil.c(4130) : error C2079: 'sourceStatBuf' uses undefined struct '_stati64'

All those structure refer to tyoe Tcl_StatBuf which is defined in the tcl.h, so I revert to this file.
So I looked ot the file tcl.h in the generic dirctory of tcl sources and look at the original definition, which I report here:


#         if _MSC_VER < 1400 || !defined(_M_IX86)
typedef struct _stati64Tcl_StatBuf;
#         else
typedef struct _stat64Tcl_StatBuf;
#         endif /* _MSC_VER < 1400 */
#         define TCL_LL_MODIFIER"I64"
#      endif /* __BORLANDC__ */

It looks like that if you are using a version less than Vs 2005 or you are not building for a X86 (like I am) machine, I get the definition

typedef struct _stati64Tcl_StatBuf;

which is not good for the compiler.

I changed the test with a slightly different version:

#         if _MSC_VER >= 1400 && defined(_M_X64)
typedef struct _stat64Tcl_StatBuf;
#         elif _MSC_VER < 1400 || !defined(_M_IX86)
typedef struct _stati64Tcl_StatBuf;
#         else
typedef struct _stat64Tcl_StatBuf;
#         endif /* _MSC_VER < 1400 */
#         define TCL_LL_MODIFIER"I64"
#      endif /* __BORLANDC__ */

so on my machine (AMD64) and building for an AMD64 with VS 2005 I get the following definition:

typedef struct _stat64Tcl_StatBuf;

The modification to the if statement will leave the original definitions for versions less than VS 2500, that I presume was the intent of the original statement, 
and also leaves the original definition when building on Windows XP Professional Edition for X86 (32 bit version).

As a general suggestion, I would change that statement, and rather than testing _M_IX86 and _M_X64, which identify the specific processor, and probably is not pertinent
too much with the definition of Tcl_statBuf, i would use the variable _WIN64 which identify if you are building wor a win32 or win64 system, leaving the alternatuve definition
only for old Visual Studio version that build for win64 (if ever supported...)

#         if _MSC_VER < 1400 && defined(_WIN64)
typedef struct _stati64Tcl_StatBuf;
#         else
typedef struct _stat64Tcl_StatBuf;
#         endif /* _MSC_VER < 1400 */
#         define TCL_LL_MODIFIER"I64"
#      endif /* __BORLANDC__ */


I have no other Visual Studio versions, or other machines to see if this works also on higher version of Visual Studio, or different machine architecture.

I attach the following files:

make_amd64.bat   (the bat that I use to rebuild on Windows XP x64 for the AMD64 target machine using Visual Studio 2005 SP1)
make_win32.bat   (the bat that I use to rebuild on Windows XP for the win32 target machine using Visual Studio 2005 SP1)
run.log          (the log file produced when building on the x64 machine using AMD64 as the target machine)
error.log        (the error log file produced when building on the x64 machine using AMD64 as the target machine)
tcl.h            (the tcl.h file that build correctly both on x64 and on win32 machine)
tcl_original.h   (the original 8.5.6 distribuited tcl.h file that fails on x64 builds correctly on win32 machine)

Hope this helps.

Regards

Fausto Lubatti
[email protected]
User Comments: sf-robot added on 2011-06-28 00:20:14:

allow_comments - 1

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

dgp added on 2011-06-13 23:48:23:
I have no ability to review, but unless
someone tests otherwise, I'm assuming 
this is "fixed", at least as fixed as is possible.

nijtmans added on 2011-04-27 17:21:30:
Patt, I think this one can be closed now. Since
the fix for [Bug 3288345] fixes this one too.

Please, evaluate.

dkf added on 2009-10-28 17:29:32:
We know about the general problem with size_t (it's issue#219196, assigned to me; now over 10 years old) but we also know we can't do anything about it without breaking both API and ABI completely (or going mad!) so we're putting off doing anything about it in the 8.* series. It's also off-topic for *this* issue.

mgammon added on 2009-10-28 16:43:37:
I have been trying to build Tcl 8.5.7 under XP64, and not getting very far.
I found this posting from March this year where you ran in to build problems for the AMD64 platform.

I've made the same fix to tcl.h to cure the Tcl_StatBuf compile failures and this fixes that problem, but I still see some worrying warnings about mismatched sizes, e.g.

..\generic\regcomp.c(468) : warning C4267: 'function' : conversion from 'size_t'  to 'unsigned int', possible loss of data

..\generic\regexec.c(224) : warning C4267: 'function' : conversion from 'size_t'  to 'unsigned int', possible loss of data

The first looks to be a fundamental problem caused by Tcl_Alloc declaring it size arg as unsigned int rather than size_t. Possibly not a critical problem so long as nothing in Tcl allocates more than 4Gb. 

The second is caused by subtracting pointers which results in a 64bit int and packing this into a long, which on AMD64 is 32bit. This looks like a more serious problem.

Looking back through other postings on Tcl and I found TIP #115 Making Tcl Truly 64-Bit Ready. This seems to describe a lot of the issues i'd expect to see with doing a AMD64 port. Its dated 2002 and doesn't seem to have gone any further.

I guess what I'm really asking is Tcl 8.5.7 ( or 8.6b1) really supported on AMD64?
I can't find a definitive statement about this but the code and postings seem to suggest that its not properly supported, yet? Can anyone confirm this?

Thanks
Mark

lubatti added on 2009-04-30 18:40:17:
Hi,

in the meantime, I've been able to rebuilt Tcl/Tk version 8.5.6 both win32 and amd64 on my machines using Visual Studio 2005 SP1 and following Pat's instructions to use Tcl_StatBuf_ defined in tclInt.h.
I've run some tests, and both versions seem to work the same way.
Unfortunately, I can't give my contribution on the binary compatibility question: it looks like MS Visual C++ is navigating in heavy darkness with this compatibility issues...

Fausto

patthoyts added on 2009-04-30 18:02:38:
What I'm pointing out is that the binary compatability is broken.

We have a further mess: MSDN states that "In versions of Visual C++ before Visual C++ 2005 time_t was 32 bits ... In Visual C++ 2005 time_t is equivalent to __time64_t by default"
So this means that we have an incompatible type between extensions and executable compiled with differing compiler versions for anything using time_t as well. This means 5 Tclp* functions that manipulate dates and anything using Tcl_StatBuf. Defining _USE_32BIT_TIME_T will restore the old size for 32bit targets.

So just specifying time_t in the Tcl_StatBuf structure wont be sufficient. We're still going to have to jiggle this per compiler version.

dkf added on 2009-04-30 17:56:35:
Be very careful with changing those definitions; Unixes really prefer to use their system versions, and quite often include platform-specific bits in there too (which Tcl mostly ignores, so it's only useful when backed by the native FS, but there you go...)

The original problem was that 'struct stat' was used in Tcl's public API back in 8.3 and before. (Bleah.)

patthoyts added on 2009-04-30 16:56:29:
This is a bigger mess than I thought. Using the following program we can explore the size of the Tcl_StatBuf structure members for Tcl compiled with various compilers:
#include <sys/stat.h>
#include <tcl.h>
int main(void) {
    Tcl_StatBuf st; st.st_size = 0;
    printf("time_t %u st_size %u st_atime %u\n",
        sizeof(time_t), sizeof(st.st_size), sizeof(st.st_atime));
    return 0;
}
Using this with tcl8.4 headers we get:
ix86 gcc-3.4.5 time_t 4 st_size 4 st_atime 4
ix86 msvc-6    time_t 4 st_size 8 st_atime 4
ix86 msvc-9    time_t 8 st_size 8 st_atime 8

amd64 msvc9     time_t 8 st_size 8 st_atime 8
amd64 msvc8     time_t 8 st_size 8 st_atime 8
amd64 gcc-4.4.0 time_t 8 st_size 4 st_atime 8 

So it's pretty well screwed whichever way you go. There is no chance that an extension compiled with gcc will be able to use this structure properly in a msvc compiled tclsh.

We can fix this with the most hideously complex looking pile of crap:
#if defined(__GNUC__)
#    if defined(_WIN64)
typedef struct _stat64 Tcl_StatBuf;
#    else /* !_WIN64 */
typedef struct _stati64 Tcl_StatBuf;
#    endif /* !_WIN64 */
#else /* !__GNUC__*/
#    if defined(__BORLANDC__)
typedef struct stati64 Tcl_StatBuf;
#    else /* !__BORLANDC__ */
#        if _MSC_VER < 1500
typedef struct _stati64 Tcl_StatBuf;
#        else  /* _MSC_VER >= 1500 */
#            if defined(_M_IX86)
#                define _USE_32BIT_TIME_T
typedef struct _stat32i64 Tcl_StatBuf;
#            else /* !defined(_M_IX86) */
typedef struct _stat64 Tcl_StatBuf;
#            endif /* !defined(_M_IX86) */
#        endif /* _MSC_VER >= 1400 */
#    endif /* !__BORLANDC__ */
#endif /* !__GNUC__*/

This produces a 32bit time_t and 64bit st_size member for 32 bit Windows with msvc6 and 9 and gcc 3. On 64bit windows we get 64 bit time_t and 64 bit st_size. 

With msvc9 (aka MSVC 2008 or cl 1500) the compiler headers for IX86 seem to produce a 64bit time_t by default so I've had to include the _USE_32BIT_TIME_T. I don't understand the use of this in the Microsoft headers. In the newer gcc headers they define this macro when the target system will be a 32 bit version of windows which seems correct. In the MSVC headers, with this macro defined _stati64 becomes defined as _stat32i64 which is what we need.

Hideous.

Given that this is utterly broken anyway I see no reason not to use an explicitly defined structure with the correct Tcl types as mentioned is earlier posts.

patthoyts added on 2009-04-30 04:51:06:
The applied patch fixes the build with MSVC 2008 but breaks it for every other version so I reverted the patch from both branches.

In tclWinFile.c NativeStat function we explicitly assign all the time members as time_t and the size member as a Tcl_WideInt. So all the ifdeffery is just to try to obtain a stat structure that matches this usage. Once upon a time (msvc6 up to msvc8) this was _stati64. More recently this structure has disappeared and in msvc9 we need to use _stat32i64 on ix86 and stat64 on x86_64. Who is to say what they'll do next time around.

So I suggest we define it thusly:
struct Tcl_StatBuf_ {
    _dev_t st_dev;
    _ino_t st_ino;
    unsigned short st_mode;
    short st_nlink;
    short st_uid;
    short st_gid;
    _dev_t st_rdev;
    Tcl_WideInt st_size;
    time_t st_atime;
    time_t st_mtime;
    time_t st_ctime;
};
I've been building a version of tcl with that and so far it seems robust. tclvfs works with this which seemed like the most likely candidate to fail in the face of Tcl_StatBuf mismatches.

dkf added on 2009-04-29 22:01:00:
Applied fix derived from patch. Assigning to Pat for his further consideration.

patthoyts added on 2009-04-29 21:59:29:
I will take this opportunity to point everyone to http://paste.tclers.tk/1538 where I propose to stop this macro mayhem for the Windows headers. Everytime a new version of the compiler or SDK comes out we have to dance around this again.
We need to either adopt an alternative as per the paste site patch or apply this one. The concern with switching to our own definition is what extensions do we screw up? If any.

Comments.....

dkf added on 2009-03-02 15:56:39:
Thanks for the submission of the right gunk for that particular compiler/platform.

Prio9 so it is flagged to be dealt with in next release

lubatti added on 2009-03-01 20:49:10:

File Added - 315664: Tcl_StatBuf_problem_x64.zip

Attachments: