valgrind tests don't test anything

valgrind tests don't test anything

[Arkadiusz Drabczyk]

I'm not sure this is what David Gibson meant in
67b6b33b9b413a450a72135b5dc59c0a1e33e647 when he said:

"At present the valgrinding won't do anything useful for testcases
invoked via a shell script - which includes all the dtc testcases.  I
plan to fix that later."

but if valgrind is called without --leak-check=full as in line 1038 in
tests/run_tests.sh it always returns 0 to the parent process and it
looks like there are no errors but in fact dtc suffers from memory
leaks:

$ valgrind ~/dtc/dtc -O dts t2080qds.dts
(...)
==9428==
==9428== HEAP SUMMARY:
==9428==     in use at exit: 286,968 bytes in 8,355 blocks
==9428==   total heap usage: 11,162 allocs, 2,807 frees, 1,157,178 bytes allocated
==9428==
==9428== LEAK SUMMARY:
==9428==    definitely lost: 5,619 bytes in 174 blocks
==9428==    indirectly lost: 106 bytes in 6 blocks
==9428==      possibly lost: 0 bytes in 0 blocks
==9428==    still reachable: 281,243 bytes in 8,175 blocks
==9428==         suppressed: 0 bytes in 0 blocks
==9428== Rerun with --leak-check=full to see details of leaked memory
==9428==
==9428== For lists of detected and suppressed errors, rerun with: -s
==9428== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

If valgrind invocation in tests/run_tests is changed to something like
this:

VALGRIND="valgrind --leak-check=full --tool=memcheck -q --error-exitcode=$VGCODE"

then `make checkm' fails:

********** TEST SUMMARY
*     Total testcases:  2067
*                PASS:  1552
*                FAIL:  1
*   Bad configuration:  0
*    valgrind errors:   514
* Strange test result:  0
**********
tests/Makefile.tests:92: recipe for target 'checkm' failed
make: *** [checkm] Error 1

-- 
Arkadiusz Drabczyk <arkadiusz-42WfZ8EewN5g9hUCZPvPmw@public.gmane.org>

Re: valgrind tests don't test anything

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2856 bytes --]

On Sun, Mar 08, 2020 at 07:32:21PM +0100, Arkadiusz Drabczyk wrote:
> I'm not sure this is what David Gibson meant in
> 67b6b33b9b413a450a72135b5dc59c0a1e33e647 when he said:
> 
> "At present the valgrinding won't do anything useful for testcases
> invoked via a shell script - which includes all the dtc testcases.  I
> plan to fix that later."
> 
> but if valgrind is called without --leak-check=full as in line 1038 in
> tests/run_tests.sh it always returns 0 to the parent process and it
> looks like there are no errors but in fact dtc suffers from memory
> leaks:
> 
> $ valgrind ~/dtc/dtc -O dts t2080qds.dts
> (...)
> ==9428==
> ==9428== HEAP SUMMARY:
> ==9428==     in use at exit: 286,968 bytes in 8,355 blocks
> ==9428==   total heap usage: 11,162 allocs, 2,807 frees, 1,157,178 bytes allocated
> ==9428==
> ==9428== LEAK SUMMARY:
> ==9428==    definitely lost: 5,619 bytes in 174 blocks
> ==9428==    indirectly lost: 106 bytes in 6 blocks
> ==9428==      possibly lost: 0 bytes in 0 blocks
> ==9428==    still reachable: 281,243 bytes in 8,175 blocks
> ==9428==         suppressed: 0 bytes in 0 blocks
> ==9428== Rerun with --leak-check=full to see details of leaked memory
> ==9428==
> ==9428== For lists of detected and suppressed errors, rerun with: -s
> ==9428== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
> 
> If valgrind invocation in tests/run_tests is changed to something like
> this:
> 
> VALGRIND="valgrind --leak-check=full --tool=memcheck -q --error-exitcode=$VGCODE"
> 
> then `make checkm' fails:
> 
> ********** TEST SUMMARY
> *     Total testcases:  2067
> *                PASS:  1552
> *                FAIL:  1
> *   Bad configuration:  0
> *    valgrind errors:   514
> * Strange test result:  0
> **********
> tests/Makefile.tests:92: recipe for target 'checkm' failed
> make: *** [checkm] Error 1

Uhh... I don't think it's accurate to say the valgrind tests don't
test *anything*.  They're not checking for leaks, but they're still
checking for use after free, use of uninitialized data and so forth.

I'm actually not particularly concerned about leaks in dtc, because
it's a strictly short runtime transient process.  You can think if it
as using the OS process as a rudimentary pool allocator.  Leaks in
libfdt would be a problem... but libfdt doesn't use the allocator at
all, so they're essentially impossible.  Between those two is probably
why I never enabled the valgrind leak detector.

If you want to submit patches which remove leaks from dtc, I'll apply
them, but as noted, I don't really care enough to track them down
myself.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: valgrind tests don't test anything

[Arkadiusz Drabczyk]

On Mon, Mar 09, 2020 at 07:29:20PM +1100, David Gibson wrote:
> Uhh... I don't think it's accurate to say the valgrind tests don't
> test *anything*.  They're not checking for leaks, but they're still
> checking for use after free, use of uninitialized data and so forth.

Ok, indeed, sorry, my fault.  I should have said that valgrind tests
do not check for memory leaks.
 
> I'm actually not particularly concerned about leaks in dtc, because
> it's a strictly short runtime transient process.  You can think if it
> as using the OS process as a rudimentary pool allocator.  Leaks in
> libfdt would be a problem... but libfdt doesn't use the allocator at
> all, so they're essentially impossible.  Between those two is probably
> why I never enabled the valgrind leak detector.

Ok, I see.  As a sidenote, --tool=memcheck is the default option for
valgrind so it could be removed but it's not a big deal.

-- 
Arkadiusz Drabczyk <arkadiusz-42WfZ8EewN5g9hUCZPvPmw@public.gmane.org>