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

 
 

bug #51351: mktime() crashes on Windows

Submitted by:  Philip Nienhuis <philipnienhuis>
Submitted on:  Fri 30 Jun 2017 09:09:40 PM UTC  
 
Category: LibrariesSeverity: 3 - Normal
Priority: 5 - NormalItem Group: Regression
Status: FixedAssigned to: None
Originator Name: philipnienhuisOpen/Closed: Closed
Release: devOperating System: Microsoft Windows

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission 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>
Project Administrator
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>
Project 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>
Project Administrator
Fri 07 Jul 2017 01:23:51 PM UTC, comment #11:

Yes, as I suspected, we need to set both

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>
Project Administrator
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>
Project Administrator
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:

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>
Project Member
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>
Project 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>
Project 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>
Project Member
Sat 01 Jul 2017 02:15:44 AM UTC, comment #5:

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

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>
Project Administrator
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>
Project 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>
Project 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>
Project 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.

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).

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>
Project 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>
Project Member

 

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

Attach File(s):
   
   
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
  • -unavailable- added by mmuetzel (Posted a comment)
  • -unavailable- added by lostbard (Posted a comment)
  • -unavailable- added by mtmiller (Posted a comment)
  • -unavailable- added by jwe (Posted a comment)
  • -unavailable- added by rik5 (Posted a comment)
  • -unavailable- added by philipnienhuis (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Sat 08 Jul 2017 03:59:37 PM UTCmtmillerStatusPatch Submitted=>Fixed
      Open/ClosedOpen=>Closed
    Fri 07 Jul 2017 01:25:45 PM UTCmtmillerAttached File-=>Added mxe.diff, #41147
      StatusNone=>Patch Submitted

    Back to the top


    Powered by Savane 3.1-cleanup1