Incorrect Time#utc_offset value of Rubyinstaller 2.0.0-p247

[phasis]

Rubyinstaller 2.0.0-p247 has a bug with Time#utc_offset.

This bug is mentioned at https://bugs.ruby-lang.org/issues/9119 

With Rubyinstaller 2.0.0-p247:

C:\Users\phasis>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"

ruby 2.0.0p247 (2013-06-27) [i386-mingw32]

32400

With Rubyinstaller 1.9.3-p448:

C:\Users\phasis>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"

ruby 1.9.3p448 (2013-06-27) [i386-mingw32]

0

With Rubyinstaller 2.0.0-p247(x64):

C:\Users\phasis>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"

ruby 2.0.0p247 (2013-06-27) [x64-mingw32]

0

BTW, with my own compiled version(Windows 7 64bit, DevKit-mingw64-32-4.7.2-20130224-1151) , Ruby 2.0.0-p247 works fine.

C:\Users\phasis>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"

ruby 2.0.0p247 (2013-06-27 revision 41674) [i386-mingw32]

0

So, I think that this is not a bug of ruby code but a bug of build environment.

I guess this is related with the version of msvcrt.dll.

I used msvcrt.dll of version 7.0.7601.17744.

What is the version of msvcrt.dll on the Rubyinstaller build platform?

How can I reproduce the same behavior of the Rubyinstaller?

Regards,

Park Heesob

[Jon]

On my Win8.1 64-bit system with a 64-bit JRuby 1.7.8 and a 32-bit MRI built with the mingwbuilds-32-4.7.3 32-bit toolchain, I see

C:\>uru ruby -e "ENV['TZ']='UTC';p Time.now.utc_offset"
jruby 1.7.8 (1.9.3p392) 2013-11-14 0ce429e on Java HotSpot(TM) 64-Bit Server VM 1.7.0_45-b18 [Windows 8-amd64]

0

ruby 2.0.0p351 (2013-11-21 revision 43741) [i386-mingw32]

-18000


C:\Windows\System32\msvcrt.dll is version 7.0.9600.16384

Do you think this could be a bug in the build toolchain, perhaps something in certain versions of the mingw-w64 CRT?

Someone should update my devkit build recipe config and try with the latest 32 and 64-bit versions of 4.8.2.

Jon

[Jon]

Seems there's no pre-built 32-bit 4.8.2 to easily update the devkit build config from

http://sourceforge.net/projects/mingw-w64/files/Toolchains%20targetting%20Win64/Personal%20Builds/mingw-builds/4.8.2/threads-win32/sjlj/

Do you have a snippet of C that shows the same failure that we can use to quickly test out different mingw-w64 toolchain versions rather than rebuilding all of mri?

If so, using the new goodies from MSYS2 one could examine the C snippet using 32 and 64bit mingw-w64 toolchains from their pacman repos:

$ pacman -Ss i686
...
mingw32/mingw-w64-i686-gcc 4.8.2-1 (mingw-w64-i686-toolchain mingw-w64-i686)
    GNU Compiler Collection (C,C++,OpenMP) for MinGW-w64

$ pacman -Ss x86_64
...
mingw64/mingw-w64-x86_64-gcc 4.8.2-1 (mingw-w64-x86_64-toolchain mingw-w64-x86_64)
    GNU Compiler Collection (C,C++,OpenMP) for MinGW-w64

[phasis]

2013년 11월 23일 토요일 오전 1시 27분 54초 UTC+9, Jon 님의 말:

I don't think changing toolchain solve the problem. You already tried mingwbuilds-32-4.7.3 32-bit toolchain.

I just want to know what is the cause of different behavior on the same OS, same version of msvcrt.dll and same toolchain.

Here is a snippet of C that shows the difference.

=====================================

#include <stdlib.h>

#include <time.h>

#include <stdio.h>

int main()

{

    time_t t;

    struct tm *tm;

    time_t t2;

    if (~(time_t)0 <= 0) {

        t = (((time_t)1) << (sizeof(time_t) * 8 - 2));

        t |= t - 1;

    }

    else {

        t = ~(time_t)0;

    }

   printf("t=%ld,sizeof(t)=%d\n",t,sizeof(time_t));

   tm = localtime(&t);

   printf("tm = %p\n",tm);

   t2 = mktime(tm);

   printf("t2=%ld\n",t2);

    return 0;

} 

====================================

On i686-w64-mingw32 gcc version 4.7.2 (rubenvb-4.7.2-release):

t=2147483647,sizeof(t)=4

tm = 00540D30

t2=-1

On x86_64-w64-mingw32 gcc version 4.7.2 (rubenvb-4.7.2-release):

t=-1,sizeof(t)=8

tm = 0000000000000000

t2=-1

Same result on both Windows 7 and Windows 8.1

Regards,

Park Heesob 

[Jon]

Yes, odd.

My results to compare to yours as both my toolchains have newer mingw-w64 header versions. Mine seem similar to yours.

# 32-bit
C:\>gcc -v
...
Target: i686-w64-mingw32
...
Thread model: win32
gcc version 4.7.3 (rev1, Built by MinGW-builds project)

C:\>gcc -Wall -O2 -o tm.exe time_check.c

C:\>tm.exe
t=2147483647,sizeof(t)=4
tm = 00660C50
t2=2147483647


# 64-bit
C:\>gcc -v
...
Target: x86_64-w64-mingw32
...
Thread model: win32
gcc version 4.8.2 (rev0, Built by MinGW-W64 project)

C:\>gcc -Wall -O2 -o tm.exe time_check.c
time_check.c: In function 'main':
time_check.c:17:5: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'time_t' [-Wformat=]
     printf("t=%ld,sizeof(t)=%d\n",t,sizeof(time_t));
     ^
time_check.c:17:5: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long long unsigned int' [-Wformat=]
time_check.c:21:5: warning: format '%ld' expects argument of type 'long int', but argument 2 has type 'time_t' [-Wformat=]
     printf("t2=%ld\n",t2);
     ^

C:\>tm.exe

[phasis]

2013년 11월 23일 토요일 오전 9시 3분 57초 UTC+9, Jon 님의 말:

I finally figured out what is the cause.

The only difference between you and me is the timezone.

My timezone and ruby developers timezone is UTC+9, and your timezone is UTC-5.

I found the above code returns different value with timezone

For timezone from UTC-12 to UTC+0,

t=2147483647,sizeof(t)=4

tm = 00760D30

t2=2147483647

For timezone from UTC+1 to UTC+13,

t=2147483647,sizeof(t)=4

tm = 005A0D30

t2=-1

Here is a modified code with same timezone setting.

================================

#include <stdlib.h>

#include <time.h>

#include <stdio.h>

int main()

{

    time_t t;

    struct tm *tm;

    time_t t2;

    if (~(time_t)0 <= 0) {

        t = (((time_t)1) << (sizeof(time_t) * 8 - 2));

        t |= t - 1;

    }

    else {

        t = ~(time_t)0;

    }

   putenv("TZ=JSP-9");

   printf("t=%ld,sizeof(t)=%d\n",t,sizeof(time_t));

   tm = localtime(&t);

   printf("tm = %p\n",tm);

   t2 = mktime(tm);

   printf("t2=%ld\n",t2);

    return 0;

}

=================================

Regards,

Park Heesob

 

[Luis Lavena]

Hello Park,

On Fri, Nov 22, 2013 at 10:49 PM, phasis <pha...@gmail.com> wrote:

I finally figured out what is the cause.

The only difference between you and me is the timezone.

My timezone and ruby developers timezone is UTC+9, and your timezone is UTC-5.

I found the above code returns different value with timezone

For timezone from UTC-12 to UTC+0,

t=2147483647,sizeof(t)=4

tm = 00760D30

t2=2147483647

For timezone from UTC+1 to UTC+13,

t=2147483647,sizeof(t)=4

tm = 005A0D30

t2=-1

Great findings! RubyInstaller automated builds are processed at UTC-3, that is why we see the constant failure.

If nobu doesn't merge your suggestion to Ruby soon, I'll over the weekend.

Thank you for taking the time and look into this.

--
Luis Lavena
AREA 17
-
Perfection in design is achieved not when there is nothing more to add,
but rather when there is nothing more to take away.
Antoine de Saint-Exupéry

[Jon]

Nice.

I just tried your fix for `configure.in` on ruby_2_0_0. It solves the marshall bug, but on my machine it causes a new TZ related test failure

[12463/13458] TestTime#test_marshal_zone = 0.00 s
 78) Failure:
test_marshal_zone(TestTime) [c:/Users/Jon/Documents/RubyDev/ruby-git/test/ruby/test_time.rb:309]:
<"JST"> expected but was
<"UTC">.

Do you see the same when you build the latest ruby_2_0_0?

[phasis]

2013년 11월 25일 월요일 오전 3시 25분 44초 UTC+9, Jon 님의 말:

I can see the same failure with UTC-5 timezone.

You can see a patch for timezone test related issue on https://bugs.ruby-lang.org/issues/9119

Regards,

Park Heesob

 

[Jon]

 

Nice.

I just tried your fix for `configure.in` on ruby_2_0_0. It solves the marshall bug, but on my machine it causes a new TZ related test failure

[12463/13458] TestTime#test_marshal_zone = 0.00 s
 78) Failure:
test_marshal_zone(TestTime) [c:/Users/Jon/Documents/RubyDev/ruby-git/test/ruby/test_time.rb:309]:
<"JST"> expected but was
<"UTC">.

Do you see the same when you build the latest ruby_2_0_0?

I can see the same failure with UTC-5 timezone.

You can see a patch for timezone test related issue on https://bugs.ruby-lang.org/issues/9119

Regards,

Park Heesob

 

Using your patches tweaked for ruby_2_0_0

http://paste.ubuntu.com/6474735/

all the TZ related failures disappear from my Win8.1 64bit machine when building using mingwbuilds-32-4.7.3

[ 4312/13458] TestGDBM#test_s_open_error = 0.02 s
 13) Failure:
test_s_open_error(TestGDBM) [c:/Users/Jon/Documents/RubyDev/ruby-git/test/gdbm/test_gdbm.rb:227]:
[Errno::EACCES, Errno::EWOULDBLOCK] exception expected, not
Class: <Errno::E10035>
Message: <"A non-blocking socket operation could not be completed immediately. - C:/Users/Jon/AppData/Local/Temp/tmptest_gdbm20131125-11856-1s8z4ce/tmptest_gdbm_11856">
---Backtrace---
c:/Users/Jon/Documents/RubyDev/ruby-git/test/gdbm/test_gdbm.rb:228:in `open'
c:/Users/Jon/Documents/RubyDev/ruby-git/test/gdbm/test_gdbm.rb:228:in `block in test_s_open_error'
---------------

...SNIP...

[ 8175/13458] TestProcess#test_execopts_unsetenv_others = 0.06 s
 62) Failure:
test_execopts_unsetenv_others(TestProcess) [c:/Users/Jon/Documents/RubyDev/ruby-git/test/ruby/test_process.rb:382]:
<""> expected but was
<"PATH=C:\\WINDOWS\\system32\\NV;.;\n">.

...SNIP...

Finished tests in 521.031981s, 25.8295 tests/s, 4284.1804 assertions/s.
13458 tests, 2232195 assertions, 2 failures, 0 errors, 104 skips

ruby -v: ruby 2.0.0p353 (2013-11-22 revision 43783) [i386-mingw32]


Perhaps the GDBM test should be skipped on Windows, and the TestProcess fail investigated. But both errors have nothing to do with TZ. Nice job. Thank you.

I don't have a mingw.org 32bit toolchain to double check that both `_gmtime32_s` and `_localtime32_s` are available. Likely you've already confirmed this.

The only minor concern I have with the patch series is whether `defined(__MINGW_VERSION_MAJOR)` should be used. If the goal is to check for a mingw-w64 based toolchain, then it's more correct. However, the `defined(__MINGW64__) || defined(__MINGW32__)` appear fine for this scenario. I suspect the original author thought that `defined(__MINGW64__)` was a valid check for mingw-w64 toolchain. It is not. It's a check for a mingw-w64 toolchain in 64bit mode.

Jon

[Jon]

Typo. Should be `defined(__MINGW64_VERSION_MAJOR)`

[Jon]

Please verify this patch with your 32 and 64bit builds. This revised patch also corrects my two ruby_2_0_0 TZ-related issues fixed with your original patch.

http://paste.ubuntu.com/6475407/

I've removed your defines because I'm concerned that they force `time_t` to use the 32bit `__time32_t` versions rather than the 64bit versions

Also, what's the intent of this win32/win32.c code:

#ifdef __MINGW64__
# ifndef MINGW_HAS_SECURE_API
   _CRTIMP errno_t __cdecl _gmtime64_s(struct tm* tm, const __time64_t *time);
   _CRTIMP errno_t __cdecl _localtime64_s(struct tm* tm, const __time64_t *time);
# endif
# define gmtime_s _gmtime64_s
# define localtime_s _localtime64_s
#endif

but I suspect the original author didn't like the MINGW_HAS_SECURE_API guarded declarations of `_gmtime64_s` and `_localtime64_s` in the mingw-w64 headers when building with a 64-bit mingw-w64 toolchain. Perhaps they thought they were overriding mingw-w64 behavior for both 32 and 64bit builds.

http://sourceforge.net/p/mingw-w64/code/HEAD/tree/tags/v2.0.8/mingw-w64-headers/crt/sec_api/time_s.h#l11

Could this issue also be fixed by hoisting it to a `configure.in` test that adds MINGW_HAS_SECURE_API to the list of defines?

I've not looked at the 32bit mingw.org headers or tested with one of their toolchains. The mingw.org toolchains may be a don't care for RubyInstaller. It's been awhile since I've retired from the RubyInstaller project, but I believe Luis is focused on officially supporting only the mingw-w64 32 and 64-bit toolchains. Luis can clarify.

[phasis]

2013년 11월 26일 화요일 오전 4시 48분 34초 UTC+9, Jon 님의 말:

If I remove the #elif define part, I can see the following error.

compiling win32/win32.c

win32/win32.c: In function 'gmtime_r':

win32/win32.c:7030:5: error: implicit declaration of function 'gmtime_s' [-Werror=implicit-function-declaration]

cc1.exe: some warnings being treated as errors

make: *** [win32/win32.o] Error 1

 

Also, what's the intent of this win32/win32.c code:

#ifdef __MINGW64__
# ifndef MINGW_HAS_SECURE_API
   _CRTIMP errno_t __cdecl _gmtime64_s(struct tm* tm, const __time64_t *time);
   _CRTIMP errno_t __cdecl _localtime64_s(struct tm* tm, const __time64_t *time);
# endif
# define gmtime_s _gmtime64_s
# define localtime_s _localtime64_s
#endif

but I suspect the original author didn't like the MINGW_HAS_SECURE_API guarded declarations of `_gmtime64_s` and `_localtime64_s` in the mingw-w64 headers when building with a 64-bit mingw-w64 toolchain. Perhaps they thought they were overriding mingw-w64 behavior for both 32 and 64bit builds.

http://sourceforge.net/p/mingw-w64/code/HEAD/tree/tags/v2.0.8/mingw-w64-headers/crt/sec_api/time_s.h#l11

Could this issue also be fixed by hoisting it to a `configure.in` test that adds MINGW_HAS_SECURE_API to the list of defines?

 

The author is Hiroshi Shirosaki  and the revision is http://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/36563

You can make a feature request for MINGW_HAS_SECURE_API.

I've not looked at the 32bit mingw.org headers or tested with one of their toolchains. The mingw.org toolchains may be a don't care for RubyInstaller. It's been awhile since I've retired from the RubyInstaller project, but I believe Luis is focused on officially supporting only the mingw-w64 32 and 64-bit toolchains. Luis can clarify.

It seems that mingw.org toolchain is outdated. 

The mingw.org toolchain cannot build with the above patch as you mentioned.

I made a mofied patch only for MINGW64 toolchain.

Regards,

Park Heesob

[Jon]

Odd. Using the updated patch, I'm able to reproduce an error (a different linker error than your compiler error) when I use the official 32-bit 4.7.2 devkit and build like

rake ruby19 local=C:\Users\Jon\Documents\RubyDev\ruby-git openssl=1.0.1e libffi=3.0.13 rubygems=2.1.11 dkver=mingw64-32-4.7.2
...
linking miniruby.exe
win32/win32.o: In function `gmtime_r':
C:\Users\Jon\Documents\RubyDev\ri-git\sandbox\ruby19_build/../../../ruby-git/win32/win32.c:6655: undefined reference to `gmtime_s'
collect2.exe: error: ld returned 1 exit status
make: *** [miniruby.exe] Error 1


However, when I use a newer 32-bit mingw-w64 4.7.3 devkit and build like the following, no build errors and all the TZ related `make test-all` errors disappear

rake ruby19 local=C:\Users\Jon\Documents\RubyDev\ruby-git openssl=1.0.1e libffi=3.0.13 rubygems=2.1.11 dkver=mingwbuilds-32-4.7.3


Are the errors you see only for 32-bit builds and no build errors or TZ related errors when you build ruby_2_0_0 with the official 64-bit 4.7.2 devkit?

[phasis]

2013년 11월 26일 화요일 오후 11시 48분 37초 UTC+9, Jon 님의 말:

It is not odd. The error message which I mentioned is raised on ruby-trunk.

What is the updated patch? After applying my modified patch, the line no 6655 is "return NULL".

I guess you omitted the elif part.

#elif __MINGW64_VERSION_MAJOR

# define gmtime_s _gmtime32_s

# define localtime_s _localtime32_s

However, when I use a newer 32-bit mingw-w64 4.7.3 devkit and build like the following, no build errors and all the TZ related `make test-all` errors disappear

rake ruby19 local=C:\Users\Jon\Documents\RubyDev\ruby-git openssl=1.0.1e libffi=3.0.13 rubygems=2.1.11 dkver=mingwbuilds-32-4.7.3

In  tims_s.h file of 32-bit mingw-w64 4.7.2 devkit, I can see only localtime_s function definition.

It seems that tims_s.h file of 32-bit mingw-w64 4.7.3 devkit added gmtime_s function definition.

Are the errors you see only for 32-bit builds and no build errors or TZ related errors when you build ruby_2_0_0 with the official 64-bit 4.7.2 devkit?

I have no build errors and no TZ related errors for 32-bit builds and for 64-bit builds with official 4.7.2 devkits after applying the modified patch.

Regards,

Park Heesob 

[Jon]

My problem with the above code is that the 32bit structs will be used and only dates through 19-Jan-2038 can be represented. The 64bit structs represent dates through 31-Dec-3000.

http://msdn.microsoft.com/en-us/library/bf12f0hc%28v=vs.110%29.aspx
http://msdn.microsoft.com/en-us/library/0z9czt0w%28v=vs.110%29.aspx


My updated patch is http://paste.ubuntu.com/6475407/ It differs from your earlier patch in the following ways:

* backported to apply to ruby_2_0_0 rather than trunk
* untested on trunk
* removes the 32bit "fallback" defines for `gmtime_s` and `localtime_s` and counts on the mingw-w64 headers to do the right thing
* solves TZ issues when `mingwbuilds-32-4.7.3` devkit is used
* fails the build when the official `mingw64-32-4.7.2` devkit is used
* solves TZ issues when `mingw64-32-4.8.2` devkit (my unreleased update to the devkit build recipes) is used

The next chance I'll have to work on this is mid-December, but I'm hoping we can discover a final patch that forces the use of the 64bit time structs. IIRC, both the 64bit time structs and the secure *_s time functions are available from WinXP SP3 and higher.

Jon

[phasis]

2013년 11월 28일 목요일 오전 7시 38분 34초 UTC+9, Jon 님의 말:

I understand your intention.

A problem of your patch is that 32-bit integer will be used as time_t type even if you can use 64bit structs.

 

I propose  a patch for 64-bit time_t and 64bit time structs as a final patch

http://paste.ubuntu.com/6487604/

The patch is for trunk.

Regards,

Park Heesob

[Jon]

 

I understand your intention.

A problem of your patch is that 32-bit integer will be used as time_t type even if you can use 64bit structs.

 

I propose  a patch for 64-bit time_t and 64bit time structs as a final patch

http://paste.ubuntu.com/6487604/

The patch is for trunk.

With your new patch backported to ruby_2_0_0

http://paste.ubuntu.com/6490353/

all TZ related `test-all` errors are fixed when I build `ruby 2.0.0p353 (2013-11-22 revision 43783) [i386-mingw32]` with the following toolchains:

* mingw64-32-4.7.2
* mingwbuilds-32-4.7.3
* mingw64-32-4.8.2

Thank you.

Jon