[Bioc-devel] Segfault/Valgrind/bgx

[Herve Pages]

Hi Ernest,

Maybe you want to use snprintf() instead of sprintf(), that would be _much_ safer!

Cheers,
H.

Ernest Turro wrote:
> 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
> 
> _______________________________________________
> Bioc-devel at stat.math.ethz.ch mailing list
> https://stat.ethz.ch/mailman/listinfo/bioc-devel