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

[Hin-Tak Leung]

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.