[Bioc-devel] Segfault/Valgrind/bgx

[Ernest Turro]

Hi Martin, Seth,

On 6 May 2007, at 18:45, Seth Falcon wrote:

> Hi Martin, Ernest,
>
> Martin Morgan <mtmorgan at fhcrc.org> writes:
>> Ernest,
>>
>> I wonder if the problem comes from writing to your C variable dirname
>> in bgx.cc:243 ?
>>
>> At the risk of spreading mis-information, from looking at the code  
>> for
>> .C, in src/main/dotcode.c:do_dotCode and RObjToCPtr, it looks like
>> what happens is that 'gobbledigook' is copied to dirname[0], with
>> allocation for strlen(gobbledigook)+1 space; you'll then overwite  
>> this
>> with the return path, and bad things happen when the return path is
>> longer than gobbledigook.
>


Thanks so much - that was indeed it. In the past, the dirname string  
was "run.1", "run.2", etc... so in practice no more than 5 characters  
long. But just before the package was accepted, I had to change it to  
file.path(tempdir(),"bgx") by default, thus making it potentially a  
lot longer. I forgot to increase the length of of dirname  
apporpriately. Well spotted - thank you!


> Nice catch, Martin.  I think there is a real issue there.
> Overwritting the char* buffer is a problem, but you won't always see a
> crash because it depends on what you overwrite.  Below are extracts
> from a small test package that can be used to demonstrate this.
>
> Now, while we are taking a look at bgx, one comment for Ernest:
>
>    R provides nice random number and statistical distribution
>    functions and you might consider using them (instead of
>    bgx/src/qnorm.c, for example) -- at least when running as an R
>    package.  I encourage you to consider using .Call instead of .C as
>    you will have a lot more control over the inputs and what you
>    return to R.
>

Thanks Seth. I'll definitely look into this for the next release.

Cheers,

Ernest


>
> + seth
>
> ### hello.R ###
>
>     hello <- function(x) {
>         .C("dotc_hello", x=as.character(x),
>            out="HELLO1234",
>            PACKAGE="hello")$out
>     }
>
> ### hello.c ###
>
>     #include <stdio.h>
>     #include <string.h>
>     #include <Rinternals.h>
>     #include <R_ext/Rdynload.h>
>
>     void dotc_hello(char **name, char **out);
>
>     static const R_CMethodDef cMethods[] = {
>         {"dotc_hello", (DL_FUNC)&dotc_hello, 2},
>         {NULL, NULL, 0}
>     };
>
>     void R_init_hello(DllInfo *info)
>     {
>       R_registerRoutines(info, cMethods, NULL, NULL, NULL);
>     }
>
>     void dotc_hello(char **name, char **out)
>     {
>         int nprinted, navail;
>         navail = strlen(out[0]);
>         nprinted = sprintf(out[0], "hello, %s", name[0]);
>         if (nprinted > navail) {
>             Rprintf("used %d, but only %d available\n", nprinted,  
> navail);
>         }
>     }
>
>
>
>>
>> Perhaps pass the return path in from R?
>>
>> Hope that helps,
>>
>> Martin
>>
>> Ernest Turro <ernest.turro at ic.ac.uk> writes:
>>
>>> Dear all,
>>>
>>> my package ( svn co https://hedgehog.fhcrc.org/bioconductor/trunk/
>>> madman/Rpacks/bgx ) segfaults sometimes. I say sometimes because,  
>>> for
>>> instance, on lamb1 it doesn't, and on wellington it does, on my own
>>> machine it doesn't - except if I run it through valgrind...). When I
>>> run the code as a standalone binary, there are no segfaults. Anyway,
>>> the problem according to valgrind's stack trace appears to be in
>>> libR.so - not directly in my own C code. I think it occurs when  
>>> calling:
>>>
>>>    # bgx/R/mcmc.R:36
>>>    # free allocated memory on user interrupt/end of simulation
>>>    on.exit(.C("freeBGXMemory", as.integer(out.ind), as.integer
>>> (numberGenesToWatch), PACKAGE = "bgx"))
>>>
>>> Specifically, when calling as.integer(numberGenesToWatch).
>>>
>>> The relevant part of valgrind's stack trace is below. The full trace
>>> is below that. Does anyone have any ideas? I've been debugging for
>>> quite a while and I really don't know what more to do..
>>>
>>> Many thanks!
>>
>> -- 
>> Martin Morgan
>> Bioconductor / Computational Biology
>> http://bioconductor.org
>>
>> _______________________________________________
>> Bioc-devel at stat.math.ethz.ch mailing list
>> https://stat.ethz.ch/mailman/listinfo/bioc-devel
>>
>
> -- 
> Seth Falcon | Computational Biology | Fred Hutchinson Cancer  
> Research Center
> http://bioconductor.org