R devel - Oct 2018 - Seg fault stats::runmed

Dear all,

I just found this issue:

dd1 = c(rep(NaN,82), rep(-1, 144), rep(1, 74))
xx = runmed(dd1, 21)

-> R crashes reproducibly in R 3.4.3, R3.4.4 (Ubuntu 14.04/Ubuntu 16.04)

With GDB:
Program received signal SIGSEGV, Segmentation fault.
swap (l=53, r=86, window=window at entry=0xc59308, 
outlist=outlist at entry=0x12ea2e8, nrlist=nrlist at entry=0x114fdd8, 
print_level=print_level at entry=0) at Trunmed.c:64
64??? ??? outlist[nr/* = nrlist[l] */] = l;

Valgrind also reports access to unallocated memory and/or writing past 
the end of the heap.

The crash does not happen if the order is changed:

dd2 = c(rep(-1, 144), rep(1, 74), rep(NaN,82))
xx = runmed(dd2,21)

Error in if (a < b) { : missing value where TRUE/FALSE needed

Best regards,
Hilmar

-- 
Dr. Hilmar Berger, MD
Max Planck Institute for Infection Biology
Charit?platz 1
D-10117 Berlin
GERMANY

Phone:  + 49 30 28460 430
Fax:    + 49 30 28460 401
  
E-Mail: berger at mpiib-berlin.mpg.de
Web   : www.mpiib-berlin.mpg.de

>>>>> Hilmar Berger 
>>>>>     on Fri, 5 Oct 2018 10:17:49 +0200 writes:
    > Dear all, I just found this issue:

    > I just found this issue:

    > dd1 = c(rep(NaN,82), rep(-1, 144), rep(1, 74))
    > xx = runmed(dd1, 21)

    >> R crashes reproducibly in R 3.4.3, R3.4.4 (Ubuntu 14.04/Ubuntu
16.04)

and also in the latest development version (we call "R-devel").

THank you very much, Hilmar!

I will have a look, to ensure missing values (incl NaN) are
handled propertly.

Martin

--
Martin Maechler
ETH Zurich and R Core Team

    > With GDB:
    > Program received signal SIGSEGV, Segmentation fault.
    > swap (l=53, r=86, window=window at entry=0xc59308, 
    > outlist=outlist at entry=0x12ea2e8, nrlist=nrlist at entry=0x114fdd8, 
    > print_level=print_level at entry=0) at Trunmed.c:64
    > 64??? ??? outlist[nr/* = nrlist[l] */] = l;

    > Valgrind also reports access to unallocated memory and/or writing past 
    > the end of the heap.

    > The crash does not happen if the order is changed:

    > dd2 = c(rep(-1, 144), rep(1, 74), rep(NaN,82))
    > xx = runmed(dd2,21)

    > Error in if (a < b) { : missing value where TRUE/FALSE needed

>>>>> Martin Maechler 
>>>>>     on Fri, 5 Oct 2018 12:16:37 +0200 writes:
>>>>> Hilmar Berger 
>>>>>     on Fri, 5 Oct 2018 10:17:49 +0200 writes:
    >> Dear all, I just found this issue:

    >> I just found this issue:

    >> dd1 = c(rep(NaN,82), rep(-1, 144), rep(1, 74))
    >> xx = runmed(dd1, 21)

    >>> R crashes reproducibly in R 3.4.3, R3.4.4 (Ubuntu 14.04/Ubuntu
16.04)

    > and also in the latest development version (we call
"R-devel").

    > THank you very much, Hilmar!

    > I will have a look, to ensure missing values (incl NaN) are
    > handled propertly.

That "look" had several parts to it, with long breaks in between;
finally Hilmar kindly asked me privately, and I committed
changes to R-devel (and R 3.6.0 patched), with NEWS entry

    * runmed(x, *) when x contains missing values now works for
      algorithm="Stuetzle", also based on smoothEnds(y) working with
      NA's, and no longer segfaults for the "Turlach" algorithm;
      reported by Hilmar Berger.

but the changes were not at all sufficient to correctly deal
with NA / NaN's in runmed() --- and hence the above NEWS entry
was "fake news" as some may call it.

So, the last 2 weeks or so, I've spent several working days and
some extra hours trying to get this going. {several tries proved
to be insufficient, logically wrong, too optimistic, ...}

In the end, I've implemented a simplistic "imputation"-scheme
for the default case, *and* also added a new optional argument
'na.action' to runmed(),
and committed this half an hour ago :

------------------------------------------------------------------------
r76744 | maechler | 2019-06-27 15:51:04 +0200 (Thu, 27. Jun 2019) 

   M doc/NEWS.Rd
   M src/library/stats/R/runmed.R
   M src/library/stats/man/runmed.Rd
   M src/library/stats/man/smoothEnds.Rd
   M src/library/stats/src/Srunmed.c
   M src/library/stats/src/Trunmed.c
   M src/library/stats/src/init.c
   M src/library/stats/src/statsR.h
   M tests/Examples/stats-Ex.Rout.save
   M tests/reg-tests-1d.R

runmed(<NAs>, "Turlach") did still seg.fault. Now, NEWS entry
(76682) should be true; new argument `na.action = ".."` determines
*how* NA/NaN are treated
------------------------------------------------------------------------

As this does prevent seg.faults, and if it is acceptable to the
release process, this may also make it into the upcoming R 3.6.1.

--
Martin Maechler
ETH Zurich and R Core Team