[Rd] another fix for R crashes under enable-strict-barrier, lto, trunk at 72156

[luke-tierney at uiowa.edu]

I already fixed the particular upstream issue in main.c. I agree that
at least with the barrier check mmuch more null checking would be nice
to have; I put it on my TODO list but won't get there for a while --
if someone else has time earlier go ahead.

Best,

luke

On Mon, 20 Feb 2017, Hin-Tak Leung wrote:

> On 2nd thought, I think a better fix to the segfault is something like this:
>
> --- a/src/main/memory.c
> +++ b/src/main/memory.c
> @@ -3444,6 +3444,8 @@ R_xlen_t (XTRUELENGTH)(SEXP x) { return XTRUELENGTH(CHK2(x)); }
> int  (IS_LONG_VEC)(SEXP x) { return IS_LONG_VEC(CHK2(x)); }
> 
> const char *(R_CHAR)(SEXP x) {
> +    if(!x)
> +       error("de-referncing null. Check the validity of your data.");
>     if(TYPEOF(x) != CHARSXP)
>        error("%s() can only be applied to a '%s', not a '%s'",
>              "CHAR", "CHARSXP", type2char(TYPEOF(x)));
>
>
> The segfault happens in the middle of tests/no-segfault.R . I have since built R 3.2.x and 3.3.x with --enable-strict-barrier so this is probably new to R 3.4. I think 
> tests/no-segfault.R is supposed to try to trigger a segfault with invalid data, and it is supposed to be caught. That it isn't caught with some combinations of configure is obviously broken; OTOH, testing for segfault with invalid data is also intentional; so I think a better solution is to be verbose about it, instead of what I suggested earlier, silently letting the invalid data through and let upstream cope.
>
> I had a stack trace - but it wasn't obvious where-else a check should be made. The segfault happens is in the eval loop, which is fairly general by itself.
>
> In any case, that was the whole point of me having   --enable-memory-profiling --enable-strict-barrier --with-valgrind-instrumentation=2 : I work(ed) with people who like to write buggy code. Invalid input data and invalid stuff somewhere in the middle is expected, from these people... 
>
> To be honest, I think a few more null checks and a few more tests in tests/no-segfault.R could be added. 
>
> --------------------------------------------
> On Mon, 2/20/17, Martin Maechler <maechler at stat.math.ethz.ch> wrote:
>
> Subject: Re: [Rd] another fix for R crashes under enable-strict-barrier, lto, trunk at 72156
> To: "Hin-Tak Leung" <htl10 at users.sourceforge.net>
> Cc: r-devel at r-project.org, "bonsai list" <outmodedbonsai-announce at lists.sourceforge.net>
> Date: Monday, February 20, 2017, 9:56 AM
>
> >>>>>
> Hin-Tak Leung <htl10 at users.sourceforge.net>
> >>>>>     on Sat, 11
> Feb 2017 19:30:26 +0000 writes:
>
>     > I haven' t touched R for some 18
> months, and so I have no
>     > idea if
> this is a recent problems or not; but it certainly
>     > did not segfault two years ago. 
> Since it has been
>     > crashing
> (segfault) under 'make check-all' for over a
>     > month, I reckon I'll have to
> look at it myself, to have it
>     >
> fixed.
>
>     > I have
> been having the ' --enable-memory-profiling
> --enable-strict-barrier
> --with-valgrind-instrumentation=2" options
>
>     > for perhaps a
> decade - because I work(ed) with people who
>     > like to write buggy code :-(. And I
> also run 'make
>     > check-all'
> from time to time until two years ago.
>
>     > ./configure
> --enable-memory-profiling --enable-strict-barrier
> --enable-byte-compiled-packages
> --with-valgrind-instrumentation=2 --enable-lto
>
>     > current R dev
> crashes in make check-all . The fix is this:
>
>
>     >
> --- a/src/main/memory.c
>     > +++
> b/src/main/memory.c
>     > @@ -3444,7
> +3444,7 @@ R_xlen_t (XTRUELENGTH)(SEXP x) { return
> XTRUELENGTH(CHK2(x)); }
>     >  int 
> (IS_LONG_VEC)(SEXP x) { return IS_LONG_VEC(CHK2(x)); }
>
>     >  const char
> *(R_CHAR)(SEXP x) {
>     > -   
> if(TYPEOF(x) != CHARSXP)
>     > +   
> if(x && (TYPEOF(x) != CHARSXP))
>  
>   >         error("%s() can only be
> applied to a '%s', not a '%s'",
>     >           
>    "CHAR", "CHARSXP",
> type2char(TYPEOF(x)));
>     >     
> return (const char *)CHAR(x);
>
>
>     > It is a fairly
> obvious fix to a bug since
>
>     > include/Rinternals.h:#define
> TYPEOF(x) ((x)->sxpinfo.type)
>
>     > and it was trying to de-reference
> "0->sxpinfo.type" (under
>    
> > --enable-strict-barrier I think).
>
> Thank you  Hin-Tak!
>
> I did not yet try to reproduce the segfault,
> and I am not
> the expert here.  Just some
> remarks and a follow up question:
>
> Typically, the above R_CHAR() is equivalent to
> the  CHAR()
> macro which is used in many
> places.  I  _think_ that the bug is
> that
> this is called with '0' instead of a proper SEXP 
> in your
> case and the bug fix may be more
> appropriate "up stream", i.e.,
> at
> the place where that call happens  rather than inside
> R_CHAR.
>
> Any
> chance you saw or can get more info about the location of
> the crash, such as a stack trace ? 
>
> The idiom 
>    if(TYPEOF(x)  ==  <some>SXP)
> is used in many places in the R sources, and I
> think we never
> prepend that with a  'x
> && '  like you propose above.
>
> ______________________________________________
> R-devel at 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-tierney at uiowa.edu
Iowa City, IA 52242                 WWW:  http://www.stat.uiowa.edu