Bug 532

Summary: ./waf --valgrind --regression passes despite memory leak
Product: ns-3 Reporter: Tom Henderson <tomh>
Component: regressionAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal CC: gjcarneiro, mathieu.lacage, raj.b
Priority: P5    
Version: ns-3-dev   
Hardware: All   
OS: All   
Attachments: possible patch
detect valgrind leaks

Description Tom Henderson 2009-03-26 21:58:26 UTC
./waf --valgrind --regression passes despite memory leaks.  This was a problem in the past but was fixed at some point-- now it doesn't work again.

==29514== 128 bytes in 2 blocks are definitely lost in loss record 1 of 1
==29514==    at 0x4C23809: operator new(unsigned long) (vg_replace_malloc.c:230)
==29514==    by 0x54D3FB4: ns3::GlobalRouteManagerImpl::SPFCalculate(ns3::Ipv4Address) (global-route-manager-impl.cc:1166)
==29514==    by 0x54D4748: ns3::GlobalRouteManagerImpl::InitializeRoutes() (global-route-manager-impl.cc:574)
==29514==    by 0x54C8255: ns3::GlobalRouteManager::InitializeRoutes() (global-route-manager.cc:92)
==29514==    by 0x54C829E: ns3::GlobalRouteManager::PopulateRoutingTables() (global-route-manager.cc:41)
==29514==    by 0x4072C4: main (tcp-large-transfer.cc:113)
==29514== 
==29514== LEAK SUMMARY:
==29514==    definitely lost: 128 bytes in 2 blocks.
==29514==      possibly lost: 0 bytes in 0 blocks.
==29514==    still reachable: 0 bytes in 0 blocks.
==29514==         suppressed: 0 bytes in 0 blocks.
PASS test-tcp-large-transfer
==29499== 
==29499== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 7 from 1)
==29499== malloc/free: in use at exit: 0 bytes in 0 blocks.
==29499== malloc/free: 1,657,534 allocs, 1,657,534 frees, 99,070,621 bytes allocated.
==29499== For counts of detected errors, rerun with: -v
==29499== All heap blocks were freed -- no leaks are possible.
PASS test-csma-star
[669/669] regression-test-collector
Regression testing summary:
SKIP: 1 of 22 tests have been skipped (test-tcp-nsc-lfn)
PASS: 21 of 22 tests passed
Build finished successfully (00:05:23)
Comment 1 Rajib Bhattacharjea 2009-03-27 01:22:39 UTC
Does this mean there is a regression in ns-3-dev, or simply that the script passes even IF you introduce a memory leak?
Comment 2 Gustavo J. A. M. Carneiro 2009-03-27 07:49:09 UTC
Created attachment 403 [details]
possible patch

Can you try this patch?
Comment 3 Tom Henderson 2009-03-27 11:25:07 UTC
(In reply to comment #1)
> Does this mean there is a regression in ns-3-dev, or simply that the script
> passes even IF you introduce a memory leak?
> 

No, ns-3-dev was clean; it was some other debugging I was doing.  
Comment 4 Tom Henderson 2009-03-29 17:53:30 UTC
(In reply to comment #2)
> Created an attachment (id=403) [details]
> possible patch
> 
> Can you try this patch?
> 


This didn't work, using ./waf --valgrind --regression with the patched regression.py:

==12619== Memcheck, a memory error detector.
==12619== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==12619== Using LibVEX rev 1804, a library for dynamic binary translation.
==12619== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==12619== Using valgrind-3.3.0-Debian, a dynamic binary instrumentation framework.
==12619== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==12619== For more details, rerun with: -v
==12619== 
==12612== 
==12612== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8 from 1)
==12612== malloc/free: in use at exit: 136 bytes in 3 blocks.
==12612== malloc/free: 72,167 allocs, 72,164 frees, 3,474,423 bytes allocated.
==12612== For counts of detected errors, rerun with: -v
==12612== searching for pointers to 3 not-freed blocks.
==12612== checked 834,536 bytes.
==12612== 
==12612== 136 (24 direct, 112 indirect) bytes in 1 blocks are definitely lost in loss record 2 of 3
==12612==    at 0x4C23809: operator new(unsigned long) (vg_replace_malloc.c:230)
==12612==    by 0x4066E9: main (csma-broadcast.cc:117)
==12612== 
==12612== LEAK SUMMARY:
==12612==    definitely lost: 24 bytes in 1 blocks.
==12612==    indirectly lost: 112 bytes in 2 blocks.
==12612==      possibly lost: 0 bytes in 0 blocks.
==12612==    still reachable: 0 bytes in 0 blocks.
==12612==         suppressed: 0 bytes in 0 blocks.
PASS test-csma-broadcast


To reproduce, just create some object in one of the scripts using operator new and do not later delete it.
Comment 5 Gustavo J. A. M. Carneiro 2009-03-30 09:27:25 UTC
I think this is a bug in valgrind itself :-(

gjc@dark-tower:ns-3-dev$ valgrind --leak-check=full --error-exitcode=1 ./build/debug/examples/csma-star
==24413== Memcheck, a memory error detector.
==24413== Copyright (C) 2002-2007, and GNU GPL'd, by Julian Seward et al.
==24413== Using LibVEX rev 1854, a library for dynamic binary translation.
==24413== Copyright (C) 2004-2007, and GNU GPL'd, by OpenWorks LLP.
==24413== Using valgrind-3.3.1-Debian, a dynamic binary instrumentation framework.
==24413== Copyright (C) 2000-2007, and GNU GPL'd, by Julian Seward et al.
==24413== For more details, rerun with: -v
==24413== 
echo $?
==24413== 
==24413== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 8 from 1)
==24413== malloc/free: in use at exit: 1,234 bytes in 1 blocks.
==24413== malloc/free: 1,651,789 allocs, 1,651,788 frees, 99,071,921 bytes allocated.
==24413== For counts of detected errors, rerun with: -v
==24413== searching for pointers to 1 not-freed blocks.
==24413== checked 627,328 bytes.
==24413== 
==24413== 1,234 bytes in 1 blocks are definitely lost in loss record 1 of 1
==24413==    at 0x4C2582C: operator new[](unsigned long) (vg_replace_malloc.c:274)
==24413==    by 0x40908B: main (csma-star.cc:207)
==24413== 
==24413== LEAK SUMMARY:
==24413==    definitely lost: 1,234 bytes in 1 blocks.
==24413==      possibly lost: 0 bytes in 0 blocks.
==24413==    still reachable: 0 bytes in 0 blocks.
==24413==         suppressed: 0 bytes in 0 blocks.
gjc@dark-tower:ns-3-dev$ echo $?
0
Comment 6 Gustavo J. A. M. Carneiro 2009-03-30 09:31:53 UTC
Similar Debian bug report:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=506239
Comment 7 Mathieu Lacage 2009-03-30 09:32:52 UTC
leaks are not errors:

http://thread.gmane.org/gmane.comp.debugging.valgrind/6504
Comment 8 Mathieu Lacage 2009-04-16 07:17:37 UTC
Created attachment 418 [details]
detect valgrind leaks

this is a working workaround. I would love to be able to check this in.
Comment 9 Tom Henderson 2009-04-16 08:42:39 UTC
(In reply to comment #8)
> Created an attachment (id=418) [details]
> detect valgrind leaks
> 
> this is a working workaround. I would love to be able to check this in.
> 

+1 from me-- surprised that valgrind makes it so hard by default to check this
Comment 10 Gustavo J. A. M. Carneiro 2009-04-16 09:59:11 UTC
OK for me.  I have some nitpicks, but nothing important.

If you want to improve your patch, you can:

 1. Not use os.popen, use subprocess.Popen instead;
 2. Not create a wrapper script, instead regexp-filter in the parent process (via redirect stderr to a pipe and process lines from the pipe instead).

But for the time being, no objections.
Comment 11 Mathieu Lacage 2009-04-16 10:12:20 UTC
(In reply to comment #10)
> OK for me.  I have some nitpicks, but nothing important.
> 
> If you want to improve your patch, you can:
> 
>  1. Not use os.popen, use subprocess.Popen instead;

ok

>  2. Not create a wrapper script, instead regexp-filter in the parent process
> (via redirect stderr to a pipe and process lines from the pipe instead).

I tried to do this but I could not find how to do this in the wscript which builds a command from wutils.py get_command_template. I fear that doing this within the wscript would require extensive changes to the wscript itself, right ?

Comment 12 Gustavo J. A. M. Carneiro 2009-04-16 10:21:38 UTC
True; it requires some more refactoring.  Never mind.  If I have time I'll take care of it some day.
Comment 13 Gustavo J. A. M. Carneiro 2009-04-16 11:48:08 UTC
OK, I fixed this better, but thanks anyway, your patch helped. :-)