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

2017-02-20 Thread luke-tierney 

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  wrote:

Subject: Re: [Rd] another fix for R crashes under enable-strict-barrier, lto, 
trunk@72156
To: "Hin-Tak Leung" 
Cc: r-devel@r-project.org, "bonsai list" 

Date: Monday, February 20, 2017, 9:56 AM

>
Hin-Tak Leung 
>     on Sat, 11
Feb 2017 19:30:26 + 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)  ==  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

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

2017-02-20 Thread 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  wrote:

 Subject: Re: [Rd] another fix for R crashes under enable-strict-barrier, lto, 
trunk@72156
 To: "Hin-Tak Leung" 
 Cc: r-devel@r-project.org, "bonsai list" 

 Date: Monday, February 20, 2017, 9:56 AM

 >
 Hin-Tak Leung 
 >     on Sat, 11
 Feb 2017 19:30:26 + 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)  ==  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

Re: [Rd] [FORGED] Re: Replaying a recorded plot (mixed base and grid) from pdf() in cairo_pdf() crashes R

2017-02-20 Thread Paul Murrell 

Hi

This appears to be happening (at least) because cairo_pdf() delays 
initialising a Cairo surface until BM_NewPage(), rather than 
initiliasing a Cairo surface in BM_Open(), and replayPlot() triggers 
some activity (set clip region) on the device BEFORE a new page is 
started (so the pointer to the Cairo surface is null, so BOOM).


Not sure yet whether to blame replayPlot() for not starting with a new 
page operation OR to blame cairo_pdf() for not initialising a Cairo 
surface at device startup.


If anyone who knows more about Cairo (or cairo_pdf()) wants to point out 
a good reason for the way cairo_pdf() currently works, please don't hold 
back.


Paul

On 21/02/17 05:30, Yihui Xie wrote:

A quick follow-up: I just used cairo_pdf() as the recording device,
and it crashes R as well, so it is probably not relevant to pdf() but
an issue specific to cairo_pdf().

cairo_pdf()
dev.control('enable')

library("grid")
plot(1)
grid.text("A")

res = recordPlot()
dev.off()

cairo_pdf()
replayPlot(res)
dev.off()


Regards,
Yihui
--
https://yihui.name


On Mon, Feb 20, 2017 at 10:24 AM, Yihui Xie  wrote:

Hi,

I wonder if this is expected or I'm doing a wrong thing.

pdf()
dev.control('enable')

library("grid")
plot(1)
grid.text("A")

res = recordPlot()
dev.off()

cairo_pdf()
replayPlot(res)
dev.off()


 *** caught segfault ***
address 0x4, cause 'memory not mapped'



sessionInfo()

R version 3.3.2 (2016-10-31)
Platform: x86_64-apple-darwin13.4.0 (64-bit)
Running under: macOS Sierra 10.12.3

locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8

attached base packages:
[1] stats graphics  grDevices utils datasets  methods   base

loaded via a namespace (and not attached):
[1] tools_3.3.2 yaml_2.1.14

Regards,
Yihui
--
https://yihui.name


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



--
Dr Paul Murrell
Department of Statistics
The University of Auckland
Private Bag 92019
Auckland
New Zealand
64 9 3737599 x85392
p...@stat.auckland.ac.nz
http://www.stat.auckland.ac.nz/~paul/

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

[Rd] [patch] Eliminate warnings from gcc -Wold-style-declaration

2017-02-20 Thread Mikko Korpela 
Motivated by the recent R-devel list message with the title 
"Registration of native routines", I modified an R package to use 
registration of native routines along the lines of the example in 
section 5.4.2 of R-exts (development version). Among other compiler 
flags, I have '-Wextra' permanently set for installing packages. When 
installing the modified package, I got the following warning (tested 
with gcc 4.8.4 and 6.3.0):



init.c:10:1: warning: ‘static’ is not at beginning of declaration 
[-Wold-style-declaration]
 const static R_CallMethodDef R_CallDef[] = {


I believe this is a case of the compiler (with '-Wold-style-declaration' 
as part of '-Wextra') adhering to the following section of the C99 (or 
C11) standard, see e.g.

:


6.11 Future language directions


...


6.11.5 Storage-class specifiers

The placement of a storage-class specifier other than at the beginning
of the declaration specifiers in a declaration is an obsolescent
feature.


The attached patch (against SVN revision 72220) is a suggestion of how 
to eliminate the sources of such obsolescence warnings from the R source 
tree, including the example in the R-exts manual. The patch was 
originally made for r72218, and against that version it was tested 
successfully on Ubuntu 14.04.5 LTS (gcc 4.8.4) and OS X 10.7.5 (gcc 
6.3.0 installed from Homebrew). Revision 72220 failing 'make check' 
("Error: testing 'utils' failed") with or without the patch must be 
unrelated to the changes made in the patch. The files 
"src/library/tools/src/gramLatex.y", "src/library/tools/src/gramRd.y" 
and "src/main/gram.y" were processed with 'bison -l -y' (GNU Bison 
version 2.7.1, erroneusly reporting the version as 2.7.12-4996) to 
create the corresponding .c files.


As the warnings are only given when the user asks for "extra" warnings, 
this is obviously non-critical and not urgent, but might be something to 
consider.


--
Mikko Korpela
Department of Geosciences and Geography
University of Helsinki
Index: doc/manual/R-exts.texi
===
--- doc/manual/R-exts.texi	(revision 72220)
+++ doc/manual/R-exts.texi	(working copy)
@@ -9549,7 +9549,7 @@
 
 #define CALLDEF(name, n)  @{#name, (DL_FUNC) , n@}
 
-const static R_CallMethodDef R_CallDef[] = @{
+static const R_CallMethodDef R_CallDef[] = @{
CALLDEF(spline_basis, ?),
CALLDEF(spline_value, ?),
@{NULL, NULL, 0@}
@@ -9751,7 +9751,7 @@
 
 #define CALLDEF(name, n)  @{#name, (DL_FUNC) , n@}
 
-const static R_CallMethodDef R_CallDef[] = @{
+static const R_CallMethodDef R_CallDef[] = @{
 CALLDEF(spline_basis, 4),
 CALLDEF(spline_value, 5),
 @{NULL, NULL, 0@}
Index: src/library/grDevices/src/cairo/cairoBM.c
===
--- src/library/grDevices/src/cairo/cairoBM.c	(revision 72220)
+++ src/library/grDevices/src/cairo/cairoBM.c	(working copy)
@@ -1,7 +1,7 @@
 /*
  *  R : A Computer Language for Statistical Data Analysis
  *  Copyright (C) 1995, 1996  Robert Gentleman and Ross Ihaka
- *  Copyright (C) 1997--2015  The R Core Team
+ *  Copyright (C) 1997--2017  The R Core Team
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -449,7 +449,7 @@
 return TRUE;
 }
 
-const static struct {
+static const struct {
 const char * const name;
 X_GTYPE gtype;
 } devtable[] = {
Index: src/library/splines/src/splines.c
===
--- src/library/splines/src/splines.c	(revision 72220)
+++ src/library/splines/src/splines.c	(working copy)
@@ -234,7 +234,7 @@
 
 #define CALLDEF(name, n)  {#name, (DL_FUNC) , n}
 
-const static R_CallMethodDef R_CallDef[] = {
+static const R_CallMethodDef R_CallDef[] = {
 CALLDEF(spline_basis, 4),
 CALLDEF(spline_value, 5),
 {NULL, NULL, 0}
Index: src/library/stats/src/fexact.c
===
--- src/library/stats/src/fexact.c	(revision 72220)
+++ src/library/stats/src/fexact.c	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  R : A Computer Language for Statistical Data Analysis
- *  Copyright (C) 1999-2010   The R Core Team.
+ *  Copyright (C) 1999-2017   The R Core Team.
  *
  *  Based on ACM TOMS643 (1993)
  *
@@ -343,7 +343,7 @@
 const double amiss = -12345.;
 
 /* TOL is chosen as the square root of the smallest relative spacing. */
-const static double tol = 3.45254e-7;
+static const double tol = 3.45254e-7;
 
 const char* ch_err_5 =
 	"The hash table key cannot be computed because the largest key\n"
Index: src/library/tools/src/gramLatex.c
===
--- src/library/tools/src/gramLatex.c	(revision 72220)
+++ src/library/tools/src/gramLatex.c	(working copy)
@@ -2371,11 +2371,11 @@
 /* Special

Re: [Rd] Wish List: Extensions to the derivatives table

2017-02-20 Thread Martin Maechler 
> 
> The issue is that without an extensible derivative table or the proposed 
> extensions, it is not possible to automatically produce (without manual 
> modification of the deriv3 output) a function that avoids catastrophic 
> cancellation regardless of the working range.
> Manual modification is not onerous as a one-time exercise, but can be time 
> consuming when it must be done numerous times, for example when evaluating 
> the impact of different parameterizations on parameter effects curvature.  
> The alternative of more flexible differentiation does not seem to be a 
> difficult addition to R.  In S+ (which does not have deriv3) it would simply 
> involve adding the following lines to the switch statement in D
> 
>   expm1 = make.call("*", make.call("exp", expr[[2]]), D(expr[[2]], name)),
>   log1p = make.call("/", D(expr[[2]], name), make.call("+", 1., expr[[2]])),
>   log2 = make.call("/", make.call("/", D(expr[[2]], name), expr[[2]]), 
> quote(log(2)) ),
>   log10 = make.call("/", make.call("/", D(expr[[2]], name), expr[[2]]), 
> quote(log(10)) ),
>   cospi = make.call("*", make.call("*", make.call("sinpi", expr[[2]]), 
> make.call("-", D(expr[[2]], name))), quote(pi)),
>   sinpi = make.call("*", make.call("*", make.call("cospi", expr[[2]]), 
> D(expr[[2]], name)), quote(pi)),
>   tanpi = make.call("/", make.call("*", D(expr[[2]], name), quote(pi)), 
> make.call("^", make.call("cospi", expr[[2]]), 2)),
> 
> Jerry

You are right, Jerry, it would be nice if R's derivative table
could be extended by the useR  using simple R code.
As Duncan Murdoch has mentioned already, this is now provided as
a byproduct of the functionality in the CRAN package 'nlsr'
{after that is tweaked, as you mentioned}, which is nice and
good to know (for all of us).

As one person who knows how important it may be to avoid cancellation,
I still would be willing to add to the "derivatives table" in
R's C source  if people like you provided  a (tested!) patch to
the source, which is in
https://svn.r-project.org/R/trunk/src/library/stats/src/deriv.c

Martin


> From: Avraham Adler [mailto:avraham.ad...@gmail.com]
> Sent: Friday, February 17, 2017 4:16 PM
> To: Jerry Lewis; r-devel@r-project.org
> Subject: Re: [Rd] Wish List: Extensions to the derivatives table
> 
> Hi.
> 
> Unless I'm misremembering, log, exp, sin, cos, and tan are all handled in 
> deriv3. The functions listed are  specially coded slightly more accurate 
> versions but can be substituted with native ones for which deriv/deriv3 will 
> work automatically. I believe that if you  write your functions using log(a + 
> 1) instead of log1p(a) or log(x) / log(2) instead of log2(x) deriv3 will work 
> fine.


> Thanks,
> Avi
> 
> On Fri, Feb 17, 2017 at 2:02 PM Jerry Lewis 
> > wrote:
> The derivative table resides in the function D.  In S+ that table is 
> extensible because it is written in the S language.  R is faster but less 
> flexible, since that table is programmed in C.  It would be useful if R 
> provided a mechanism for extending the derivative table, or barring that, 
> provided a broader table.  Currently unsupported mathematical functions of 
> one argument include expm1, log1p, log2, log10, cospi, sinpi, and tanpi.
> 
> While manual differentiation of these proposed additions is straight-forward, 
> their absence complicates what otherwise could be much simpler, such as using 
> deriv() or deriv3() to generate functions, for example to use as an nls model.
> 
> Thanks,

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

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

2017-02-20 Thread Martin Maechler 
> Hin-Tak Leung 
> on Sat, 11 Feb 2017 19:30:26 + 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)  ==  SXP)
is used in many places in the R sources, and I think we never
prepend that with a  'x && '  like you propose above.




> So there.

> While I subscribe to R-devel, I switched off delivery, so
> please CC if a response is required.

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