bugGNU Octave - Bugs: bug #51351, mktime() crashes on Windows

 
 

bug #51351: mktime() crashes on Windows

Submitter:  Philip Nienhuis <philipnienhuis>
Submitted:  Fri 30 Jun 2017 09:09:40 PM UTC
   
 
Category:  Libraries Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Regression
Status:  Fixed Assigned to:  None
Originator Name:  philipnienhuis Open/Closed:  * Closed
Release:  * dev Operating System:  * Microsoft Windows
Fixed Release:  None Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 08 Jul 2017 03:59:37 PM UTC, comment #14: 

Pushed to mxe-octave

https://hg.octave.org/mxe-octave/rev/e9b032b9340a

Yes, defining the extra variable does no harm. On the stable branch, the tzset function is not even tested for. So the variable is set to bypass a test that is never done. But just in case that changes, or when that feature appears on stable, we don't have to remember to add it later.

Mike Miller <mtmiller>
Group Member
Fri 07 Jul 2017 09:45:33 PM UTC, comment #13: 

That patch works at least for 4.3.0+ (dev). Thanks very much for this fix.
(That such a small detail induced so much work ...)

BTW why is it needed for stable-octave, as 4.2.1 is not affected by the gnulib repo update? But maybe it doesn't harm?

Philip Nienhuis <philipnienhuis>
Group Member
Fri 07 Jul 2017 01:25:45 PM UTC, comment #12: 

Please try the attached patch to mxe-octave

(file #41147)

Mike Miller <mtmiller>
Group Member
Fri 07 Jul 2017 01:23:51 PM UTC, comment #11: 

Yes, as I suspected, we need to set both


gl_cv_func_gettimeofday_clobber=no
gl_cv_func_tzset_clobber=no


when cross-building for Windows. See this recent thread on the bug-gnulib list (https://lists.gnu.org/archive/html/bug-gnulib/2017-07/msg00054.html)

In the same message Bruno says they will try to add hints to allow cross-compiling for Windows to do the right thing.

Mike Miller <mtmiller>
Group Member
Tue 04 Jul 2017 12:58:31 AM UTC, comment #10: 

Thanks for debugging this. It's possible that something in gnulib is not quite cross-building right. The localtime-buffer.c module was added to gnulib in May.

Does anyone know why the Octave build files in mxe-octave force gl_cv_func_gettimeofday_clobber to be set to "no"? The time changes in gnulib add a new dependency on tzset, do we also need to set gl_cv_func_tzset_clobber=no for the same reason when cross-building?

Mike Miller <mtmiller>
Group Member
Mon 03 Jul 2017 11:55:10 PM UTC, comment #9: 

I am seeing the same issue as Philip on a cross build from hg id 1147e5c912aa.
Running Philip's test with gdb on Win10 64bit yields:

octave:1> t = localtime (time ())
t =

  scalar structure containing the fields:

    usec =  697191
    sec =  19
    min =  12
    hour =  1
    mday =  4
    mon =  6
    year =  117
    wday =  2
    yday =  184
    isdst =  1
    gmtoff = 0
    zone = Mitteleuropõische Sommerzeit

octave:2> mktime (t)

Thread 1 received signal SIGSEGV, Segmentation fault.
rpl_localtime (timep=0x63eed8, timep@entry=<error reading variable: DWARF-2 expression error: Loop detected (257).>)
    at /home/[...]/mxe-octave-2/tmp-default-octave/octave-4.3.0+/libgnu/localtime-buffer.c:38
38      /home/[...]/mxe-octave-2/tmp-default-octave/octave-4.3.0+/libgnu/localtime-buffer.c: No such file or directory.


The backtrace shows hundreds and hundreds of recursive calls to the same function "rpl_localtime" from the same line 38 where "localtime" is called. I stopped at around 800 deep in the stack without reaching where the function was originally called from.
Please, let me know if I can do anything else to help debugging.

Markus Mützel <mmuetzel>
Group administrator
Mon 03 Jul 2017 07:18:30 PM UTC, comment #8: 

With (only) the "offending" cset 9578bbaf2bbd backed out, I get an installer that runs fine in Windows 7 and finishes __run_test_suite__.m to the end w/o crash.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jul 2017 04:28:14 PM UTC, comment #7: 

Perhaps related - bug #51057.

The vido package was crashing because of a call to localtime.  If I changed it to ::localtime, it worked ok.

John Donoghue <lostbard>
Group Member
Sat 01 Jul 2017 09:05:31 AM UTC, comment #6: 

(1) I think confirmation is needed from other people building & running Windows dev builds. (Although it is reliably reproducible on my systems.)
Any dev cross-build from after 30 May should portray this bug.
The test build bots don't run _run_test_suite_, do they?

BTW my systems: Win7 Prof. 64bit  &  Win10 Prof. 64 bit
My build system = Mageia-5 64bit w. gcc-4.9.2


(2) (@Mike/@Rik) a test case is extremely simple (taken from time-cc.test):

t = localtime (time ())
mktime (t)

For fine builds I get 1.4-something (anyway a number) and Octave returns to a prompt.
For the "wrong" builds Octave stalls for a few seconds and then I get the dreaded Windows pop-up telling that Octave has just crashed ("octave-gui has stopped working ... Problem Event Name  APPCRASH ...")

I do have an old msys setup, I use that for OF package development (mainly hg, tar, etc). It does have gcc 4.7.2.  If I can get instructions I'm happy to try & help out.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jul 2017 02:15:44 AM UTC, comment #5: 

Nothing obviously having to do with time from what I can see:


 ChangeLog                 |  147 ++++++++++++++++++++++++++++++++++++++++++++++
 build-aux/texinfo.tex     |   58 +++++++++--------
 doc/build-automation.texi |   39 +++++------
 doc/gnulib.texi           |    2 +-
 gnulib-tool               |   59 ++++++++++++------
 lib/canonicalize-lgpl.c   |    6 +-
 lib/classpath.c           |    6 +-
 lib/diffseq.h             |    8 +-
 lib/float.in.h            |    2 +-
 lib/getopt-pfx-core.h     |    5 +
 lib/getopt-pfx-ext.h      |    5 +
 lib/gettext.h             |    3 +-
 lib/localename.c          |    2 +-
 lib/same-inode.h          |    2 +-
 lib/savewd.c              |   12 +++-
 lib/sched.in.h            |    9 +-
 lib/stat-w32.c            |    6 +
 lib/xalloc-oversized.h    |    2 +-
 m4/float_h.m4             |    6 +-
 m4/fmal.m4                |    4 +-
 m4/frexpl.m4              |    4 +-
 m4/host-cpu-c-abi.m4      |   19 ++++-
 m4/logbl.m4               |    4 +-
 m4/sched_h.m4             |   21 +++++-
 m4/sys_types_h.m4         |    5 +-
 modules/fstat             |    6 +-
 modules/stat              |    6 +-
 modules/strtod-obsolete   |    2 +-
 modules/uniwidth/base     |    2 +-
 modules/uniwidth/width    |    2 +-
 30 files changed, 346 insertions(+), 108 deletions(-)


Philip - how do you know that mktime is causing the crash? Do you have a test case where you can call mktime(some_struct) and result in a crash? Can we reduce this to a C test so someone who has a working msys development environment on Windows can git clone gnulib and check it out independently?

Mike Miller <mtmiller>
Group Member
Sat 01 Jul 2017 01:53:08 AM UTC, comment #4: 

Have there been any additional changes to these functions in gnulib since the last time we (I) updated?

Maybe a test for the gnulib function independent of Octave would be simpler?

John W. Eaton <jwe>
Group administrator
Fri 30 Jun 2017 10:36:45 PM UTC, comment #3: 

Yes, the double build is a killer.

I just recently used bisect to track down a bug and it took about a week.  In other words, I feel your pain.

In my case it was unclear whether the bug came from stable or dev and bisect would keep reporting that a merge cset was the problem.

Do you have test code in Octave that generates the problem?  I can at least try it with my Windows XP virtual machine and see if I get a crash as well.

Rik <rik5>
Group administrator
Fri 30 Jun 2017 10:29:14 PM UTC, comment #2: 

Yes I agree that this is probably an upstream bug.
Looking at the gnulib overview you provided I suppose it's the late April / early May 2017 series of changes that are suspicious.

Unfortunately my C++ proficiency is too lacking to come up with a proper test case.

I'm a little put off as well by the amount of work that was needed to pinpoint the cause (> 2 weeks and, most of all, the double build process (make dist followed by cross-building) and continuous reboot alternation between Windows and Linux. That process has IMO unduly monopolized my build box).

For me it may be simpler to just backout the cset in question and see if that works for me.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 30 Jun 2017 09:38:54 PM UTC, comment #1: 

Checking Octave's Mercurial repository, updates to gnulib occurred in May, 2017 and Dec, 2016.


changeset:   23539:9578bbaf2bbd
user:        John W. Eaton <jwe@octave.org>
date:        Tue May 30 12:38:27 2017 -0400
summary:     maint: Update gnulib subrepo to latest changes.

changeset:   22889:2ffc87d7d9a4
parent:      22886:5ede69715ebe
user:        John W. Eaton <jwe@octave.org>
date:        Tue Dec 13 15:03:21 2016 -0500
summary:     maint: Update gnulib subrepo to latest changes.


gnulib's git repository shows the following changes to the mktime module in that period (http://git.savannah.gnu.org/gitweb/?p=gnulib.git&a=search&h=HEAD&st=commit&s=mktime).


2017-05-09         Bruno Haible        tzset: Expand comment about TZ problem on native Windows.
* lib/mktime.c (mktime): Likewise.

2017-05-05         Bruno Haible        time-c++-tests: Update.
(tzalloc, tzfree, localtime_rz, mktime_z): Declare, missing since

2017-05-02         Paul Eggert        tzset: update doc for TZ problems on MS-Windows
...localtime.texi, doc/posix-functions/mktime.texi:

2017-05-01         Bruno Haible        mktime: Fix dependencies.
mktime: Fix dependencies.
* modules/mktime (Depends-on): Add 'time'.

2017-04-30         Bruno Haible        mktime: Work around TZ problem on native Windows.
mktime: Work around TZ problem on native...
* lib/mktime.c: Add #ifs to make the algorithmic...
* m4/mktime.m4 (gl_FUNC_MKTIME_WORKS): New...
gl_FUNC_MKTIME. If guessing, set gl_cv_func_working_mk...
(gl_FUNC_MKTIME): Require it. Require AC_CANONICAL_HOST.
Set REPLACE_MKTIME to 1 on native Windows. Define...
NEED_MKTIME_WINDOWS.
(gl_FUNC_MKTIME_INTERNAL): Require gl_FUNC_MKTIME_WORKS...
gl_FUNC_MKTIME. Set WANT_MKTIME_INTERNAL, not...
NEED_MKTIME_INTERNAL.
...gl_FUNC_TIMEGM): Require gl_FUNC_MKTIME_WORKS, not
gl_FUNC_MKTIME. Cope with 'guessing yes' value.
* modules/mktime-internal (configure.ac): Test...
not REPLACE_MKTIME.
* doc/posix-functions/mktime.texi: Mention the native Windows

2017-04-30         Bruno Haible        Document the problem with the Cygwin environment variable TZ.
* doc/posix-functions/mktime.texi: Likewise.

2017-04-27         Pádraig Brady        time_rz: fix heap buffer overflow vulnerability
localtime_rz and mktime_z were affected since commit 4bc76593.

2017-04-22         Paul Eggert        parse-datetime: fix %z and prefer signed int
(parse_datetime2): Simplify call to debug_mktime_not_ok.

2017-01-21         Paul Eggert        parse-datetime: handle timezones reentrantly
(mktime_ok, get_effective_timezone):

2017-01-05         Assaf Gordon        parse-datetime: add debug warning about DST changes
where 'mktime' returns a different isdst value.

2017-01-05         Assaf Gordon        parse-datetime: add debug warning about date arithmetic
...invalid dates. Normalization with 'mktime(3)'


This looks like an upstream bug.  Can you make a test case in C which reproduces the problem?  It is probably necessary to report the bug on their mailing lint.

Rik <rik5>
Group administrator
Fri 30 Jun 2017 09:09:40 PM UTC, original submission:  

cset 9578bbaf2bbd "maint: Update gnulib subrepo to latest changes." from May 30 has the result that mktime() makes Octave crash on Windows. I suppose gnulib has introduced a botched mktime or settime module.

The crash is reproducible and happens as soon as time-cc.test is reached in _run_test_suite_, or any function is invoked that relies on mktime().

Sidenote:
It took me a long time to find out which cset is responsible for this bug, as for each bisect I needed to cross-build, reboot in Windows and test.
Very deceivingly, once "hg upgrade"d to 9578bbaf2bbd, "downgrading" (hg upgrade -r 09205eeddbef = to previous cset) doesn't seem to replace the faulty gnulib module. The only way to get a properly working Octave in Windows (= with proper mktime) is to clone (or copy from a cloned) Octave repo, then hg upgrade -r 09205eeddbef, only then invoke ./bootstrap and configure...make and then build Octave in mxe-octave.

Philip Nienhuis <philipnienhuis>
Group Member

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #41147:  mxe.diff added by mtmiller (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by lostbard (Posted a comment)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by philipnienhuis (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-08 mtmiller StatusPatch Submitted Fixed
        Open/ClosedOpen Closed
    2017-07-07 mtmiller Attached File- Added mxe.diff, #41147
        StatusNone Patch Submitted

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code