[sac-dev] Bugs found

[Brian Savage]

Kuang He,

Good work in tracking down both bugs.
I will look them over in a couple of days, after Wednesday probably.
These fixes will have to wait until after 101.2.

Cheers
Brian

On Sep 8, 2008, at  2:17 AM , Kuang He wrote:

> On Sat, Sep 6, 2008 at 2:06 AM, Kuang He <icrazy at gmail.com> wrote:
>> I'm using SAC v101.1 on a linux box (Ubuntu 8.04), and the glibc
>> version is 2.7 (2.7-10ubuntu3, to be exact).
>>
>> $ uname -a
>> Linux ....... 2.6.24-19-generic #1 SMP Fri Jul 11 23:41:49 UTC 2008
>> i686 GNU/Linux
>> .....
>> Bug 2: Putting a space after the comma in something like "&1,DIST"
>> will _sometimes_ cause SAC to suddenly abort, with a message from
>> glibc indicating possible double free. Below is an example of a case
>> where this problem does not show up and another case where the  
>> problem
>> does show up.
>>
>> $ sac
>> SAC> r vel.sac
>> SAC> evaluate to dist1 &1,dist
>> SAC> message %dist1
>>  2.84897$
>> SAC> evaluate to dist1 &1, dist
>> *** glibc detected *** /usr/local/sac/bin/sac: double free or
>> corruption (!prev): 0x0843f020 ***
>> ======= Backtrace: =========
>> /lib/tls/i686/cmov/libc.so.6[0xb7c9ba85]
>> /lib/tls/i686/cmov/libc.so.6(cfree+0x90)[0xb7c9f4f0]
>> ....
>
> With Brian's help, I was able to locate the place in the source code
> that caused this problem (his OSX machines don't have this problem):
> line 93 of src/cpf/cfmt.c . The code there does not have double
> free()'s, but code snippets shown below just do not make any sense to
> me: temporal variable strtemp1 gets created and destroyed, without
> doing anything useful at all. Commenting out these does solve the
> problem.
>
> $ diff -u src/cpf/cfmt.c.old src/cpf/cfmt.c
> --- src/cpf/cfmt.c.old  2008-09-08 00:07:33.000000000 -0400
> +++ src/cpf/cfmt.c      2008-09-08 01:48:27.000000000 -0400
> @@ -86,11 +86,14 @@
>                         iend_ = ibeg + nchar + 2;
>                         if( iend_ <= MCMSG ){
>                                 kmsg[ibeg - 1] = kmcom.kcom[j - 1][0];
> +                               /*
>                                  strtemp1 = malloc(MCMSG+1-(ibeg+1));
>                                  strncpy(strtemp1,kmsg+ibeg,MCMSG+1- 
> (ibeg+1));
>                                  strtemp1[MCMSG+1-(ibeg+1)] = '\0';
>                                 copykc( (char*)kmcom.kcom[j + 1],9,
> nchar, strtemp1);
>                                  free(strtemp1);
> +                               */
>                                 kmsg[iend_ - 2] = kmcom.kcom[j - 1] 
> [0];
>                                 kmsg[iend_ - 1] = ' ';
>                                 if( j == cmcom.jcom )
> @@ -103,11 +106,13 @@
>                         nchar = (long)( Flnum[j + 1] + 0.1 );
>                         iend_ = ibeg + nchar;
>                         if( iend_ <= MCMSG ){
> +                               /*
>                                  strtemp1 = malloc(MCMSG+1-ibeg);
>                                  strncpy(strtemp1,kmsg+ibeg-1,MCMSG 
> +1-ibeg);
>                                  strtemp1[MCMSG+1-ibeg] = '\0';
>                                 copykc( (char*)kmcom.kcom[j + 1],9,
> nchar, strtemp1);
>                                  free(strtemp1);
> +                               */
>                                 kmsg[iend_ - 1] = ' ';
>                                 if( j == cmcom.jcom )
>                                         iarrow = ibeg - ndiff;
>
>
> By the way, I think wrapping all the uses of free() to FREE() shown
> below would be a good idea. The catch is just that since the code base
> is too big, it'll take quite some time to change all of them.
>
> #define FREE(ptr)   do { if (ptr) free(ptr); } while (0)
>
>
> Best regards,
>
> -- 
> Kuang He
> Department of Physics
> University of Connecticut
> Storrs, CT 06269-3046
>
> Tel: +1.860.486.4919
> Web: http://www.phys.uconn.edu/~he/
> _______________________________________________
> sac-dev mailing list
> sac-dev at iris.washington.edu
> http://www.iris.washington.edu/mailman/listinfo/sac-dev