From ripley at stats.ox.ac.uk  Fri Dec  7 09:25:13 2007
From: ripley at stats.ox.ac.uk (ripley at stats.ox.ac.uk)
Date: Fri,  7 Dec 2007 09:25:13 +0100 (CET)
Subject: [Rd] (PR#10500) Bug#454678: r-base-core: Crash when calling
Message-ID: <20071207082513.C7DA6283416B@mail.pubhealth.ku.dk>

I would say this was user error (insisting on editing non-existent 
rownames), although the argument is documented.  You could argue that 
there are implicit rownames, but they would be 1, 2 ... not row1, row2 
....  And rownames(mat) is NULL.

For an interactive function the best solution seems to be to throw an 
error when the user asks for the impossible.

I'll fix it for 2.7.0: it certainly isn't 'important' as it has gone 
undiscovered for many years, and edit.matrix is itself little used.


BTW, 1:dim(names)[1] is dangerous: it could be 1:0.  That was the 
motivation for seq_len.


On Fri, 7 Dec 2007, edd at debian.org wrote:

>
> Ben,
>
> Thanks for the bug report. I am off two minds about it as discussed below.
> But as it does indeed create a crash / segfault, I am passing this on to the
> R bug tracker.  A suggested two-line patch is below; I tested the patch
> against a 'vanilla' 2.6.1 source tree.
>
> On 6 December 2007 at 19:32, Ben Goodrich wrote:
> | -----BEGIN PGP SIGNED MESSAGE-----
> | Hash: SHA1
> |
> | Package: r-base-core
> | Version: 2.6.1-1
> | Severity: important
> |
> | Hi Dirk,
> |
> | My strong hunch is that this bug should just be forwarded upstream but
> | it might have something to do with libc6 on Debian. To reproduce it, do
> |
> | args(utils:::edit.matrix)
> | mat <- matrix(rnorm(30), nrow = 10, ncol = 3)
> | edit(mat, edit.row.names = TRUE) #crash
>
> I can confirm that it crashes 2.6.0 and 2.6.1.  I also spent the last little
> while building a (non-stripped) debug version that reveals:
>
> (gdb) where
> #0  0xb7b2ef2c in __gconv_transform_utf8_internal () from /lib/i686/cmov/libc.so.6
> #1  0xb7b89f75 in mbrtowc () from /lib/i686/cmov/libc.so.6
> #2  0xb7db05e3 in Rstrwid (str=0x8052010 "\020!\005\b\002", slen=134595712,
>    quote=0) at printutils.c:284
> #3  0xb7db0888 in Rstrlen (s=0x8051ff8, quote=0) at printutils.c:377
> #4  0xb7d2de24 in Rf_formatString (x=0x873bbb8, n=1, fieldwidth=0xbfd0fc04,
>    quote=0) at format.c:62
> #5  0xb7db12b5 in Rf_EncodeElement (x=0x873ba10, indx=100, quote=0, dec=46 '.')
>    at printutils.c:576
> #6  0xb754ae0d in get_col_width (DE=0xbfd100b0, col=1) at dataentry.c:804
> #7  0xb754edb4 in initwin (DE=0xbfd100b0, title=0xb755eed9 "R Data Editor")
>    at dataentry.c:1986
> #8  0xb7549319 in RX11_dataentry (call=0x89b3fe8, op=0x806c970, args=0x8ba40c8,
>    rho=0x89b4bd0) at dataentry.c:382
> #9  0xb7e52771 in do_dataentry (call=0x89b3fe8, op=0x806c970, args=0x8ba40c8,
>    rho=0x89b4bd0) at X11.c:91
> #10 0xb7d6045e in do_internal (call=0x89b4020, op=0x8061fa4, args=0x8ba40c8,
>    env=0x89b4bd0) at names.c:1120
> #11 0xb7d1f352 in Rf_eval (e=0x89b4020, rho=0x89b4bd0) at eval.c:463
> #12 0xb7d21d5d in do_set (call=0x89b4074, op=0x8060df0, args=0x89b4058,
>    rho=0x89b4bd0) at eval.c:1407
> #13 0xb7d1f352 in Rf_eval (e=0x89b4074, rho=0x89b4bd0) at eval.c:463
> #14 0xb7d212b4 in do_begin (call=0x89b2798, op=0x8062458, args=0x89b4090,
>    rho=0x89b4bd0) at eval.c:1159
> #15 0xb7d1f352 in Rf_eval (e=0x89b2798, rho=0x89b4bd0) at eval.c:463
> #16 0xb7d1fb67 in Rf_applyClosure (call=0x89b1c9c, op=0x89b1ba0,
>    arglist=0x89b1e24, rho=0x89b1d7c, suppliedenv=0x89b1cd4) at eval.c:669
> #17 0xb7d60a32 in applyMethod (call=0x89b1c9c, op=0x89b1ba0, args=0x89b1e24,
>    rho=0x89b1d7c, newrho=0x89b1cd4) at objects.c:126
> #18 0xb7d61223 in Rf_usemethod (generic=0x8069af8 "edit", obj=0x8a87868,
>    call=0x89b1e94, args=0x8052110, rho=0x89b1d7c, callrho=0x8073f9c,
>    defrho=0x828f2fc, ans=0xbfd10d00) at objects.c:291
> #19 0xb7d61776 in do_usemethod (call=0x89b1e94, op=0x80711b8, args=0x89b1e78,
>    env=0x89b1d7c) at objects.c:399
> #20 0xb7d1f352 in Rf_eval (e=0x89b1e94, rho=0x89b1d7c) at eval.c:463
> #21 0xb7d1fb67 in Rf_applyClosure (call=0x89b2230, op=0x89b2150,
>    arglist=0x89b1e24, rho=0x8073f9c, suppliedenv=0x8073fb8) at eval.c:669
> #22 0xb7d1f601 in Rf_eval (e=0x89b2230, rho=0x8073f9c) at eval.c:507
> #23 0xb7d4a879 in Rf_ReplIteration (rho=0x8073f9c, savestack=0, browselevel=0,
>    state=0xbfd1116c) at main.c:263
> #24 0xb7d4aa61 in R_ReplConsole (rho=0x8073f9c, savestack=0, browselevel=0)
>    at main.c:312
> #25 0xb7d4bec7 in run_Rmainloop () at main.c:975
> #26 0xb7d4beee in Rf_mainloop () at main.c:982
> #27 0x08048733 in main (ac=0, av=0x0) at Rmain.c:35
> #28 0xb7b27450 in __libc_start_main () from /lib/i686/cmov/libc.so.6
> #29 0x08048691 in _start ()
> (gdb) up
>
> Now, two comments.
>
> Firstly, we all prefer if R would not crash.  So this may need some fixing.
>
> Secondly, I think you are rather close to bordering on user error.  As your
> snippet shows, you need to invoke args on the non-exported edit.matrix to
> learn about the edit.row.names argument. Moreover, you also know full well
> from looking at this that this will only be true when there actually are
> names set --- and you then proceed to call it when there are none.  Guess
> what:  it blows up.
>
> So we could fix this in a number of places.  Here is one which I tested; R
> Core may opt to apply this, or a better version, or to drop the issue:
>
> edd at ron:~/src/debian/R> diff -u R-2.6.1/src/library/utils/R/edit.R{.orig,}
> --- R-2.6.1/src/library/utils/R/edit.R.orig     2007-09-04 17:12:32.000000000 -0500
> +++ R-2.6.1/src/library/utils/R/edit.R  2007-12-06 21:12:32.000000000 -0600
> @@ -166,6 +166,8 @@
>     else names(datalist) <- paste("col", 1:ncol(name), sep = "")
>     modes <- as.list(rep.int(mode(name), ncol(name)))
>     if (edit.row.names) {
> +        if (is.null(dn[[1]])) ## true if forced edit.row.names as TRUE on null
> +            dn[[1]] <- paste("row", 1:dim(name)[1], sep="")
>         datalist <- c(list(row.names = dn[[1]]), datalist)
>         modes <- c(list(row.names = "character"), modes)
>     }
>
>
> This results in:
>> mat <- matrix(rnorm(30), nrow = 10, ncol = 3); edit(mat, edit.row.names = TRUE)
>            col1        col2        col3
> row1   0.6185206  0.32911907 -0.12263839
> row2   0.4553981 -1.77532265  2.06745757
> row3   0.4676557  0.58817426 -0.30507048
> row4   1.1898153 -1.24888167  1.02240513
> row5  -1.4809138  0.05212133  0.25272844
> row6   1.5709981 -1.87496256 -0.05699266
> row7   0.3770318 -0.43538598 -1.28299648
> row8   1.3900096  0.15139637 -1.01168270
> row9  -0.3973376  0.05933193  0.34420058
> row10 -1.4248380  0.86637712 -1.25193470
>> q()
>
> when the matrix is simply saved from the editor -- ie no segfault.
>
> Hope this helps, and best regards,  Dirk
>
>
> | The only other version of R I can get my hands on at the moment (2.5.1,
> | not Debian) does not crash but throws an uninformative error. This crash
> | also occurs when calling fix(), which calls edit(). It does not occur if
> | the matrix being edited has rownames already. A little gdb output is
> | pasted below. -- Thanks, Ben
> |
> | R version 2.6.1 (2007-11-26)
> | Copyright (C) 2007 The R Foundation for Statistical Computing
> | ISBN 3-900051-07-0
> |
> | R is free software and comes with ABSOLUTELY NO WARRANTY.
> | You are welcome to redistribute it under certain conditions.
> | Type 'license()' or 'licence()' for distribution details.
> |
> | (no debugging symbols found)
> | (no debugging symbols found)
> |   Natural language support but running in an English locale
> |
> | R is a collaborative project with many contributors.
> | Type 'contributors()' for more information and
> | 'citation()' on how to cite R or R packages in publications.
> |
> | Type 'demo()' for some demos, 'help()' for on-line help, or
> | 'help.start()' for an HTML browser interface to help.
> | Type 'q()' to quit R.
> |
> | (no debugging symbols found)
> | (no debugging symbols found)
> | > mat <- matrix(rnorm(30), nrow = 10, ncol = 3)
> | > edit(mat, edit.row.names = TRUE)
> | (no debugging symbols found)
> | - ---Type <return> to continue, or q <return> to quit---
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> | (no debugging symbols found)
> |
> | Program received signal SIGSEGV, Segmentation fault.
> | 0xb7b48f2c in __gconv_transform_utf8_internal () from
> | /lib/i686/cmov/libc.so.6
> |
> |
> | - -- System Information:
> | Debian Release: lenny/sid
> |   APT prefers unstable
> |   APT policy: (500, 'unstable')
> | Architecture: i386 (i686)
> |
> | Kernel: Linux 2.6.23.9-slh-smp-1 (SMP w/1 CPU core; PREEMPT)
> | Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
> | Shell: /bin/sh linked to /bin/bash
> |
> | Versions of packages r-base-core depends on:
> | ii  libbz2-1.0              1.0.3-7          high-quality block-sorting
> | file co
> | ii  libc6                   2.7-3            GNU C Library: Shared libraries
> | ii  libgcc1                 1:4.2.2-4        GCC support library
> | ii  libgfortran2            4.2.2-4          Runtime library for GNU
> | Fortran ap
> | ii  libice6                 2:1.0.4-1        X11 Inter-Client Exchange
> | library
> | ii  libjpeg62               6b-14            The Independent JPEG
> | Group's JPEG
> | ii  libpaper-utils          1.1.23           library for handling paper
> | charact
> | ii  libpcre3                7.3-2            Perl 5 Compatible Regular
> | Expressi
> | ii  libpng12-0              1.2.15~beta5-3   PNG library - runtime
> | ii  libreadline5            5.2-3            GNU readline and history
> | libraries
> | ii  libsm6                  2:1.0.3-1+b1     X11 Session Management library
> | ii  libx11-6                2:1.0.3-7        X11 client-side library
> | ii  libxt6                  1:1.0.5-3        X11 toolkit intrinsics library
> | ii  perl                    5.8.8-12         Larry Wall's Practical
> | Extraction
> | ii  refblas3 [libblas.so.3] 1.2-8            Basic Linear Algebra
> | Subroutines 3
> | ii  tcl8.4                  8.4.16-4         Tcl (the Tool Command
> | Language) v8
> | ii  tk8.4                   8.4.16-2         Tk toolkit for Tcl and X11,
> | v8.4 -
> | ii  unzip                   5.52-10          De-archiver for .zip files
> | ii  zip                     2.32-1           Archiver for .zip files
> | ii  zlib1g                  1:1.2.3.3.dfsg-7 compression library - runtime
> |
> | Versions of packages r-base-core recommends:
> | ii  r-base-dev                    2.6.1-1    GNU R installation of
> | auxiliary GN
> | ii  r-recommended                 2.6.1-1    GNU R collection of
> | recommended pa
> |
> | - -- no debconf information
> | -----BEGIN PGP SIGNATURE-----
> | Version: GnuPG v1.4.6 (GNU/Linux)
> |
> | iD8DBQFHWJSEzQDSXIcN85kRAo3+AJwJPOkxyJJrmbziYt98lP3tFXsmnQCdHRUg
> | sQvIJfAZ6cuGifDdBqKjF7c=
> | =+k7N
> | -----END PGP SIGNATURE-----
> |
> |
>
>

-- 
Brian D. Ripley,                  ripley at stats.ox.ac.uk
Professor of Applied Statistics,  http://www.stats.ox.ac.uk/~ripley/
University of Oxford,             Tel:  +44 1865 272861 (self)
1 South Parks Road,                     +44 1865 272866 (PA)
Oxford OX1 3TG, UK                Fax:  +44 1865 272595



[Rd] (PR#10500) Bug#454678: r-base-core: Crash when calling
ripley at stats.ox.ac.uk