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 <maech...@stat.math.ethz.ch> wrote:
Subject: Re: [Rd] another fix for R crashes under enable-strict-barrier, lto,
trunk@72156
To: "Hin-Tak Leung" <ht...@users.sourceforge.net>
Cc: r-devel@r-project.org, "bonsai list"
<outmodedbonsai-annou...@lists.sourceforge.net>
Date: Monday, February 20, 2017, 9:56 AM
>>>>>
Hin-Tak Leung <ht...@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@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
