BIND 10 #2669: Valgrind issues
BIND 10 Development
do-not-reply at isc.org
Thu Feb 14 11:07:03 UTC 2013
#2669: Valgrind issues
-------------------------------------+-------------------------------------
Reporter: jreed | Owner:
Type: task | jinmei
Priority: medium | Status:
Component: Unclassified | reviewing
Keywords: | Milestone:
Sensitive: 0 | Sprint-20130219
Sub-Project: Core | Resolution:
Estimated Difficulty: 5 | CVSS Scoring:
Total Hours: 0 | Defect Severity: N/A
| Feature Depending on Ticket:
| Add Hours to Ticket: 0
| Internal?: 0
-------------------------------------+-------------------------------------
Changes (by jelte):
* owner: jelte => jinmei
Comment:
Replying to [comment:4 jinmei]:
> Replying to [comment:3 jelte]:
>
> Hmm, I'm afraid I don't know how to review this branch...
>
> > I got rid of most of the fork()-related issues by adding --child-
silent-after-fork=yes (technically this could result in missed issues in
forking unit tests, but currently it is reporting leaks from stack
allocations...)
>
> Could you elaborate a bit more? What does "leaks from stack
> allocations" mean? Specifically what kind of error was reported for
> which part of the code? Isn't there any false negatives by
> suppressing this? (and if there is, are you suggesting suppressing
> others at the cost of missing them is more beneficial?)
>
Here's the first that gets hit on my system:
{{{
==4834== at 0x4C286E7: operator new(unsigned long)
(/tmp/buildd/valgrind-3.7.0/coregrind/m_replacemalloc/vg_replace_malloc.c:287)
==4834== by 0x572A998: std::string::_Rep::_S_create(unsigned long,
unsigned long, std::allocator<char> const&) (in /usr/lib/x86_64-linux-
gnu/libstdc++.so.6.0.17)
==4834== by 0x572C384: char* std::string::_S_construct<char
const*>(char const*, char const*, std::allocator<char> const&,
std::forward_iterator_tag) (in /usr/lib/x86_64-linux-
gnu/libstdc++.so.6.0.17)
==4834== by 0x572C462: std::basic_string<char, std::char_traits<char>,
std::allocator<char> >::basic_string(char const*, std::allocator<char>
const&) (in /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.17)
==4834== by 0x4518D9: isc::util::(anonymous
namespace)::InterprocessSyncFileTest_TestLock_Test::TestBody()
(/home/jelte/repos/bind10/src/lib/util/tests/interprocess_sync_file_unittest.cc:56)
==4834== by 0x4A68B9: void
testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,
void>(testing::Test*, void (testing::Test::*)(), char const*)
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:2090)
==4834== by 0x49C3A6: testing::Test::Run()
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:2162)
==4834== by 0x49C47D: testing::TestInfo::Run()
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:2338)
==4834== by 0x49C5CC: testing::TestCase::Run()
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:2445)
==4834== by 0x49C83C: testing::internal::UnitTestImpl::RunAllTests()
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:4237)
==4834== by 0x4A6439: bool
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*)
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:2090)
==4834== by 0x49BA29: testing::UnitTest::Run()
(/home/jelte/apps/gtest-1.6.0/src/gtest.cc:3874)
==4834== by 0x40CE9E: main
(/home/jelte/repos/bind10/src/lib/util/tests/run_unittests.cc:24)
==4834==
{
<insert_a_suppression_name_here>
Memcheck:Leak
fun:_Znwm
fun:_ZNSs4_Rep9_S_createEmmRKSaIcE
fun:_ZNSs12_S_constructIPKcEEPcT_S3_RKSaIcESt20forward_iterator_tag
fun:_ZNSsC1EPKcRKSaIcE
fun:_ZN3isc4util12_GLOBAL__N_138InterprocessSyncFileTest_TestLock_Test8TestBodyEv
fun:_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS_4TestEvEET0_PT_MS4_FS3_vEPKc
fun:_ZN7testing4Test3RunEv
fun:_ZN7testing8TestInfo3RunEv
fun:_ZN7testing8TestCase3RunEv
fun:_ZN7testing8internal12UnitTestImpl11RunAllTestsEv
fun:_ZN7testing8internal35HandleExceptionsInMethodIfSupportedINS0_12UnitTestImplEbEET0_PT_MS4_FS3_vEPKc
fun:_ZN7testing8UnitTest3RunEv
fun:main
}
}}}
(note, perhaps it is slightly more clear that it is about the std::string
creation if you change the test to first create a string variable and pass
that to the constructor of InterProcessSyncLocker)
From what I can tell, it's not even so much the fork() itself, it's the
call to exit() that makes it not free all. Perhaps we can get around that
by adding an ExitPlease exception that is caught in the main() of our unit
tests.
(simply returning instead of exit appears to make other tests fail, and
making a local try/catch that exists has the same problem since then the
exception itself isn't freed)
If you have any other ideas, or if i missed something obvious, please let
me know :)
> > I ran it on the centos machine, and there were two issues left in my
checkout:
> > - one was an uninitialized memory block, fixed in the second commit
>
> This one basically looks okay, although I'd do
> {{{#!cpp
> memset(data_, 0, sizeof(data_));
> }}}
> Another note is that you may need to include <cstring> for memset.
> And, to this end, I'd personally even use `vector<uint8_t>` for data_
> (then we won't have to worry about such low level thing as
> initialization with such source-of-all-evil API as memset).
>
ack, changed to vector.
> > - the other one appears to be inside the specific version of libsqlite
on that old centos. I think I reported this as well last time, and more
recent versions don't seem to have this issue. Anyway if we're not gonna
upgrade, this is the repression I used on the centos buildbot:
>
> I'm not sure about how to comment on this. Are you suggesting we
> upgrade sqlite3 on the buildbox that runs valgrind?
actually i had my two systems mixed up; the buildbot version seems fine,
it is my local centos VM that has the bad libsqlite :) (my original plan
was to either upgrade or suppress that one issue)
--
Ticket URL: <http://bind10.isc.org/ticket/2669#comment:6>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list