[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

github-bugzi...@puremagic.com changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

--

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

--- Comment #12 from github-bugzi...@puremagic.com ---
Commits pushed to master at https://github.com/dlang/phobos

https://github.com/dlang/phobos/commit/9b1f72d89b6db38a05b958baf2195f552fe0aa57
Fix issue 12507: SysTime.init.toString() segfaults.

This adds a special TimeZone type internal to SysTime which _timezone is
directly initialized to so that SysTime.init will work without
segfaulting but will still be uniquely identifiable with the is
operator. Or at least, _timezone is _supposed_ to be directly
initialized to it, but issue# 17740 currently prevents that. So,
_timezone has been temporarily renamed to _timezoneStorage and private
getters and setters named _timezone have been added. The getter then
does a null check and returns InitTimeZone() for SysTime.init to
simulate the member variable having been initialized to InitTimeZone().
Once issue# 17740 has been fixed, these accessors will be unnecessary,
and the code should be updated so that _timezone is properly a variable
again and is directly initialized with InitTimeZone().

The new TimeZone type - InitTimeZone - is internal to SysTime and can
only be obtained by the timezone getter on SysTime.init. It acts the
same as UTC except that it is not special-cased by the to*String
functions and thus will print out its timezone as +00:00 instead of z,
which is perfectly legitimate per the spec. And as such, if _timezone
were directly initialized with InitTimeZone(), there would be no extra
checks due to this change, and everything would just work. However,
until issue# 17740 is fixed, there will be an extra null check any time
that a function is called on _timezone, because _timezone is currently a
wrapper that does a null check rather than being a member variable
directly like it's supposed to be.

Unlike previous attempts along these lines, this does not make it so
that SysTime.init has NaN behavior such that any operation (other than
assignment) on an an uninitialized SysTime would result in SysTime.init,
and the timezone setter property does not set the SysTime to
SysTime.init if it's passed this TimeZone. So, unfortunately, it _is_
possible to have other SysTime values with the special TimeZone, but it
was deemed unnecessarily complex for too little benefit to add the NaN
behavior. And regardless, SysTime.init is still uniquely identifiable
via the is operator. It's just that it can't technically be uniquely
identified by the timezone getter, which was never a supported feature
anyway.

https://github.com/dlang/phobos/commit/ec8bb20d28fa2ff9eeac56a00edc9d916638e61c
Merge pull request #6591 from jmdavis/issue_17732_again

Fix issue 12507: SysTime.init.toString() segfaults.
merged-on-behalf-of: Petar Kirov 

--

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

Nathan S.  changed:

   What|Removed |Added

 CC||n8sh.second...@hotmail.com

--- Comment #11 from Nathan S.  ---
Pull request: https://github.com/dlang/phobos/pull/6581

I doubt it is of great interest to anyone precisely what is displayed by
`SysTime.init.toString`, but it shouldn't cause a segfault, and fixing that is
trivial.

--

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

--- Comment #10 from Rob T  ---
I just encounter this issue and spent an hour or so figuring out what the cause
was. segfaults tend to be very difficult to track down and deal with. 

A quick sub-optimal fix will be better than leaving it to seg fault.

--

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail--- via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

Rob T  changed:

   What|Removed |Added

 CC||al...@ucora.com

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

Jonathan M Davis  changed:

   What|Removed |Added

 CC||c...@dawg.eu

--- Comment #9 from Jonathan M Davis  ---
*** Issue 17732 has been marked as a duplicate of this issue. ***

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

--- Comment #8 from Jonathan M Davis  ---
(In reply to MichaelZ from comment #7)
> Since issue# 12624 has apparently been fixed, how are things looking for
> this issue?

I need to find time to dig out my previous fix and make sure that it's correct
and functional, but I haven't had a lot of time for D lately and other issues
have been higher priority. I expect to get to it relatively soon though.

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

dlang@bregalad.de changed:

   What|Removed |Added

 CC||dlang@bregalad.de

--- Comment #7 from dlang@bregalad.de ---
(In reply to Jonathan M Davis from comment #6)
> (In reply to Alaksiej Stankievič from comment #5)
> > What is currently status of this?
> > 
> > I have hit this just now, and it consumes 1 hour of investigation.
> 
> As the "Depends on" field indicates, it's blocked by issue# 12624.
> Attempting to provide a default TimeZone to SysTime won't compile on Windows
> due to a bug in the compiler backend.

Since issue# 12624 has apparently been fixed, how are things looking for this
issue?

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507
Issue 12507 depends on issue 12624, which changed state.

Issue 12624 Summary: [REG 2.064] Internal error: backend\cgobj.c 2313 with 
Rebindable!(immutable TimeZone) in std.datetime
https://issues.dlang.org/show_bug.cgi?id=12624

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |WORKSFORME

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

--- Comment #6 from Jonathan M Davis  ---
(In reply to Alaksiej Stankievič from comment #5)
> What is currently status of this?
> 
> I have hit this just now, and it consumes 1 hour of investigation.

As the "Depends on" field indicates, it's blocked by issue# 12624. Attempting
to provide a default TimeZone to SysTime won't compile on Windows due to a bug
in the compiler backend.

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

Alaksiej Stankievič  changed:

   What|Removed |Added

 CC||harald_zea...@tut.by

--- Comment #5 from Alaksiej Stankievič  ---
What is currently status of this?

I have hit this just now, and it consumes 1 hour of investigation.

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

David Monagle da...@monagle.com.au changed:

   What|Removed |Added

 CC||da...@monagle.com.au

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

Jonathan M Davis jmdavisp...@gmx.com changed:

   What|Removed |Added

 Depends on||12624

--

[Issue 12507] SysTime.init.toString should not segfault

[via Digitalmars-d-bugs]

https://issues.dlang.org/show_bug.cgi?id=12507

--- Comment #4 from Jonathan M Davis jmdavisp...@gmx.com ---
Attempt #2: https://github.com/D-Programming-Language/phobos/pull/2088

--

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=12507


Jonathan M Davis jmdavisp...@gmx.com changed:

   What|Removed |Added

 CC||jmdavisp...@gmx.com


--- Comment #1 from Jonathan M Davis jmdavisp...@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: ---

[Issue 12507] SysTime.init.toString should not segfault

[d-bugmail]

https://d.puremagic.com/issues/show_bug.cgi?id=12507



--- Comment #2 from Jonathan M Davis jmdavisp...@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: ---