segfault with POSIXlt zone=NULL zone=""

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

segfault with POSIXlt zone=NULL zone=""

frederik-2
Hi all,

I ran into a segfault while playing with dates.

    $ R --no-init-file
    ...
    > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d

    Attaching package: ‘lubridate’

    The following object is masked from ‘package:base’:

        date

    Warning message:
    package ‘lubridate’ was built under R version 3.4.0

     *** caught segfault ***
    address (nil), cause 'unknown'

    Traceback:
     1: format.POSIXlt(x, usetz = TRUE)
     2: format(x, usetz = TRUE)
     3: print(format(x, usetz = TRUE), ...)
     4: print.POSIXlt(x)
     5: function (x, ...) UseMethod("print")(x)

    Possible actions:
    ...

Hope I'm not doing something illegal...

Thanks,

Frederick

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: segfault with POSIXlt zone=NULL zone=""

Joshua Ulrich
On Tue, Dec 6, 2016 at 6:37 AM,  <[hidden email]> wrote:
> Hi all,
>
> I ran into a segfault while playing with dates.
>
>     $ R --no-init-file
>     ...
>     > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
>
If you're asking about a bug in R, you should provide a *minimal*
reproducible example (i.e. one without any package dependencies).
This has nothing to do with lubridate, so you can reproduce the
behavior with:

d <- as.POSIXlt(Sys.time())
d$zone <- NULL
d$zone <- ""
d

>     Attaching package: ‘lubridate’
>
>     The following object is masked from ‘package:base’:
>
>         date
>
>     Warning message:
>     package ‘lubridate’ was built under R version 3.4.0
>
>      *** caught segfault ***
>     address (nil), cause 'unknown'
>
>     Traceback:
>      1: format.POSIXlt(x, usetz = TRUE)
>      2: format(x, usetz = TRUE)
>      3: print(format(x, usetz = TRUE), ...)
>      4: print.POSIXlt(x)
>      5: function (x, ...) UseMethod("print")(x)
>
>     Possible actions:
>     ...
>
> Hope I'm not doing something illegal...
>
You are.  You're changing the internal structure of a POSIXlt object
by re-ordering the list elements.  You should not expect a malformed
POSIXlt object to behave as if it's correctly formed.  You can see
it's malformed by comparing it's unclass()'d output.

d <- as.POSIXlt(Sys.time())
unclass(d)  # valid POSIXlt object
d$zone <- NULL
d$zone <- ""
unclass(d)  # your malformed POSIXlt object

> Thanks,
>
> Frederick
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel



--
Joshua Ulrich  |  about.me/joshuaulrich
FOSS Trading  |  www.fosstrading.com
R/Finance 2016 | www.rinfinance.com

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: segfault with POSIXlt zone=NULL zone=""

frederik-2
Hi Joshua,

Thank you for minimizing my test case.

> > Hope I'm not doing something illegal...
> >
> You are.  You're changing the internal structure of a POSIXlt object
> by re-ordering the list elements.  You should not expect a malformed
> POSIXlt object to behave as if it's correctly formed.  You can see
> it's malformed by comparing it's unclass()'d output.
>
> d <- as.POSIXlt(Sys.time())
> unclass(d)  # valid POSIXlt object
> d$zone <- NULL
> d$zone <- ""
> unclass(d)  # your malformed POSIXlt object

I don't know if these questions are not already obvious, but:

1. Is there a reasonable way to fail more elegantly when a user makes
this mistake?

2. Should we update the documentation for POSIXlt to warn people that
the optional "zone" list element must precede the optional "gmtoff"
list element, in cases where both are present?

Thanks,

Frederick

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: segfault with POSIXlt zone=NULL zone=""

Joshua Ulrich
On Tue, Dec 6, 2016 at 10:30 AM,  <[hidden email]> wrote:

> Hi Joshua,
>
> Thank you for minimizing my test case.
>
>> > Hope I'm not doing something illegal...
>> >
>> You are.  You're changing the internal structure of a POSIXlt object
>> by re-ordering the list elements.  You should not expect a malformed
>> POSIXlt object to behave as if it's correctly formed.  You can see
>> it's malformed by comparing it's unclass()'d output.
>>
>> d <- as.POSIXlt(Sys.time())
>> unclass(d)  # valid POSIXlt object
>> d$zone <- NULL
>> d$zone <- ""
>> unclass(d)  # your malformed POSIXlt object
>
> I don't know if these questions are not already obvious, but:
>
> 1. Is there a reasonable way to fail more elegantly when a user makes
> this mistake?
>
It's not just "this mistake".  See below.

> 2. Should we update the documentation for POSIXlt to warn people that
> the optional "zone" list element must precede the optional "gmtoff"
> list element, in cases where both are present?
>
No, because that's not the only way to create a malformed POSIXlt
object. Reordering *any* of the elements results in a segfault, and
there are probably other things you could do to the internal structure
of POSIXlt objects to cause segfaults.

Maybe update the documentation to say, "If you update the internal
structure of a POSIXlt object, you deserve whatever happens."? ;-)

> Thanks,
>
> Frederick



--
Joshua Ulrich  |  about.me/joshuaulrich
FOSS Trading  |  www.fosstrading.com
R/Finance 2016 | www.rinfinance.com

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

ok to segfault with POSIXlt zone=NULL zone=""?

frederik-2
In reply to this post by frederik-2
Hi all,

Here's a more minimal version of my earlier bug report (thanks, Joshua
Ulrich):

d=as.POSIXlt(Sys.time()); d$zone=NULL; d$zone=""; d

I got some helpful, if glib, feedback from Joshua that the segfault
may be caused by the changing of the order of the list elements in 'd'
(representing the "internal structure" of the POSIXlt object).

He seems to think that it's OK for R to segfault - I was wondering if
someone else could lend a second opinion. My understanding is that we
should try to avoid segfaulting as a way of handling errors, if only
because they become much more difficult to debug when the R session is
forced to quit.

I don't know exactly which line is causing the bug, but looking at the
code for do_formatPOSIXlt in "src/main/datetime.c", it seems that
there would not be a huge performance penalty to add an extra sanity
check to prevent this from occurring.

Thank you,

Frederick

On Tue, Dec 06, 2016 at 04:37:20AM -0800, [hidden email] wrote:

> Hi all,
>
> I ran into a segfault while playing with dates.
>
>     $ R --no-init-file
>     ...
>     > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
>
>     Attaching package: ‘lubridate’
>
>     The following object is masked from ‘package:base’:
>
>         date
>
>     Warning message:
>     package ‘lubridate’ was built under R version 3.4.0
>
>      *** caught segfault ***
>     address (nil), cause 'unknown'
>
>     Traceback:
>      1: format.POSIXlt(x, usetz = TRUE)
>      2: format(x, usetz = TRUE)
>      3: print(format(x, usetz = TRUE), ...)
>      4: print.POSIXlt(x)
>      5: function (x, ...) UseMethod("print")(x)
>
>     Possible actions:
>     ...
>
> Hope I'm not doing something illegal...
>
> Thanks,
>
> Frederick
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: ok to segfault with POSIXlt zone=NULL zone=""?

Duncan Murdoch-2
I agree this is a bug; R should never segfault.  I wouldn't call it a
high priority one, since you can avoid the problem by not messing with
R's internal structures.

It's unlikely to get fixed unless someone posts it as a bug report to
bugs.r-project.org (because low priority bugs reported only on mailing
lists get forgotten).

So please post a minimal example there, possibly accompanied with a
patch.  If you don't have an account, you can write to me privately and
I'll set one up for you.  (We no longer allow people to create their own
accounts because of abuse by spammers.)

Duncan Murdoch

On 06/12/2016 12:27 PM, [hidden email] wrote:

> Hi all,
>
> Here's a more minimal version of my earlier bug report (thanks, Joshua
> Ulrich):
>
> d=as.POSIXlt(Sys.time()); d$zone=NULL; d$zone=""; d
>
> I got some helpful, if glib, feedback from Joshua that the segfault
> may be caused by the changing of the order of the list elements in 'd'
> (representing the "internal structure" of the POSIXlt object).
>
> He seems to think that it's OK for R to segfault - I was wondering if
> someone else could lend a second opinion. My understanding is that we
> should try to avoid segfaulting as a way of handling errors, if only
> because they become much more difficult to debug when the R session is
> forced to quit.
>
> I don't know exactly which line is causing the bug, but looking at the
> code for do_formatPOSIXlt in "src/main/datetime.c", it seems that
> there would not be a huge performance penalty to add an extra sanity
> check to prevent this from occurring.
>
> Thank you,
>
> Frederick
>
> On Tue, Dec 06, 2016 at 04:37:20AM -0800, [hidden email] wrote:
>> Hi all,
>>
>> I ran into a segfault while playing with dates.
>>
>>     $ R --no-init-file
>>     ...
>>     > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
>>
>>     Attaching package: ‘lubridate’
>>
>>     The following object is masked from ‘package:base’:
>>
>>         date
>>
>>     Warning message:
>>     package ‘lubridate’ was built under R version 3.4.0
>>
>>      *** caught segfault ***
>>     address (nil), cause 'unknown'
>>
>>     Traceback:
>>      1: format.POSIXlt(x, usetz = TRUE)
>>      2: format(x, usetz = TRUE)
>>      3: print(format(x, usetz = TRUE), ...)
>>      4: print.POSIXlt(x)
>>      5: function (x, ...) UseMethod("print")(x)
>>
>>     Possible actions:
>>     ...
>>
>> Hope I'm not doing something illegal...
>>
>> Thanks,
>>
>> Frederick
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: segfault with POSIXlt zone=NULL zone=""

Martin Maechler
In reply to this post by Joshua Ulrich
>>>>> Joshua Ulrich <[hidden email]>
>>>>>     on Tue, 6 Dec 2016 09:51:16 -0600 writes:

    > On Tue, Dec 6, 2016 at 6:37 AM,  <[hidden email]> wrote:
    >> Hi all,
    >>
    >> I ran into a segfault while playing with dates.
    >>
    >> $ R --no-init-file
    >> ...
    >> > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
    >>
    > If you're asking about a bug in R, you should provide a *minimal*
    > reproducible example (i.e. one without any package dependencies).
    > This has nothing to do with lubridate, so you can reproduce the
    > behavior with:

    > d <- as.POSIXlt(Sys.time())
    > d$zone <- NULL
    > d$zone <- ""
    > d

    [..........]
   
    >> Hope I'm not doing something illegal...
    >>
    > You are.  You're changing the internal structure of a POSIXlt object
    > by re-ordering the list elements.  You should not expect a malformed
    > POSIXlt object to behave as if it's correctly formed.  You can see
    > it's malformed by comparing it's unclass()'d output.

    > d <- as.POSIXlt(Sys.time())
    > unclass(d)  # valid POSIXlt object
    > d$zone <- NULL
    > d$zone <- ""
    > unclass(d)  # your malformed POSIXlt object

Indeed, really illegal, i.e. "against the law" ... ;-)

Thank you, Joshua!

Still, if R segfaults without the user explicitly
calling .Call(), .Internal()  or similar -- as here --
we usually acknowledge there *is* a bug in R .. even if it is
only triggered by a users "illegal" messing around.

an MRE for the above, where I really only re-order the "internal" list:

d <- as.POSIXlt("2016-12-06"); dz <- d$zone; d$zone <- NULL; d$zone <- dz; f <- format(d)

>  *** caught segfault ***
> address 0x80000020, cause 'memory not mapped'

> Traceback:
>  1: format.POSIXlt(d)
>  2: format(d)

The current code is "optimized for speed" (not perfectly), and
a patch should hopefully address the C code.

Note that a smaller MRE -- which does *not* re-order, but just
invalidate the time zone is

  d <- as.POSIXlt("2016-12-06"); d$zone <- 1; f <- format(d)

------

I have now committed a "minimal" patch (to the C code) which for
the above two cases gives a sensible error rather than a
seg.fault :

  > d <- as.POSIXlt("2016-12-06"); d$zone <- 1 ; f <- format(d)
  Error in format.POSIXlt(d) :
    invalid 'zone' component in "POSIXlt" structure

  > d <- as.POSIXlt("2016-12-06"); dz <- d$zone; d$zone <- NULL; d$zone <- dz; f <- format(d)
  Error in format.POSIXlt(d) :
    invalid 'zone' component in "POSIXlt" structure
  >

I guess that it should still be possible to produce a segfault
with invalid 'POSIXlt' structures though.

Martin

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: ok to segfault with POSIXlt zone=NULL zone=""?

Spencer Graves-3
In reply to this post by frederik-2
I got a similar result from R-Studio 1.0.44 with

 > sessionInfo()
R version 3.3.2 (2016-10-31)
Platform: x86_64-apple-darwin13.4.0 (64-bit)
Running under: OS X El Capitan 10.11.6

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats     graphics  grDevices utils
[5] datasets  methods   base

loaded via a namespace (and not attached):
[1] tools_3.3.2

 > d <- as.POSIXlt(Sys.time())
 > d$zone <- NULL
 > d$zone <- ""

:  "R Session Aborted.  R encountered a fatal error.  The session was
terminated.  Start New Session".


       I got essentially the same result with R 3.3.1.  Then I installed
3.3.2 and got the above.


       Spencer Graves


On 12/6/2016 11:27 AM, [hidden email] wrote:

> Hi all,
>
> Here's a more minimal version of my earlier bug report (thanks, Joshua
> Ulrich):
>
> d=as.POSIXlt(Sys.time()); d$zone=NULL; d$zone=""; d
>
> I got some helpful, if glib, feedback from Joshua that the segfault
> may be caused by the changing of the order of the list elements in 'd'
> (representing the "internal structure" of the POSIXlt object).
>
> He seems to think that it's OK for R to segfault - I was wondering if
> someone else could lend a second opinion. My understanding is that we
> should try to avoid segfaulting as a way of handling errors, if only
> because they become much more difficult to debug when the R session is
> forced to quit.
>
> I don't know exactly which line is causing the bug, but looking at the
> code for do_formatPOSIXlt in "src/main/datetime.c", it seems that
> there would not be a huge performance penalty to add an extra sanity
> check to prevent this from occurring.
>
> Thank you,
>
> Frederick
>
> On Tue, Dec 06, 2016 at 04:37:20AM -0800, [hidden email] wrote:
>> Hi all,
>>
>> I ran into a segfault while playing with dates.
>>
>>      $ R --no-init-file
>>      ...
>>      > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
>>
>>      Attaching package: ‘lubridate’
>>
>>      The following object is masked from ‘package:base’:
>>
>>          date
>>
>>      Warning message:
>>      package ‘lubridate’ was built under R version 3.4.0
>>
>>       *** caught segfault ***
>>      address (nil), cause 'unknown'
>>
>>      Traceback:
>>       1: format.POSIXlt(x, usetz = TRUE)
>>       2: format(x, usetz = TRUE)
>>       3: print(format(x, usetz = TRUE), ...)
>>       4: print.POSIXlt(x)
>>       5: function (x, ...) UseMethod("print")(x)
>>
>>      Possible actions:
>>      ...
>>
>> Hope I'm not doing something illegal...
>>
>> Thanks,
>>
>> Frederick
>>
>> ______________________________________________
>> [hidden email] mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
> ______________________________________________
> [hidden email] mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel
Reply | Threaded
Open this post in threaded view
|

Re: segfault with POSIXlt zone=NULL zone=""

frederik-2
In reply to this post by Duncan Murdoch-2
Thanks for the reply, Duncan.

It looks like Martin can commit a patch faster than I can open a bug
report...

Frederick

On Tue, Dec 06, 2016 at 12:39:32PM -0500, Duncan Murdoch wrote:

> I agree this is a bug; R should never segfault.  I wouldn't call it a high
> priority one, since you can avoid the problem by not messing with R's
> internal structures.
>
> It's unlikely to get fixed unless someone posts it as a bug report to
> bugs.r-project.org (because low priority bugs reported only on mailing lists
> get forgotten).
>
> So please post a minimal example there, possibly accompanied with a patch.
> If you don't have an account, you can write to me privately and I'll set one
> up for you.  (We no longer allow people to create their own accounts because
> of abuse by spammers.)
>
> Duncan Murdoch

On Tue, Dec 06, 2016 at 06:49:05PM +0100, Martin Maechler wrote:

> >>>>> Joshua Ulrich <[hidden email]>
> >>>>>     on Tue, 6 Dec 2016 09:51:16 -0600 writes:
>
>     > On Tue, Dec 6, 2016 at 6:37 AM,  <[hidden email]> wrote:
>     >> Hi all,
>     >>
>     >> I ran into a segfault while playing with dates.
>     >>
>     >> $ R --no-init-file
>     >> ...
>     >> > library(lubridate); d=as.POSIXlt(floor_date(Sys.time(),"year")); d$zone=NULL; d$zone=""; d
>     >>
>     > If you're asking about a bug in R, you should provide a *minimal*
>     > reproducible example (i.e. one without any package dependencies).
>     > This has nothing to do with lubridate, so you can reproduce the
>     > behavior with:
>
>     > d <- as.POSIXlt(Sys.time())
>     > d$zone <- NULL
>     > d$zone <- ""
>     > d
>
>     [..........]
>    
>     >> Hope I'm not doing something illegal...
>     >>
>     > You are.  You're changing the internal structure of a POSIXlt object
>     > by re-ordering the list elements.  You should not expect a malformed
>     > POSIXlt object to behave as if it's correctly formed.  You can see
>     > it's malformed by comparing it's unclass()'d output.
>
>     > d <- as.POSIXlt(Sys.time())
>     > unclass(d)  # valid POSIXlt object
>     > d$zone <- NULL
>     > d$zone <- ""
>     > unclass(d)  # your malformed POSIXlt object
>
> Indeed, really illegal, i.e. "against the law" ... ;-)
>
> Thank you, Joshua!
>
> Still, if R segfaults without the user explicitly
> calling .Call(), .Internal()  or similar -- as here --
> we usually acknowledge there *is* a bug in R .. even if it is
> only triggered by a users "illegal" messing around.
>
> an MRE for the above, where I really only re-order the "internal" list:
>
> d <- as.POSIXlt("2016-12-06"); dz <- d$zone; d$zone <- NULL; d$zone <- dz; f <- format(d)
>
> >  *** caught segfault ***
> > address 0x80000020, cause 'memory not mapped'
>
> > Traceback:
> >  1: format.POSIXlt(d)
> >  2: format(d)
>
> The current code is "optimized for speed" (not perfectly), and
> a patch should hopefully address the C code.
>
> Note that a smaller MRE -- which does *not* re-order, but just
> invalidate the time zone is
>
>   d <- as.POSIXlt("2016-12-06"); d$zone <- 1; f <- format(d)
>
> ------
>
> I have now committed a "minimal" patch (to the C code) which for
> the above two cases gives a sensible error rather than a
> seg.fault :
>
>   > d <- as.POSIXlt("2016-12-06"); d$zone <- 1 ; f <- format(d)
>   Error in format.POSIXlt(d) :
>     invalid 'zone' component in "POSIXlt" structure
>
>   > d <- as.POSIXlt("2016-12-06"); dz <- d$zone; d$zone <- NULL; d$zone <- dz; f <- format(d)
>   Error in format.POSIXlt(d) :
>     invalid 'zone' component in "POSIXlt" structure
>   >
>
> I guess that it should still be possible to produce a segfault
> with invalid 'POSIXlt' structures though.
>
> Martin
>

______________________________________________
[hidden email] mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel