Tcl Source Code

View Ticket
Login
Ticket UUID: 3571241
Title: _USE_32BIT_TIME_T wrong for 32bit __MINGW32__
Type: Bug Version: current: 8.5.14
Submitter: earnie Created on: 2012-09-24 15:28:49
Subsystem: 52. Portability Support Assigned To: nijtmans
Priority: 5 Medium Severity:
Status: Closed Last Modified: 2013-05-22 17:18:11
Resolution: Invalid Closed By: nijtmans
    Closed on: 2013-05-22 10:18:11
Description:
The use of _USE_32BIT_TIME_T requires libmsvcr80.a and the MSVCR80.DLL.  The MinGW.org team requires that distributions for software only require MSVCRT.DLL since this is the known system default.  To resolve the issue I removed the define for this macro in win/tclWinPort.h to build the software.
User Comments: nijtmans added on 2013-05-22 17:18:11:

allow_comments - 1

Closing this, because it is not a Tcl bug.

See:
<https://sourceforge.net/p/mingw/bugs/1973/>

earnie added on 2013-05-22 04:28:53:

File Added - 463473: use-32bit-time-t-debacle.diff

earnie added on 2013-05-22 04:28:05:
>  and that's how Microsoft is using it in it's own header files. I think that mingw should do the same.

We simply cannot.  We support legacy while newer versions of MSVC are supporting cutting-edge.  It's a nightmare either for the compilation, the link or the execution depending on what we do.  MinGW builds to -lmsvcrt by default since it is guaranteed to be on the system as the system C library;  while MSVC builds to -lmsvcr## by default (where ## is the version used by the compiler) since it is part of the product.

Now the MinGW user can specify -lmsvcr80 to get the appropriate functions but that runs into known collision issues since msvcrt.dll and mscvr80.dll will both be used and they both control memory independently so it is discouraged.

Now I have a dilemma in that I need to support at least Windows 2000 and its version of MSVCRT.DLL which will not contain the needed functions for _USE_32BIT_TIME_T and even if the OS does such as Vista and above the MinGW libmsvcrt.a import library doesn't define them since we cannot ensure that they are contained in it.  The MinGW user has a choice to use the MSVCRT.DLL directly to link against and that works when the functions exist in it.

I need to think about what we want to do and query the users to determine best course of action for MinGW but since you've included the _USE_32BIT_TIME_T macro by default you'll need to be aware of the options for the user and document it.

I've attached the patch I'm contemplating.  It adds a warning when MSVCRT_VERSION is not >= 800 and _USE_32BIT_TIME_T is defined.  It currently defines _USE_32BIT_TIME_T if MSVCRT_VERSION is >= 800 but I don't think that will stay since libmsvcrt.a doesn't contain the import of the functions.  It will depend on the users desires after I ask them about it.

earnie added on 2013-05-20 20:10:39:
> _USE_32BIT_TIME_T should be used to switch back time_t to
32-bit, when the default is 64-bit. 

I cannot do that, there are 64bit versions of XP; just not widely used.

> If functions are missing
from some dll version, appropriate inline wrapper functions can be
defined which do the same in terms existing functions, this way it
should be made to work in all situations.

I've thought of that, but it will not happen for the 4.0 release.

> _USE_32BIT_TIME_T should be used to switch back time_t to
32-bit, when the default is 64-bit.

I'll do some further research on that.  It was confusing to me in the original MinGW headers why _USE_32BIT_TIME_T meant use 64bit.

nijtmans added on 2013-05-20 03:57:29:
> The _USE_32BIT_TIME_T should be set based on the version of MSVCRT.DLL. In
> other words on XP OS version the time structures for time_t and the stat
> structures do not work with _USE_32BIT_TIME_T is defined.

If you replace _USE_32BIT_TIME_T to _HAVE_32BIT_TIME_T in
your description, then I quite agree: _HAVE_32BIT_TIME_T can
be eliminated using MSVCRT_VERSION. If functions are missing
from some dll version, appropriate inline wrapper functions can be
defined which do the same in terms existing functions, this way it
should be made to work in all situations.

_USE_32BIT_TIME_T should be used to switch back time_t to
32-bit, when the default is 64-bit. That's the switch that
Microsoft made in VC2005, and that's how Microsoft is
using it in it's own header files. I think that mingw
should do the same.

earnie added on 2013-05-19 22:58:37:
The issue is the version of MSVCRT.DLL on the users system.

The _USE_32BIT_TIME_T should be set based on the version of MSVCRT.DLL.  In other words on XP OS version the time structures for time_t and the stat structures do not work with _USE_32BIT_TIME_T is defined.

I'm working to develop some means of an autoconf m4 macro that can be used to determined which MSVCRT.DLL version we have so we know the supporting structures at build time.  It will set something like MSVCRT_VERSION to the value of the MSVCRT.DLL version based on MajorLinkerVersion and MinorLinkerVersion obtained from $WINDIR/system32/msvcrt.dll via objdump.  The _USE_32BIT_TIME_T will then be obsoleted and replaced by MSVCRT_VERSION.  I will default to XP OS for MSVCRT_VERSION when it is not defined.

I sent a query to the mingw-users list to find the values represented by the OSes available.  The following is the list of values for (MajorLinkerVersion * 100) + MinorLinkerVersion:

ME - 600
XP - 710
Vista - ???
Win7 - 900
Win8 - 1010

I'm guessing Vista has the 32bit time_t structures but I don't know yet.  I hope to get a VM built soon to find out.

Note, that this will cause a backward incompatibility of a binary built on those OS supporting 32bit time_t versus those who do not.  The work around for those who do not would be to find the MSVCR##.DLL where ## is >= 80 and copy it to MSVCRT.DLL in a non-standard location and add the non-standard location to the PATH before the Windows directories or copy it into the directories with the TCL binaries.

Blame MS for this PITA we feel but they have every right to update the MSVCRT.DLL for each new OS.

nijtmans added on 2013-05-19 21:29:32:
My patch was sofar tested with the 8.4.20-rc2 only. If
you want, I can post a new patch later, when I'm done
testing with other Tcl versions. But I hope from the
current patch, you get the idea what's wrong.

I also created a special version of Tcl 9.0 (still highly
experimental), which makes the switch from 32-bit
time_t to to 64-bit time_t:
    <http://core.tcl.tk/tcl/info/807f110dd7>
This compiles fine with VS2005+ and with your
w32api-4.0-rc1, but it's binary incompatible
with Tcl 8.x, that's why this switch can
only be done in a new major Tcl version.

Thanks!

earnie added on 2013-05-18 03:07:44:
The patch breaks on XP giving SEGV errors when doing the ``make test' step.  I'm testing with the 8.6.0 version.  More later ...

earnie added on 2013-05-16 22:25:05:
Thanks for the patch.  I'll try to incorporate it into the -rc2 release coming soon.

nijtmans added on 2013-05-16 21:13:52:
When trying the latest w32api-4.0-rc1, I noticed that
the described problem popped-up again. The
fact that _USE_32BIT_TIME_T requires msvcr80
is really a mingw problem, not a Tcl problem:
Older msvcrt.dll's have all 32-bit functions,
they just don't have "32" in the function
names.

See attached "mingw.patch" for my
(minimal) fix to the mingw headers.
It is sufficient for Tcl to compile and
run as expected, but it is not tested
thoroughly.

Please consider this for your
w32api-4.0-rc2.

nijtmans added on 2013-05-16 20:59:59:

File Added - 463372: mingw.patch

earnie added on 2012-09-26 03:50:07:
[quote]Are you saying that Tcl compiled with mingw and HAVE_32BIT_TIME_T set will work on WOW64 but not on Windows XP?[/quote]

Yes and no.  The functions exist in the DLL but not in the libmsvcrt.a import library supplied by MinGW.org nor by mingw-w64.  I need to think about how to deliver an import library for the differing systems.  If the user uses msvcrt.dll directly to resolve the symbols instead of the import library then yes it should work.  I'll experiment with it.

[quote]You could try tclvfs, That's a package known to use the Tcl_StatBuf structure. See: https://sourceforge.net/projects/tclvfs/[/quote]

Thanks, I'll give that a try.

nijtmans added on 2012-09-26 02:57:39:
> Further inspection shows me that my Windows XP 32bit system doesn't contain
> the 32bit functions in msvcrt.dll but my Windows 7 SysWOW64/msvcrt.dll
> does.
Are you saying that Tcl compiled with mingw and _HAVE_32BIT_TIME_T set will
work on WOW64 but not on Windows XP?

> Do you have any test scripts or know of another tcl dependent package I can
> use to validate this with?
You could try tclvfs, That's a package known to use the Tcl_StatBuf
structure. See:
https://sourceforge.net/projects/tclvfs/

Regards,
        Jan Nijtmans

earnie added on 2012-09-26 02:40:09:
After further thought, study and the response from twylite, I have reworked the alias functions such that if _HAVE_32BIT_TIME_T is defined it will call the 32bit functions otherwise they will call the 64bit functions and do datatype typing as appropriate.  The fortunate thing is that the tm structure is the same regardless of the call being used.

Further inspection shows me that my Windows XP 32bit system doesn't contain the 32bit functions in msvcrt.dll but my Windows 7 SysWOW64/msvcrt.dll does.

Do you have any test scripts or know of another tcl dependent package I can use to validate this with?

twylite added on 2012-09-25 23:12:31:
"In versions of Visual C++ and Microsoft C/C++ before Visual C++ 2005, time_t was a long int (32 bits) and hence could not be used for dates past 3:14:07 January 19, 2038, UTC. In Visual C++ 2005, time_t is equivalent to __time64_t by default, but defining _USE_32BIT_TIME_T changes time_t to __time32_t and forces many time functions to call versions that take the 32-bit time_t." -- http://msdn.microsoft.com/en-us/library/w4ddyt9h(v=vs.80).aspx

This macro is required to force time_t to 32-bits (as expected by the Tcl sources in a 32-bit environment) when the compiler is MSVC2005 or later.  Earlier versions of the compiler don't recognise the macro and safely ignore it.

What you are saying is that the MinGW headers respect _USE_32BIT_TIME_T, but the libraries you link against don't.  In other words you have a mismatch between the headers and the libraries.  That is a problem in your development environment, not with Tcl.

"Since a 32bit time_t is an after thought and was given to help out legacy
code and since 64bit time_t is the default it should be up to the users of
the source to have it enabled or not during the configure process with
disabled being the default to match the compilers."

The need for _USE_32BIT_TIME_T relates to assumptions in the Tcl sources about typecasting and time_t arithmetic in a 32-bit environment.  The Tcl developers decide that, not the person compiling the software.

earnie added on 2012-09-25 22:59:23:
_USE_32BIT_TIME_T was designed as an after thought to resolve a legacy code compatibility issue.  It should only be defined for those applications that require a time_t structure that is 32 bits.  By default, time_t is 64 bits and only 64 bits time_t structure is provided by the default runtime.

So if you define _USE_32BIT_TIME_T in your code you need to provide a library that supports the functions something like -lmsvcr80 but then the user must be aware that msvcr80.dll is required and it might not be since it isn't supplied by default.

The changes I've made to the repository code isn't a regression the existing code was regressed already.  The project specified _USE_32BIT_TIME_T but it was previously not used properly.  With my changes the project specifying _USE_32BIT_TIME_T give the appropriate declarations and must also specify a library during the link that contains those functions.  However, the linked application might not run due to dependencies on non-standard libraries.

While mingw-w64 64bit compiled code will work since _USE_32BIT_TIME_T isn't defined in tclWinPort.h because _WIN64 is defined; the mingw-w64 32bit compiled code will give the same link time error since they have no means to guard based on the version of MSVCRT.  The new code for MinGW.org _USE_32BIT_TIME_T is now more inline with that of mingw-w64 code.

The resolution here is to define HAVE_32BIT_TIME_T when --with-32bit-time is specified and one of -lmsvcr80, -lmsvcr80d, -lmsvcr90, -lmsvcr90d, -lmsvcr100, -lmsvcr100d, -lmsvcr110 or -lmsvcr110d is found and the resulting binary works during configure.  Then modify tclWinPort.h to filter the define for _USE_32BIT_TIME_T based on HAVE_32BIT_TIME_T.

Since a 32bit time_t is an after thought and was given to help out legacy code and since 64bit time_t is the default it should be up to the users of the source to have it enabled or not during the configure process with disabled being the default to match the compilers.

nijtmans added on 2012-09-25 20:32:23:
I'm sorry that this causes a regression in mingw, but the fact that it
compiles fine in older mingw versions, in mingw_w64 and all msvc
versions tell me that the problem is really in mingw.
> It is an issue now for MinGW.org because the current repository
> code, which will soon be released, has the extra guards removed and has
> exposed the issue
So, that's where the regression is. mingw shouln't redefine localtime
to _localtime32 for msvcrt versions where this function doesn't
exist, independant from the setting of _USE_32BIT_TIME_T.
In older MSVC headers, _USE_32BIT_TIME_T didn't have
any effect, so in the mingw headers for MSVCRT < 2005
the setting of _USE_32BIT_TIME_T should not have any
effect either. It's an incompatibility with MSVC < 2005

earnie added on 2012-09-25 19:25:40:
I downloaded the latest package 8.5.12.  I used the tcl package to test changes I am making to the MinGW.org mingwrt and w32api source before I distribute them.

The problem is that _USE_32BIT_TIME_T is set explicitly in tclWinPort.h as delivered.  The functions enabled by _USE_32BIT_TIME_T do not exist in the MSVCRT.DLL.  _WIN64 doesn't have the functions at all so there is a guard to not define _USE_32BIT_TIME_T.  _WIN32 has the functions but only in a non-standard DLL.

The problem occurs during linking with ld complaining about unresolved symbols for __localtime32, __mktime32 and __gmtime32.  They are unresolved because libmsvcrt.a does not contain the symbols.  These symbols are used based on _USE_32BIT_TIME_T in include/time.h when we create an alias function of localtime, mktime and gmtime respectively.

Now, with the current release of w32api this may not have been an issue because there were some extra nonsense (IMO) guards which prevented the use of these symbols even when _USE_32BIT_TIME_T was enabled.  With MSVC it wouldn't cause an issue because the DLL used by it would contain these symbols.  It is an issue now for MinGW.org because the current repository code, which will soon be released, has the extra guards removed and has exposed the issue.  The issue is that MSVCRT.DLL does not contain the functions enabled by _USE_32BIT_TIME_T.

nijtmans added on 2012-09-25 15:37:16:
Some questions:
What Tcl version are you talking about? 8.5.?, 8.6.?

>an dependency that cannot be guaranteed to be available
A dependancy with what? Tcl compiles out-of-the-box on
mingw, mingw_w64 and MSVC, without the need for
a configuration option. _USE_32BIT_TIME_T is only
set when BUILD_tcl is set, in tclWinPort.h which is
not supposed to be used by extensions anyway.
So, what's the problem?

earnie added on 2012-09-25 01:58:18:
The problem is that _USE_32BIT_TIME_T is the wrong solution for the original issue.  It should not be used since this causes an dependency that cannot be guaranteed to be available.  Perhaps a use of a configure time option of HAVE_LOCALTIME32 would be a better solution and a --with-localtime32 option be used with configure to set it.

nijtmans added on 2012-09-24 23:50:36:
So, what's the problem?

Attachments: