Thread overview
[Issue 12507] New: SysTime.init.toString should not segfault
Apr 03, 2014
Vladimir Panteleev
Apr 03, 2014
Jonathan M Davis
Apr 03, 2014
Jonathan M Davis
Apr 03, 2014
Jonathan M Davis
April 03, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=12507

           Summary: SysTime.init.toString should not segfault
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: Phobos
        AssignedTo: nobody@puremagic.com
        ReportedBy: thecybershadow@gmail.com


--- Comment #0 from Vladimir Panteleev <thecybershadow@gmail.com> 2014-04-03 06:26:34 EEST ---
SysTime.init.toString unconditionally dereferences the timezone field (indirectly - it calls adjTime which does so), which will be null for SysTime.init.

This needlessly complicates debugging - writeln(t) will segfault if t has not
been set.

Question: what should be returned? null? "null" the string literal (like what
printf does for null pointers)? The time sans timezone ("0001-Jan-01
00:00:00")?

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 03, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=12507


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #1 from Jonathan M Davis <jmdavisProg@gmx.com> 2014-04-02 21:55:08 PDT ---
SysTime.init is supposed to be equivalent to SysTime(0, LocalTime()) - which would be 0001-Jan-01T00:00:00. The problem is that the timezone can't be initialized with LocalTime at compile time, so SysTime.init can't have a valid timezone. I believe that this limitation is documented.

The only way to work around it would be to check for the timezone for null on every operation that uses it, which would then incur constant overhead for all SysTime objects just to try and make it so that SysTime.init is valid. I concluded that that wasn't worth it and instead opted for documenting the behavior.

toString is just one of SysTime's functions which is affected by the problem. I suppose that we could add a check specifically for toString and let the operations still segfault for SysTime.init, but that seems rather inconsistent to me. My take on it is generally that you just shouldn't be doing much of anything with SysTime.init except using it as a default-initialized value that gets reassigned before you actually do anything with the object.

I'm not super-enthused with the situation, but as far as I can see, there is no solution that results in SysTime.init being valid without adding extra overhead.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 03, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=12507



--- Comment #2 from Jonathan M Davis <jmdavisProg@gmx.com> 2014-04-02 22:08:36 PDT ---
Actually, on further reflection, I think that it might be possible to make SysTime.init equivalent to SysTime(0, UTC()) and have it have a valid timezone. Around dconf last year, someone made it so that immutable classes could be created at compile time and continue on to runtime - so if a struct's member is an immutable class, it can be initialized at compile time. At the time, I tried to make that work with SysTime but ran into two problems:

1. SysTime uses a Rebindable!(immutable TimeZone), and the Rebindable couldn't
be assigned at compile time (a limitation with unions during CTFE IIRC).

2. Because LocalTime needs to call tzset when it's initialized, even if the issue with Rebindable was fixed, SysTime.init's timezone field couldn't be initialized with LocalTime.

However, UTC doesn't have that problem. So, if the issue with Rebindable has been fixed (IIRC, I opened a bug on it, but I'd have to track it down), then we could make SysTime.init use UTC. That's not quite idea IMHO, since SysTime normally defaults to using LocalTime as its timezone, but it _would_ allow us to  make it so that SysTime.init had a valid timezone without adding runtime checks, and since SysTime.init is currently invalid anyway, it wouldn't break any code.

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
April 03, 2014
https://d.puremagic.com/issues/show_bug.cgi?id=12507



--- Comment #3 from Jonathan M Davis <jmdavisProg@gmx.com> 2014-04-02 23:01:59 PDT ---
https://github.com/D-Programming-Language/phobos/pull/2066

-- 
Configure issuemail: https://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------