[Rd] Possible ALTREP bug

2021-05-28 Thread Gábor Csárdi
I have found some weird SEXP corruption behavior with ALTREP, which
could be a bug. (Or I could be doing something wrong.)

I have an integer ALTREP vector that calls back to R from the Elt
method. When this vector is indexed in a lapply(), its first element
gets corrupted. Sometimes it's just a type change to logical, but
sometimes the corruption causes a crash.

I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
that demonstrates this: https://github.com/gaborcsardi/redfish

The R callback in this package calls `loadNamespace("Matrix")`, but
the same crash happens for other packages as well, and sometimes it
also happens if I don't load any packages at all. (But that example
was much more complicated, so I went with the package loading.)

It is somewhat random, and sometimes turning off the JIT avoids the
crash, but not always.

Hopefully I am just doing something wrong in the ALTREP code (see
https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
is not actually a bug.

Thanks,
Gabor

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] [External] Possible ALTREP bug

2021-05-28 Thread luke-tierney

integer and real Elt methods are not expected to allocate. You would
have to suspend GC to be able to do that. This currently can't be done
from package code.

Best,

luke

On Fri, 28 May 2021, Gábor Csárdi wrote:


I have found some weird SEXP corruption behavior with ALTREP, which
could be a bug. (Or I could be doing something wrong.)

I have an integer ALTREP vector that calls back to R from the Elt
method. When this vector is indexed in a lapply(), its first element
gets corrupted. Sometimes it's just a type change to logical, but
sometimes the corruption causes a crash.

I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
that demonstrates this: https://github.com/gaborcsardi/redfish

The R callback in this package calls `loadNamespace("Matrix")`, but
the same crash happens for other packages as well, and sometimes it
also happens if I don't load any packages at all. (But that example
was much more complicated, so I went with the package loading.)

It is somewhat random, and sometimes turning off the JIT avoids the
crash, but not always.

Hopefully I am just doing something wrong in the ALTREP code (see
https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
is not actually a bug.

Thanks,
Gabor

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel



--
Luke Tierney
Ralph E. Wareham Professor of Mathematical Sciences
University of Iowa  Phone: 319-335-3386
Department of Statistics andFax:   319-335-3017
   Actuarial Science
241 Schaeffer Hall  email:   luke-tier...@uiowa.edu
Iowa City, IA 52242 WWW:  http://www.stat.uiowa.edu
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] Possible ALTREP bug

2021-05-28 Thread Jiefei Wang
Hi Gabor,

Calling back to an R function from the ALTREP function is not safe.
There has been a heated discussion on why you should not do it and
that the main reason that we do not have any R level ALTREP API. If
you are interested in it you can find it from this issue:
https://github.com/Bioconductor/Contributions/issues/1222

Best,
Jiefei

On Fri, May 28, 2021 at 4:54 PM Gábor Csárdi  wrote:
>
> I have found some weird SEXP corruption behavior with ALTREP, which
> could be a bug. (Or I could be doing something wrong.)
>
> I have an integer ALTREP vector that calls back to R from the Elt
> method. When this vector is indexed in a lapply(), its first element
> gets corrupted. Sometimes it's just a type change to logical, but
> sometimes the corruption causes a crash.
>
> I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
> that demonstrates this: https://github.com/gaborcsardi/redfish
>
> The R callback in this package calls `loadNamespace("Matrix")`, but
> the same crash happens for other packages as well, and sometimes it
> also happens if I don't load any packages at all. (But that example
> was much more complicated, so I went with the package loading.)
>
> It is somewhat random, and sometimes turning off the JIT avoids the
> crash, but not always.
>
> Hopefully I am just doing something wrong in the ALTREP code (see
> https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
> is not actually a bug.
>
> Thanks,
> Gabor
>
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] [External] Possible ALTREP bug

2021-05-28 Thread Gábor Csárdi
Thank you Luke, that makes a lot of sense,
Gabor

On Fri, May 28, 2021 at 3:41 PM  wrote:
>
> integer and real Elt methods are not expected to allocate. You would
> have to suspend GC to be able to do that. This currently can't be done
> from package code.
>
> Best,
>
> luke
>
> On Fri, 28 May 2021, Gábor Csárdi wrote:
>
> > I have found some weird SEXP corruption behavior with ALTREP, which
> > could be a bug. (Or I could be doing something wrong.)
> >
> > I have an integer ALTREP vector that calls back to R from the Elt
> > method. When this vector is indexed in a lapply(), its first element
> > gets corrupted. Sometimes it's just a type change to logical, but
> > sometimes the corruption causes a crash.
> >
> > I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
> > that demonstrates this: https://github.com/gaborcsardi/redfish
> >
> > The R callback in this package calls `loadNamespace("Matrix")`, but
> > the same crash happens for other packages as well, and sometimes it
> > also happens if I don't load any packages at all. (But that example
> > was much more complicated, so I went with the package loading.)
> >
> > It is somewhat random, and sometimes turning off the JIT avoids the
> > crash, but not always.
> >
> > Hopefully I am just doing something wrong in the ALTREP code (see
> > https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
> > is not actually a bug.
> >
> > Thanks,
> > Gabor
> >
> > __
> > R-devel@r-project.org mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
> >
>
> --
> Luke Tierney
> Ralph E. Wareham Professor of Mathematical Sciences
> University of Iowa  Phone: 319-335-3386
> Department of Statistics andFax:   319-335-3017
> Actuarial Science
> 241 Schaeffer Hall  email:   luke-tier...@uiowa.edu
> Iowa City, IA 52242 WWW:  http://www.stat.uiowa.edu

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] [External] Possible ALTREP bug

2021-05-28 Thread Jim Hester
>From reading the discussion on the Bioconductor issue tracker it seems like
the reason the GC is not suspended for the non-string ALTREP Elt methods is
primarily due to performance concerns.

If this is the case perhaps an additional flag could be added to the
`R_set_altrep_*()` functions so ALTREP authors could indicate if GC should
be halted when that particular method is called for that particular ALTREP
class.

This would avoid the performance hit (other than a boolean check) for the
standard case when no allocations are expected, but allow authors to
indicate that R should pause GC if needed for methods in their class.

On Fri, May 28, 2021 at 9:42 AM  wrote:

> integer and real Elt methods are not expected to allocate. You would
> have to suspend GC to be able to do that. This currently can't be done
> from package code.
>
> Best,
>
> luke
>
> On Fri, 28 May 2021, Gábor Csárdi wrote:
>
> > I have found some weird SEXP corruption behavior with ALTREP, which
> > could be a bug. (Or I could be doing something wrong.)
> >
> > I have an integer ALTREP vector that calls back to R from the Elt
> > method. When this vector is indexed in a lapply(), its first element
> > gets corrupted. Sometimes it's just a type change to logical, but
> > sometimes the corruption causes a crash.
> >
> > I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
> > that demonstrates this: https://github.com/gaborcsardi/redfish
> >
> > The R callback in this package calls `loadNamespace("Matrix")`, but
> > the same crash happens for other packages as well, and sometimes it
> > also happens if I don't load any packages at all. (But that example
> > was much more complicated, so I went with the package loading.)
> >
> > It is somewhat random, and sometimes turning off the JIT avoids the
> > crash, but not always.
> >
> > Hopefully I am just doing something wrong in the ALTREP code (see
> > https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
> > is not actually a bug.
> >
> > Thanks,
> > Gabor
> >
> > __
> > R-devel@r-project.org mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
> >
>
> --
> Luke Tierney
> Ralph E. Wareham Professor of Mathematical Sciences
> University of Iowa  Phone: 319-335-3386
> Department of Statistics andFax:   319-335-3017
> Actuarial Science
> 241 Schaeffer Hall  email:   luke-tier...@uiowa.edu
> Iowa City, IA 52242 WWW:  http://www.stat.uiowa.edu
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

[[alternative HTML version deleted]]

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


[Rd] Typo in Writing R Extensions

2021-05-28 Thread Vincent Goulet via R-devel
Hi,

Just noticed this: on line 15296 of the current (master) R-exts.texi (section 7 
of the compiled document), one reads

would do most likely do different things, to the justifiable 

Either one of the "do" is in extra.

Best,

Vincent Goulet
Université Laval
__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] [External] Possible ALTREP bug

2021-05-28 Thread Gabriel Becker
Hi Jim et al,

Just to hopefully add a bit to what Luke already answered, from what I am
recalling looking back at that bioconductor thread Elt methods are used in
places where there are hard implicit assumptions that no garbage collection
will occur (ie they are called on things that aren't PROTECTed), and beyond
that, in places where there are hard assumptions that no error (longjmp)
will occur. I could be wrong, but I don't know that suspending garbage
collection would protect from the second one. Ie it is possible that an
error *ever* being raised from R code that implements an elt method could
cause all hell to break loose.

Luke or Tomas Kalibera would know more.

I was disappointed that implementing ALTREPs in R code was not in the cards
(it was in my original proposal back in 2016 to the DSC) but I trust Luke
that there are important reasons we can't safely allow that.

Best,
~G

On Fri, May 28, 2021 at 8:31 AM Jim Hester  wrote:

> From reading the discussion on the Bioconductor issue tracker it seems like
> the reason the GC is not suspended for the non-string ALTREP Elt methods is
> primarily due to performance concerns.
>
> If this is the case perhaps an additional flag could be added to the
> `R_set_altrep_*()` functions so ALTREP authors could indicate if GC should
> be halted when that particular method is called for that particular ALTREP
> class.
>
> This would avoid the performance hit (other than a boolean check) for the
> standard case when no allocations are expected, but allow authors to
> indicate that R should pause GC if needed for methods in their class.
>
> On Fri, May 28, 2021 at 9:42 AM  wrote:
>
> > integer and real Elt methods are not expected to allocate. You would
> > have to suspend GC to be able to do that. This currently can't be done
> > from package code.
> >
> > Best,
> >
> > luke
> >
> > On Fri, 28 May 2021, Gábor Csárdi wrote:
> >
> > > I have found some weird SEXP corruption behavior with ALTREP, which
> > > could be a bug. (Or I could be doing something wrong.)
> > >
> > > I have an integer ALTREP vector that calls back to R from the Elt
> > > method. When this vector is indexed in a lapply(), its first element
> > > gets corrupted. Sometimes it's just a type change to logical, but
> > > sometimes the corruption causes a crash.
> > >
> > > I saw this on macOS from R 3.5.3 to 4.2.0. I created a small package
> > > that demonstrates this: https://github.com/gaborcsardi/redfish
> > >
> > > The R callback in this package calls `loadNamespace("Matrix")`, but
> > > the same crash happens for other packages as well, and sometimes it
> > > also happens if I don't load any packages at all. (But that example
> > > was much more complicated, so I went with the package loading.)
> > >
> > > It is somewhat random, and sometimes turning off the JIT avoids the
> > > crash, but not always.
> > >
> > > Hopefully I am just doing something wrong in the ALTREP code (see
> > > https://github.com/gaborcsardi/redfish/blob/main/src/test.c), and it
> > > is not actually a bug.
> > >
> > > Thanks,
> > > Gabor
> > >
> > > __
> > > R-devel@r-project.org mailing list
> > > https://stat.ethz.ch/mailman/listinfo/r-devel
> > >
> >
> > --
> > Luke Tierney
> > Ralph E. Wareham Professor of Mathematical Sciences
> > University of Iowa  Phone: 319-335-3386
> > Department of Statistics andFax:   319-335-3017
> > Actuarial Science
> > 241 Schaeffer Hall  email:   luke-tier...@uiowa.edu
> > Iowa City, IA 52242 WWW:  http://www.stat.uiowa.edu
> > __
> > R-devel@r-project.org mailing list
> > https://stat.ethz.ch/mailman/listinfo/r-devel
> >
>
> [[alternative HTML version deleted]]
>
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

[[alternative HTML version deleted]]

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] [External] Possible ALTREP bug

2021-05-28 Thread luke-tierney

Since the INTEGER_ELT, REAL_ELT, etc, functions are fairly new it may
be possible to check that places where they are used allow for them to
allocate. I have fixed the one that got caught by Gabor's example, and
a rchk run might be able to pick up others if rchk knows these could
allocate. (I may also be forgetting other places where the _ELt
methods are used.)  Fixing all call sites for REAL, INTEGER, etc, was
never realistic so there GC has to be suspended during the method
call, and that is done in the dispatch mechanism.

The bigger problem is jumps from inside things that existing code
assumes will not do that. Catching those jumps is possible but
expensive; doing anything sensible if one is caught is really not
possible.

Best,

luke

On Fri, 28 May 2021, Gabriel Becker wrote:


Hi Jim et al,
Just to hopefully add a bit to what Luke already answered, from what I am
recalling looking back at that bioconductor thread Elt methods are used in
places where there are hard implicit assumptions that no garbage collection
will occur (ie they are called on things that aren't PROTECTed), and beyond
that, in places where there are hard assumptions that no error (longjmp)
will occur. I could be wrong, but I don't know that suspending garbage
collection would protect from the second one. Ie it is possible that an
error *ever* being raised from R code that implements an elt method could
cause all hell to break loose.

Luke or Tomas Kalibera would know more.

I was disappointed that implementing ALTREPs in R code was not in the cards
(it was in my original proposal back in 2016 to the DSC) but I trust Luke
that there are important reasons we can't safely allow that.

Best,
~G

On Fri, May 28, 2021 at 8:31 AM Jim Hester  wrote:
  From reading the discussion on the Bioconductor issue tracker it
  seems like
  the reason the GC is not suspended for the non-string ALTREP Elt
  methods is
  primarily due to performance concerns.

  If this is the case perhaps an additional flag could be added to
  the
  `R_set_altrep_*()` functions so ALTREP authors could indicate if
  GC should
  be halted when that particular method is called for that
  particular ALTREP
  class.

  This would avoid the performance hit (other than a boolean
  check) for the
  standard case when no allocations are expected, but allow
  authors to
  indicate that R should pause GC if needed for methods in their
  class.

  On Fri, May 28, 2021 at 9:42 AM  wrote:

  > integer and real Elt methods are not expected to allocate. You
  would
  > have to suspend GC to be able to do that. This currently can't
  be done
  > from package code.
  >
  > Best,
  >
  > luke
  >
  > On Fri, 28 May 2021, Gábor Csárdi wrote:
  >
  > > I have found some weird SEXP corruption behavior with
  ALTREP, which
  > > could be a bug. (Or I could be doing something wrong.)
  > >
  > > I have an integer ALTREP vector that calls back to R from
  the Elt
  > > method. When this vector is indexed in a lapply(), its first
  element
  > > gets corrupted. Sometimes it's just a type change to
  logical, but
  > > sometimes the corruption causes a crash.
  > >
  > > I saw this on macOS from R 3.5.3 to 4.2.0. I created a small
  package
  > > that demonstrates this:
  https://github.com/gaborcsardi/redfish
  > >
  > > The R callback in this package calls
  `loadNamespace("Matrix")`, but
  > > the same crash happens for other packages as well, and
  sometimes it
  > > also happens if I don't load any packages at all. (But that
  example
  > > was much more complicated, so I went with the package
  loading.)
  > >
  > > It is somewhat random, and sometimes turning off the JIT
  avoids the
  > > crash, but not always.
  > >
  > > Hopefully I am just doing something wrong in the ALTREP code
  (see
  > >
  https://github.com/gaborcsardi/redfish/blob/main/src/test.c),
  and it
  > > is not actually a bug.
  > >
  > > Thanks,
  > > Gabor
  > >
  > > __
  > > R-devel@r-project.org mailing list
  > > https://stat.ethz.ch/mailman/listinfo/r-devel
  > >
  >
  > --
  > Luke Tierney
  > Ralph E. Wareham Professor of Mathematical Sciences
  > University of Iowa                  Phone:           
   319-335-3386
  > Department of Statistics and        Fax:             
   319-335-3017
  >     Actuarial Science
  > 241 Schaeffer Hall                  email: 
   luke-tier...@uiowa.edu
  > Iowa City, IA 52242                 WWW: 
  http://www.stat.uiowa.edu
  > __
  > R-devel@r-project.org mailing list
  > https://stat.ethz.ch/mailman/listinfo/r-devel
  >