Bug #9119

TestTime#test_marshal_broken_offset broken under MinGW

Added by Luis Lavena over 1 year ago. Updated over 1 year ago.

[ruby-core:58391]
Status:Closed
Priority:Normal
Assignee:Nobuyoshi Nakada
ruby -v:ruby 2.1.0dev (2013-11-17 trunk 43698) [i386-mingw32] ruby 2.0.0p350 (2013-11-13 revision 43661) [i386-mingw32] Backport:1.9.3: UNKNOWN, 2.0.0: UNKNOWN

Description

=begin
Hello,

The test (({TestTime#test_marshal_broken_offset})) has been broken under x86-mingw32 for really long time, in both 2.0 and trunk:

http://ci.rubyinstaller.org/job/ruby-2_0_0-x86-test-all/170/console

http://ci.rubyinstaller.org/job/ruby-trunk-x86-test-all/2362/console

TestTime#test_marshal_broken_offset [C:/Users/Worker/Jenkins/workspace/ruby-trunk-x86-build/test/ruby/test_time.rb:331]:
expected but was
<-10800>.
=end

patch Magnifier (1.27 KB) Heesob Park, 11/25/2013 05:12 PM

modified.patch Magnifier (1.25 KB) Heesob Park, 11/26/2013 11:40 AM

Associated revisions

Revision 43713
Added by Akira Tanaka over 1 year ago

  • configure.in (LOCALTIME_OVERFLOW_PROBLEM): Define it for cross compiling. [Bug #9119] Reported by Luis Lavena. Analyzed by Heesob Park.

Revision 43713
Added by Akira Tanaka over 1 year ago

  • configure.in (LOCALTIME_OVERFLOW_PROBLEM): Define it for cross compiling. [Bug #9119] Reported by Luis Lavena. Analyzed by Heesob Park.

Revision 44406
Added by Nobuyoshi Nakada over 1 year ago

win32/win32.c: mingw jungle

  • configure.in: let mingw do something black-magic, and check if _gmtime64_s() is available actually.
  • win32/win32.c (gmtime_s, localtime_s): use _gmtime64_s() and _localtime64_s() if available, not depending on very confusing mingw variants macros. based on the patch by phasis68 (Heesob Park) at . [Bug #9119]

Revision 44406
Added by Nobuyoshi Nakada over 1 year ago

win32/win32.c: mingw jungle

  • configure.in: let mingw do something black-magic, and check if _gmtime64_s() is available actually.
  • win32/win32.c (gmtime_s, localtime_s): use _gmtime64_s() and _localtime64_s() if available, not depending on very confusing mingw variants macros. based on the patch by phasis68 (Heesob Park) at . [Bug #9119]

History

#1 Updated by Akira Tanaka over 1 year ago

  • Assignee changed from Akira Tanaka to Nobuyoshi Nakada

The test is added at r42596 by nobu.

#2 Updated by Heesob Park over 1 year ago

I can reproduce this bug with rubyinstaller 2.0.0-p247 version.

C:\Users\phasis>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"
ruby 2.0.0p247 (2013-06-27) [i386-mingw32]
32400

But, I cannot reproduce with my own compiled version with the same development kit with the rubyinstaller.
C:\work>ruby -ve "ENV['TZ']='UTC';p Time.now.utc_offset"
ruby 2.0.0p247 (2013-06-27 revision 41674) [i386-mingw32]
0

I guess this is related with the tzset runtime function and msvcrt.dll version.

#3 Updated by Heesob Park over 1 year ago

I found a difference between the rubyinstaller version and mine.

The line "#define LOCALTIME_OVERFLOW_PROBLEM 1" is only found in my config.h.

It seems that the localtime(3) overflow check code returns incorrect value for some i686-w64-mingw32 compiler environment.

#4 Updated by Akira Tanaka over 1 year ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r43713.
Luis, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • configure.in (LOCALTIME_OVERFLOW_PROBLEM): Define it for cross compiling. [Bug #9119] Reported by Luis Lavena. Analyzed by Heesob Park.

#5 Updated by Akira Tanaka over 1 year ago

2013/11/18 phasis68 (Heesob Park) phasis@gmail.com:

Issue #9119 has been updated by phasis68 (Heesob Park).

I found a difference between the rubyinstaller version and mine.

The line "#define LOCALTIME_OVERFLOW_PROBLEM 1" is only found in my config.h.

It seems that the localtime(3) overflow check code returns incorrect value for some i686-w64-mingw32 compiler environment.

Hm. I hope that r43713 fix this problem.
--
Tanaka Akira

#6 Updated by Akira Tanaka over 1 year ago

  • Status changed from Closed to Assigned

#7 Updated by Heesob Park over 1 year ago

Finally, I found the reason why I cannot reproduce the same result with Rubyinstaller test.
The timezone of Rubyinstaller test server is UTC-3 and my timezone is UTC+9.

The localtime(3) overflow check code returns yes for timezone from UTC-12 to UTC+0, whereas returns no for timezone from UTC+1 to UTC+13.

Here is a patch for the config test.

diff --git a/configure.in b/configure.in.new
index 5d86e14..9d09d58 100644
--- a/configure.in
+++ b/configure.in.new
@@ -2137,6 +2137,7 @@ int
main()
{
time_t t;
+ putenv("TZ=JST-9");
if (~(time_t)0 <= 0) {
t = (((time_t)1) << (sizeof(time_t) * 8 - 2));
t |= t - 1;

#8 Updated by Heesob Park over 1 year ago

After applying above patch, TestTime#test_marshal_broken_offset test passes.
But TestTime#test_marshal_zone test fails on UTC-5 timezone.

I inspected this issue and found that it is a mingw 32-bit compiler specific issue.
The MSVC and MINGW64 compiler calls _gmtime64_s and _localtime64_s crt function for localtime_r implementation.
The other compiler calls FileTimeToSystemTime API function which ignores TZ environment variable.

I made a patch for timezone related test.

#9 Updated by Heesob Park over 1 year ago

I noticed the attached patch has problems with mingw.org 32bit toolchain.

The updated patch is only affects for MINGW64 toolchain.

Refer to https://groups.google.com/forum/#!topic/rubyinstaller/zNNccqwWMsA for more information.

#10 Updated by Heesob Park over 1 year ago

Is there any concerns about this issue?

I made another patch.
http://paste.ubuntu.com/6487604/
It solves the test failure and also enforce 64-bit time_t type on MinGW64 32-bit build.

#11 Updated by Nobuyoshi Nakada over 1 year ago

  • Status changed from Assigned to Closed

This issue was solved with changeset r44406.
Luis, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


win32/win32.c: mingw jungle

  • configure.in: let mingw do something black-magic, and check if _gmtime64_s() is available actually.
  • win32/win32.c (gmtime_s, localtime_s): use _gmtime64_s() and _localtime64_s() if available, not depending on very confusing mingw variants macros. based on the patch by phasis68 (Heesob Park) at . [Bug #9119]

Also available in: Atom PDF