search

kylelutz / chemkit

A C++ library for molecular modelling, cheminformatics and molecular visualization.
http://www.chemkit.org
BSD 3-Clause "New" or "Revised" License
54 stars 26 forks source link

Fix invalid memory read in ring perception #2

Closed kylelutz closed 12 years ago

kylelutz commented 13 years ago

Problem: Valgrind reports an "Invalid read of size 4" in the SSSR implementation in the libchemkit library.

How to reproduce: Run valgrind on the benzimidazole method of the ring-perception auto test. The output is as follows:

$ valgrind ./ringperceptiontest benzimidazole
==25592== Memcheck, a memory error detector
==25592== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==25592== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for copyright info
==25592== Command: ./ringperceptiontest benzimidazole
==25592== 
********* Start testing of RingPerceptionTest *********
Config: Using QTest library 4.7.0, Qt 4.7.0
PASS   : RingPerceptionTest::initTestCase()
==25592== Invalid read of size 4
==25592==    at 0x4E865D2: chemkit::(anonymous namespace)::Sssr::isUnique(std::vector > const&) const (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E883D6: chemkit::MolecularGraph::sssr_rpPath(chemkit::MolecularGraph const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E826F6: chemkit::MolecularGraph::sssr(chemkit::Fragment const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E827EA: chemkit::MolecularGraph::sssr(chemkit::Molecule const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E91894: chemkit::Molecule::rings() const (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E919A8: chemkit::Molecule::ringCount() const (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x40FD06: RingPerceptionTest::benzimidazole() (in /home/kyle/dev/chemkit/build/tests/auto/chemkit/ring-perception/ringperceptiontest)
==25592==    by 0x42E987: RingPerceptionTest::qt_metacall(QMetaObject::Call, int, void**) (in /home/kyle/dev/chemkit/build/tests/auto/chemkit/ring-perception/ringperceptiontest)
==25592==    by 0x545CBF8: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (in /usr/lib/libQtCore.so.4.7.0)
==25592==    by 0x545E305: QMetaObject::invokeMethod(QObject*, char const*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) (in /usr/lib/libQtCore.so.4.7.0)
==25592==    by 0x50D1385: ??? (in /usr/lib/libQtTest.so.4.7.0)
==25592==    by 0x50D2321: QTest::qExec(QObject*, int, char**) (in /usr/lib/libQtTest.so.4.7.0)
==25592==  Address 0x8953ee4 is 0 bytes after a block of size 20 alloc'd
==25592==    at 0x4C28973: operator new(unsigned long) (vg_replace_malloc.c:261)
==25592==    by 0x4E6F229: std::vector >::vector(std::vector > const&) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E6FA0C: std::vector >, std::allocator > > >::_M_insert_aux(__gnu_cxx::__normal_iterator >*, std::vector >, std::allocator > > > >, std::vector > const&) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E88D1A: chemkit::MolecularGraph::sssr_rpPath(chemkit::MolecularGraph const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E826F6: chemkit::MolecularGraph::sssr(chemkit::Fragment const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E827EA: chemkit::MolecularGraph::sssr(chemkit::Molecule const*) (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E91894: chemkit::Molecule::rings() const (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x4E919A8: chemkit::Molecule::ringCount() const (in /home/kyle/dev/chemkit/build/src/chemkit/libchemkit.so)
==25592==    by 0x40FD06: RingPerceptionTest::benzimidazole() (in /home/kyle/dev/chemkit/build/tests/auto/chemkit/ring-perception/ringperceptiontest)
==25592==    by 0x42E987: RingPerceptionTest::qt_metacall(QMetaObject::Call, int, void**) (in /home/kyle/dev/chemkit/build/tests/auto/chemkit/ring-perception/ringperceptiontest)
==25592==    by 0x545CBF8: QMetaMethod::invoke(QObject*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) const (in /usr/lib/libQtCore.so.4.7.0)
==25592==    by 0x545E305: QMetaObject::invokeMethod(QObject*, char const*, Qt::ConnectionType, QGenericReturnArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument, QGenericArgument) (in /usr/lib/libQtCore.so.4.7.0)
==25592== 
PASS   : RingPerceptionTest::benzimidazole()
PASS   : RingPerceptionTest::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped
********* Finished testing of RingPerceptionTest *********
==25592== 
==25592== HEAP SUMMARY:
==25592==     in use at exit: 760 bytes in 7 blocks
==25592==   total heap usage: 1,774 allocs, 1,767 frees, 57,684 bytes allocated
==25592== 
==25592== LEAK SUMMARY:
==25592==    definitely lost: 0 bytes in 0 blocks
==25592==    indirectly lost: 0 bytes in 0 blocks
==25592==      possibly lost: 0 bytes in 0 blocks
==25592==    still reachable: 760 bytes in 7 blocks
==25592==         suppressed: 0 bytes in 0 blocks
==25592== Rerun with --leak-check=full to see details of leaked memory
==25592== 
==25592== For counts of detected and suppressed errors, rerun with: -v
==25592== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)
kylelutz commented 12 years ago

Fixed by: 0a2a657250c9052f4983afaae0e57785639a8f6c