iqbal-lab-org / gramtools

Genome inference from a population reference graph
MIT License
92 stars 15 forks source link

Valgrind finds Invalid read (means bad memory access) in BackwardSearchTest.NoVariants1 #28

Closed iqbal-lab closed 7 years ago

iqbal-lab commented 8 years ago

At this commit:

[master a7c1d1e] Fixed bugs in references to data files, and fixed buggy mask input file.

I ran valgrind on the fast unit tests:

valgrind --leak-check=yes ./unittest_bidir_search_bwd_fwd

[==========] Running 9 tests from 1 test case. [----------] Global test environment set-up. [----------] 9 tests from BackwardSearchTest [ RUN ] BackwardSearchTest.NoVariants1 ==78121== Invalid read of size 1 ==78121== at 0x43547E: bidir_search_fwd(sdsl::csa_wt<sdsl::wt_int<sdsl::int_vector<(unsigned char)1>, sdsl::rank_support_v5<(unsigned char)1, (unsigned char)1>, sdsl::select_support_mcl<(unsigned char)1, (unsigned char)1>, sdsl::select_support_mcl<(unsigned char)0, (unsigned char)1> >, 2u, 16777216u, sdsl::sa_order_sa_sampling<(unsigned char)0>, sdsl::isa_sampling<(unsigned char)0>, sdsl::int_alphabet<sdsl::sd_vector<sdsl::int_vector<(unsigned char)1>, sdsl::select_support_mcl<(unsigned char)1, (unsigned char)1>, sdsl::select_support_mcl<(unsigned char)0, (unsigned char)1> >, sdsl::rank_support_sd<(unsigned char)1, sdsl::int_vector<(unsigned char)1>, sdsl::select_support_mcl<(unsigned char)1, (unsigned char)1>, sdsl::select_support_mcl<(unsigned char)0, (unsigned char)1> >, sdsl::select_support_sd<(unsigned char)1, sdsl::int_vector<(unsigned char)1>, sdsl::select_support_mcl<(unsigned char)1, (unsigned char)1>, sdsl::select_support_mcl<(unsigned char)0, (unsigned char)1> >, sdsl::int_vector<(unsigned char)0> > >&, unsigned long, unsigned long, unsigned long, unsigned long, gnu_cxx::__normal_iterator<unsigned char*, std::vector<unsigned char, std::allocator > >, gnu_cxx::__normal_iterator<unsigned char, std::vector<unsigned char, std::allocator > >, std::list<std::pair<unsigned long, unsigned long>, std::allocator<std::pair<unsigned long, unsigned long> > >&, std::list<std::pair<unsigned long, unsigned long>, std::allocator<std::pair<unsigned long, unsigned long> > >&, std::list<std::vector<std::pair<unsigned int, std::vector<int, std::allocator > >, std::allocator<std::pair<unsigned int, std::vector<int, std::allocator > > > >, std::allocator<std::vector<std::pair<unsigned int, std::vector<int, std::allocator > >, std::allocator<std::pair<unsigned int, std::vector<int, std::allocator > > > > > >&, std::vector<int, std::allocator >&, unsigned long, bool&, bool) (bidir_search_fwd.cpp:134) ==78121== by 0x4360AE: BackwardSearchTest_NoVariants1_Test::TestBody() (unittest_bidir_search_bwdfwd.cpp:78) ==78121== by 0x4815C7: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test, void (testing::Test::)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwdfwd) ==78121== by 0x47CA91: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test, void (testing::Test::)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x464042: testing::Test::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x46481F: testing::TestInfo::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x464EB9: testing::TestCase::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x46B67F: testing::internal::UnitTestImpl::RunAllTests() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwdfwd) ==78121== by 0x4828C7: bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl, bool (testing::internal::UnitTestImpl::)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwdfwd) ==78121== by 0x47D7C9: bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl, bool (testing::internal::UnitTestImpl::)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x46A2A5: testing::UnitTest::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x43F343: RUN_ALL_TESTS() (gtest.h:2288) ==78121== Address 0x5341b61 is 0 bytes after a block of size 1 alloc'd ==78121== at 0x4A08EB6: operator new(unsigned long) (vg_replace_malloc.c:287) ==78121== by 0x42981B: __gnu_cxx::newallocator::allocate(unsigned long, void const) (new_allocator.h:94) ==78121== by 0x427BE2: std::_Vector_base<unsigned char, std::allocator >::_M_allocate(unsigned long) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x440955: void std::vector<unsigned char, std::allocator >::_M_emplace_back_aux(unsigned char&&) (vector.tcc:402) ==78121== by 0x440186: void std::vector<unsigned char, std::allocator >::emplace_back(unsigned char&&) (vector.tcc:102) ==78121== by 0x43F95B: std::vector<unsigned char, std::allocator >::push_back(unsigned char&&) (stl_vector.h:900) ==78121== by 0x4359CB: BackwardSearchTest_NoVariants1_Test::TestBody() (unittest_bidir_search_bwdfwd.cpp:59) ==78121== by 0x4815C7: void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test, void (testing::Test::)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwdfwd) ==78121== by 0x47CA91: void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test, void (testing::Test::_)(), char const) (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x464042: testing::Test::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x46481F: testing::TestInfo::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== by 0x464EB9: testing::TestCase::Run() (in /Net/fs1/home/zam/dev/git/gramtools/src/unittest_bidir_search_bwd_fwd) ==78121== [ OK ] BackwardSearchTest.NoVariants1 (64701 ms) [ RUN ] BackwardSearchTest.OneSNP [ OK ] BackwardSearchTest.OneSNP (34704 ms) ..etc

The bad access happens here

` while (it!=sa_intervals.end() && it_rev!=sa_intervals_rev.end() && it_s!=sites.end()) {

                    //calculate sum to return- can do this in top fcns                                                                         

                    if (bidir_search(csa_rev,(*it).first,(*it).second,(*it_rev).first,(*it_rev).second,c)>0) {

                            ++it;

                            ++it_rev;

                            ++it_s;
                    }
                    else {
                            if (it==sa_intervals.begin()) first_del=true;
                            //might need to see first_del from top fcns to check if there are matches in the reference                         
                            it=sa_intervals.erase(it);
                            it_rev=sa_intervals_rev.erase(it_rev);
                            it_s=sites.erase(it_s);
                    }
            }
            ++pat_it;  
            c=*pat_it;   <<<<<<<<<<<< HERE!!

`

iqbal-lab commented 8 years ago

Note, this is in bidir_search_FORWARD

iqbal-lab commented 8 years ago

I think this is because there is just one byte here, and incrementing the iterator goes off the end somehow

iqbal-lab commented 8 years ago

Reproduced on latest commit: 45ab6dc68fe47bb3fa315680e0e33b862bfdbe75

iqbal-lab commented 7 years ago

I know @rffrancon has set up valgrind - is this bug still there?

iqbal-lab commented 7 years ago

Robyn says fixed