[Tinyos-devel] Small memory leak in sim_log.c and some other minor issues from Valgrind

[Philip Levis]

On Jul 24, 2008, at 7:21 AM, Andrey Gursky wrote:

> Hi!
>
> I've let an application (I'm working on) go under Valgrind. Here it is
> the result:
>
> ==8555== 1,211 bytes in 131 blocks are definitely lost in loss  
> record 12
> of 22
> ==8555==    at 0x4022AB8: malloc (vg_replace_malloc.c:207)
> ==8555==    by 0x804E4B9: fillInOutput (sim_log.c:80)
> ==8555==    by 0x804E3F4: sim_log_debug (sim_log.c:242)
> ==8555==    by 0x805F21A: sim_mote_set_start_time (SimMoteP.nc:121)
> ==8555==    by 0x804C3D2: Mote::bootAtTime(long long) (tossim.c:150)
> ==8555==    by 0x8062271: main (TestTymoLoop.c:62)
>
> The line 80 in $(TOSROOT)/tos/lib/tossim/sim_log.c
>
>   char* newName = (char*)malloc(strlen(name) + 1);
>

This is definitely a leak. It pertains to creating debugging channels;  
the memory structure for a channel, once created, is never freed. The  
situation that would cause the leak to be significant is if the driver  
script creates and destroys lots of debugging channels dynamically.  
So, this should definitely be fixed, but it is not critical.


> --------------
>
> Also there are many invalid reads
>
> ==8555== Invalid read of size 1
> ==8555==    at 0x4024B62: memcpy (mc_replace_strmem.c:402)
> ==8555==    by 0x8058240: SerialActiveMessageC$Model$receive
> (SerialActiveMessageC.nc:143)
> ==8555==    by 0x8058203:
> SerialActiveMessageC$serial_active_message_deliver_handle
> (SerialActiveMessageC.nc:270)
> ==8555==    by 0x804F34E: sim_run_next_event (sim_tossim.c:135)
> ==8555==    by 0x804C014: Tossim::runNextEvent() (tossim.c:280)
> ==8555==    by 0x80623DF: main (TestTymoLoop.c:95)
>
> in $(TOSROOT)/tos/lib/tossim/sf/sim/SerialActiveMessageC.nc
>
> line 143:
>         memcpy(bufferPointer, msg, sizeof(message_t));

This one is a bit funny. Having never used valgrind, I assume invalid  
read means that the source pointer to memcpy (second argument) is  
somehow invalid? Due to TinyOS's buffer swap semantics, this could be  
very hard to track down.


>
> ------------
>
> two backtraces of "Source and destination overlap in memcpy"
>
> line 240 of $(TOSROOT)/tos/lib/tossim/sim_noise.c :
>   memcpy(pKey, pKey+1, NOISE_HISTORY-1);
>
>
> 1)
> ==8555== Source and destination overlap in memcpy(0x81A9560,  
> 0x81A9561, 19)
> ==8555==    at 0x4024B12: memcpy (mc_replace_strmem.c:402)
> ==8555==    by 0x80506A1: arrangeKey (sim_noise.c:240)
> ==8555==    by 0x80502F4: makeNoiseModel (sim_noise.c:423)
> ==8555==    by 0x80501B6: sim_noise_create_model (sim_noise.c:77)
> ==8555==    by 0x804C2B1: Mote::createNoiseModel() (tossim.c:201)
> ==8555==    by 0x806230C: main (TestTymoLoop.c:73)
>
> 2)
> ==8555== Source and destination overlap in memcpy(0x81A9560,  
> 0x81A9561, 19)
> ==8555==    at 0x4024B12: memcpy (mc_replace_strmem.c:402)
> ==8555==    by 0x80506A1: arrangeKey (sim_noise.c:240)
> ==8555==    by 0x8051856: sim_noise_generate (sim_noise.c:391)
> ==8555==    by 0x80564C6: CpmModelC$noise_hash_generation  
> (CpmModelC.nc:125)
> ==8555==    by 0x805632B: CpmModelC$packetNoise (CpmModelC.nc:264)
> ==8555==    by 0x805621B: CpmModelC$Model$clearChannel (CpmModelC.nc: 
> 203)
> ==8555==    by 0x80561E1: TossimPacketModelC$GainRadioModel 
> $clearChannel
> (GainRadioModel.nc:48)
> ==8555==    by 0x805509A: TossimPacketModelC$send_backoff
> (TossimPacketModelC.nc:205)
> ==8555==    by 0x804F34E: sim_run_next_event (sim_tossim.c:135)
> ==8555==    by 0x804C014: Tossim::runNextEvent() (tossim.c:280)
> ==8555==    by 0x80623DF: main (TestTymoLoop.c:95)

This code was written with the assumption that it's OK to overlap  
source and destination in memcpy; I guess it's not! You're supposed to  
use memmove in this case.

Unfortunately, it's unlikely these will make their way into the next  
TinyOS release, as we're in the end of testing. But they will go in  
after that. Any help you can provide on the serial bug would be  
greatly appreciated.

Phil