From luke-tierney at uiowa.edu  Mon Feb 20 23:57:22 2017
From: luke-tierney at uiowa.edu (luke-tierney at uiowa.edu)
Date: Mon, 20 Feb 2017 16:57:22 -0600 (CST)
Subject: [Rd] another fix for R crashes under enable-strict-barrier, lto,
 trunk@72156
In-Reply-To: <1216112776.2681006.1487631046686@mail.yahoo.com>
References: <1216112776.2681006.1487631046686.ref@mail.yahoo.com>
	<1216112776.2681006.1487631046686@mail.yahoo.com>
Message-ID: <alpine.DEB.2.20.1702201655270.2589@luke-Latitude>

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


[Rd] another fix for R crashes under enable-strict-barrier, lto, trunk@72156
luke-tierney at uiowa.edu