search

stephenslab / mashr

An R package for multivariate adaptive shrinkage.
https://stephenslab.github.io/mashr
Other
88 stars 19 forks source link

Avoid segfault in proj_EM if logging is disabled #76

Closed jthiltges closed 4 years ago

jthiltges commented 4 years ago

proj_EM may generate log output without checking that logfile is defined. If logfile is not defined, this causes a segfault.

Check that keeplog is set before writing to logfile.

gdb output showing the segfault, with logfile set to NULL ``` Program terminated with signal SIGSEGV, Segmentation fault. #0 0x00002b82632f4ee9 in vfprintf () from /lib64/libc.so.6 [Current thread is 1 (Thread 0x2b8267e02120 (LWP 64352))] (gdb) bt #0 0x00002b82632f4ee9 in vfprintf () from /lib64/libc.so.6 #1 0x00002b82632fffc8 in fprintf () from /lib64/libc.so.6 #2 0x00002b827ff24170 in proj_EM (data=0x55c09a093150, N=704, gaussians=0x55c04cdaffe0, K=6, fixamp=0x55c04989d400, fixmean=0x55c04989d420, fixcovar=0x55c04989d440, avgloglikedata=0x7ffd6b72eb88, tol=9.9999999999999995e-07, maxiter=1000000000, likeonly=false, w=0, keeplog=false, logfile=0x0, tmplogfile=0x0, noproj=true, diagerrs=false, noweight=true) at proj_EM.c:64 #3 0x00002b827ff25e11 in proj_gauss_mixtures (data=0x55c09a093150, N=704, gaussians=0x55c04cdaffe0, K=6, fixamp=0x55c04cba4e60, fixmean=0x55c04b8de360, fixcovar=0x55c04c3bb200, avgloglikedata=0x7ffd6b72eb88, tol=9.9999999999999995e-07, maxiter=1000000000, likeonly=false, w=0, splitnmerge=0, keeplog=false, logfile=0x0, convlogfile=0x0, noproj=true, diagerrs=false, noweight=true) at proj_gauss_mixtures.c:135 ... (gdb) frame 2 #2 0x00002b827ff24170 in proj_EM (data=0x55c09a093150, N=704, gaussians=0x55c04cdaffe0, K=6, fixamp=0x55c04989d400, fixmean=0x55c04989d420, fixcovar=0x55c04989d440, avgloglikedata=0x7ffd6b72eb88, tol=9.9999999999999995e-07, maxiter=1000000000, likeonly=false, w=0, keeplog=false, logfile=0x0, tmplogfile=0x0, noproj=true, diagerrs=false, noweight=true) at proj_EM.c:64 64 in proj_EM.c (gdb) print logfile $11 = (FILE *) 0x0 ```
gaow commented 4 years ago

Thank you @jthiltges for catching this. We haven't ran into this bug because apparently we dont have a case where the loglik decreases ... Thanks for the patch I'm going to merge it for now anyways.

We are in fact in the process of improving ED and will come up with a better version of it. It would be of tremendous help if you could share with us your input matrices to cov_ed() function that we can use to test our new ED -- we'll see the behavior of loglik. You can email it to me if you prefer: wangow@gmail.com. Thanks a lot!

jthiltges commented 4 years ago

Thank you. I submitted this PR on behalf of one of our computing center users. I'll pass along your request. Best regards!